<Impression> tags are not extracted from a parent wrapper

[shlomitc]

Causes the wrapping VAST provider to not be notified when the inner-vast impression is sent.

I think this is quite a simple fix and I will open a PR for that, but I just wanted to know where is the right place to do it.

No doubt it should inside the VASTAd function flow. At first look I thought it should be where you copy tracking and creatives from parent (line 578). But since parentAd.impressions is an array of string, you don't need to copy the same way you extract TrackingEvents (companions/nonlinears/...)

Any other thoughts?

[jonhoo]

Somewhere around 578 sounds good. You should have access to the parent's impressions using pa.impressions.

[Fire-Brand]

+1 on that PR :+1:

[shlomitc]

I started my PR and got to look into the code the tests. When I added my fix tests failed, so I had to look what I broke, but tests looks perfect.

It seems that whoever integrated the library in my project, did not not use the track() function of the TrackingEvents object to fire the events. He just extracted them from the parsed ads object and used another pixel firing mechanism instead.

So this means I should parse the wrapper tags out myself instead of adding it to the VASTAd as I suggested.

I'll skip this PR, but it was a great way to understand the library, which is quite brilliant! Anyway, I'll open a a PR and add grunt tasks to run the tests with every build.

[jonhoo]

Great, thank you!

[sterpe]

@shlomitc @jonhoo, I also run into an inability to use the track() function effectively, as there is no way to know when the pixel was either 200 OK, aborted, or errored and in the case where multiple pixels have been collected in the bucket for the same event, there wasn't a clear way to introduce an aggregate cb() or promise to the function. (To me at least).

[jonhoo]

@sterpe: it should be possible to add some error tracking to the track() requests (using .onerror/.onload for Image() and the return status for XHR), and then simply return a list of promises (or perhaps a promise.all or promise.any). After all, track() doesn't currently return anything useful, so this would be backwards compatible too.