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 having multiple Criteo standalone ads on one page #11285

Merged
merged 1 commit into from Sep 27, 2017
Merged

Fix having multiple Criteo standalone ads on one page #11285

merged 1 commit into from Sep 27, 2017

Conversation

Spark-NF
Copy link
Contributor

Ads: Fix having multiple Criteo standalone ads

The usage of computeInMasterFrame for Criteo standalone was removed.

Not only forcing the bidding to happen on the master frame was unnecessary, it also meant that only one standalone frame would get its bid request sent (and its display done, indirectly).

Copy link
Contributor

@zhouyx zhouyx left a comment

Choose a reason for hiding this comment

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

LGTM

@zhouyx zhouyx merged commit a47109c into ampproject:master Sep 27, 2017
@zhouyx zhouyx moved this from Ads backlog to Done in 3P Ads & Analytics Support Sep 27, 2017
dmvjs pushed a commit to dmvjs/amphtml that referenced this pull request Jan 31, 2018
dmvjs added a commit to dmvjs/amphtml that referenced this pull request Feb 6, 2018
…o ke/amp-addthis

* 'ke/amp-addthis' of https://github.com/dmvjs/amphtml: (62 commits)
  type definitions to appease GCC
  type definitions to appease GCC
  type definitions to appease GCC
  type definitions to appease GCC
  type definitions to appease GCC
  type definitions to appease GCC
  type definitions to appease GCC
  test fixes
  Add required logic for view/engagement analytics without an iframe
  Lojson call refactor; saving WIP
  Code review cleanup
  Make all images in AMP have the `async` attribute. (ampproject#11366)
  skip test (ampproject#11459)
  Fix having multiple Criteo standalone ads on one page (ampproject#11285)
  add `bt` (binary type) query param to error reporting query string (ampproject#11452)
  Enables vendor iframe to send response to creative (ampproject#11306)
  Remove noisy 3p error for user error reporting (ampproject#11382)
  Ramp Fast Fetch crypto verifier experiment to 10% (ampproject#11447)
  Updates doubleclick impl to include docs on fluid (ampproject#11437)
  Revert "Add a visual diff test for amp-pinterest" (ampproject#11442)
  ...
dmvjs added a commit to dmvjs/amphtml that referenced this pull request Feb 6, 2018
…o jr/amp-addthis

* 'jr/amp-addthis' of https://github.com/dmvjs/amphtml: (54 commits)
  Add required logic for view/engagement analytics without an iframe
  Lojson call refactor; saving WIP
  Code review cleanup
  Make all images in AMP have the `async` attribute. (ampproject#11366)
  skip test (ampproject#11459)
  Fix having multiple Criteo standalone ads on one page (ampproject#11285)
  add `bt` (binary type) query param to error reporting query string (ampproject#11452)
  Enables vendor iframe to send response to creative (ampproject#11306)
  Remove noisy 3p error for user error reporting (ampproject#11382)
  Ramp Fast Fetch crypto verifier experiment to 10% (ampproject#11447)
  Updates doubleclick impl to include docs on fluid (ampproject#11437)
  Revert "Add a visual diff test for amp-pinterest" (ampproject#11442)
  Ramp experiment down to 0 (ampproject#11441)
  Updated changelog with recent validator release (ampproject#11440)
  Remove pfx parameter from Doubleclick Fast Fetch reuests (ampproject#11439)
  Allow element to accept custom share attributes (with fallback); remove other ownerDocument refs
  Add wait code for amp.article.html (ampproject#11429)
  fix inabox-viewport test (ampproject#11428)
  Remove camelcase from data attributes
  Amp Ad Fast Fetch allow amp-image prefetch (ampproject#11391)
  ...
dmvjs added a commit to dmvjs/amphtml that referenced this pull request Feb 6, 2018
…o jr/amp-addthis

* 'jr/amp-addthis' of https://github.com/dmvjs/amphtml: (54 commits)
  Add required logic for view/engagement analytics without an iframe
  Lojson call refactor; saving WIP
  Code review cleanup
  Make all images in AMP have the `async` attribute. (ampproject#11366)
  skip test (ampproject#11459)
  Fix having multiple Criteo standalone ads on one page (ampproject#11285)
  add `bt` (binary type) query param to error reporting query string (ampproject#11452)
  Enables vendor iframe to send response to creative (ampproject#11306)
  Remove noisy 3p error for user error reporting (ampproject#11382)
  Ramp Fast Fetch crypto verifier experiment to 10% (ampproject#11447)
  Updates doubleclick impl to include docs on fluid (ampproject#11437)
  Revert "Add a visual diff test for amp-pinterest" (ampproject#11442)
  Ramp experiment down to 0 (ampproject#11441)
  Updated changelog with recent validator release (ampproject#11440)
  Remove pfx parameter from Doubleclick Fast Fetch reuests (ampproject#11439)
  Allow element to accept custom share attributes (with fallback); remove other ownerDocument refs
  Add wait code for amp.article.html (ampproject#11429)
  fix inabox-viewport test (ampproject#11428)
  Remove camelcase from data attributes
  Amp Ad Fast Fetch allow amp-image prefetch (ampproject#11391)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants