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

resolveAll() broken and conceptually wrong. #359

Closed
redman684 opened this issue May 20, 2020 · 2 comments
Closed

resolveAll() broken and conceptually wrong. #359

redman684 opened this issue May 20, 2020 · 2 comments

Comments

@redman684
Copy link

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.

@ZacharieTFR
Copy link
Contributor

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 3.0-version and the last call to getNextAds return only ad4.

About the conception being wrong, the IAB specify that When the Ad Pod is served as a singular Inline response (without a Wrapper), playing the Pod or selecting one or more standalone ads from the buffet is left to the video player’s discretion which can be applied to the use case that you mentionned. On that the specs are a little fuzzy. In order for get() to return the adpod first, the adpod should be placed before any standalone ads.

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 3.0-version

@redman684
Copy link
Author

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.
Here they need to be, which is a odd, as the elements in XML are considered unordered (unless specified otherwise).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants