Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix bug where fallback was not working if first ad element return empty #466

Merged
merged 1 commit into from
Jun 26, 2024

Conversation

Rapha0511
Copy link
Contributor

Description

https://github.com/dailymotion/vast-client-js/blob/master/src/parser/vast_parser.js#L461C25-L461C25

The condition will never be reach since !resolvedAds will always be false.
So in case the first Ad element of a VAST is empty or it fails, it will never fallback to the next Ad element.

Issue

If any Ad element (going from top to bottom) fails to fetch xml or return empty vast, it will not fallback to next Ad Element.

Issue Link

Type

  • Breaking change
  • Enhancement
  • Fix
  • Documentation
  • Tooling

@@ -458,7 +458,7 @@ export class VASTParser extends EventEmitter {
return Promise.all(resolveWrappersPromises).then((unwrappedAds) => {
const resolvedAds = util.flatten(unwrappedAds);

if (!resolvedAds && this.remainingAds.length > 0) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me

!0 = true
!(non zero length) = false

@shubham-si
Copy link

shubham-si commented May 24, 2024

@ZacharieTFR @Rapha0511 when we are planning to release this fix

@ZacharieTFR ZacharieTFR merged commit e7d4c38 into master Jun 26, 2024
2 checks passed
@ZacharieTFR ZacharieTFR deleted the fix/fallback-when-empty-vast branch June 26, 2024 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants