-
Notifications
You must be signed in to change notification settings - Fork 216
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
resolveAll() broken and conceptually wrong. #359
Comments
Hello @redman684, thanks for spotting the typos, we will fix them ASAP. Indeed the default value is false. Which version of vast-client are you using ? I'm using the About the conception being wrong, the IAB specify that The purpose of resolveAll is to avoid useless calls to resolve every wrapper chain of the initial VAST by setting it to false as most times only the first Ad or AdPod are needed (following ones are usually either optional ads or fallback ones). Let me know if you can reproduce the issue with the |
I'm using Master. Didn't notice it was that far behind. The documentation doesn't seem changed at this point. The last call to getNextAds returns ad2, ad3 and ad4 according to the example in the documentation. I have not tried the actual call. OK, so when a wrapper is present, the pod will be returned first (as specified by VAST3). While when no wrapper is present, for which VAST3 specifically does not specify an order, the ads are returned in the order they occur in the VAST, instead of pod first. I must agree, it is not wrong, although it did cause my confusion. Another thing which caused my issues, is that the sequence has to start at 1 at the and increment by 1. This is indeed as specified, although I didn't expect it to be implemented this strict. What is an error, is that a 3-2-1 sequence is not accepted. VAST explicitly specifies the sequence needs not appear in the right order or back to back. |
I've only tested it with a single VAST, but passing resolveAll=false does not return only the ad pod (if available) or first ad. It returns all ads of the VAST.
I'm using the vast-client.js in a browser.
According to https://github.com/dailymotion/vast-client-js/blob/master/docs/api/vast-client.md passing the resolveAll=false parameters should return only the first Ad or AdPod.
It said here https://github.com/dailymotion/vast-client-js/blob/master/docs/api/vast-client.md#parameters-1 the default is true, but the default actually is false.
Then in https://github.com/dailymotion/vast-client-js/blob/master/docs/api/vast-client.md#how-does-resolveall-work it is explained that passing resolveAll=false would return only ad1 as a starter.
The first call to getNextAds() whould return ad2 and ad3, which I can understand.
The second call to getNextAds() whould return ad2, ad3 and ad4, while this should be only ad4.
PS. In the image the 4th ad is labeled ad1. An obvious typo.
If this (minus the typos) were indeed what would happen, it would still be wrong. https://www.iab.com/wp-content/uploads/2015/06/VASTv3_0.pdf describes that when an ad pod is present, that pod should be played and the remaining ads are part of a buffet which the player can pick from when ads in the pod fail.
So get() should return ad2 and ad3 (the pod).
The first call to getNextAds() should return ad1, the second call to getNextAds() should return ad4.
In reality it does neither. It returns all ads. Or at least it does when the ads are in a single VAST, like the examples seem to describe.
What I think is happening here, and what is what the name "resolveAll" is referring to, is that it does not download any further URLs once an ad has been encountered.
The text was updated successfully, but these errors were encountered: