Skip to content

Fix AdPods parsing behavior with resolveAll to false#487

Merged
ZacharieTFR merged 4 commits intomasterfrom
ad-pod-resolving
Feb 13, 2025
Merged

Fix AdPods parsing behavior with resolveAll to false#487
ZacharieTFR merged 4 commits intomasterfrom
ad-pod-resolving

Conversation

@ZacharieTFR
Copy link
Copy Markdown
Contributor

Description

Previously, when a VAST contained an adPod with standalone ads, if a standalone ad appeared first in the VAST order, it would be parsed first, causing the adPod to not be resolved. As a result, the Player couldn't be aware that the VAST contained an adPod. According to IAB specifications, the adPod should take priority and play before any standalone ads. This PR addresses the issue by ensuring that the adPod is resolved first, followed by the standalone ads, when getNextAds is called.

Issue

#360

Type

  • Breaking change
  • Enhancement
  • Fix
  • Documentation
  • Tooling

@ZacharieTFR ZacharieTFR self-assigned this Jan 24, 2025
@dm-ad-sdk dm-ad-sdk Bot requested a review from clarissedmn January 27, 2025 08:54
@clementFrancon clementFrancon self-requested a review January 27, 2025 09:10
Comment thread spec/vast_client.spec.js Outdated
});

it('resolves only next ad if requested', async () => {
fit('resolves only next ad if requested', async () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps we can create a ticket to add an ESLint rule to prevent fit from being pushed to production?

Suggested change
fit('resolves only next ad if requested', async () => {
it('resolves only next ad if requested', async () => {

@clarissedmn
Copy link
Copy Markdown
Contributor

After removing fit from the tests, it looks like two tests are failing

@ZacharieTFR ZacharieTFR merged commit 22b9de8 into master Feb 13, 2025
@ZacharieTFR ZacharieTFR deleted the ad-pod-resolving branch February 13, 2025 14:57
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