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

Adagio Adapter: getSlotPosition breaks ad slot styles #9035

Closed
pokutuna opened this issue Sep 25, 2022 · 0 comments · Fixed by #9036
Closed

Adagio Adapter: getSlotPosition breaks ad slot styles #9035

pokutuna opened this issue Sep 25, 2022 · 0 comments · Fixed by #9036

Comments

@pokutuna
Copy link
Contributor

Type of issue

Bug

Description

Since adding the Adagio adapter, I have found that some ad slots remain hidden.

Adagio adapter sends adunit_position which means slot position from window.top with bid request.

Adagio adapter temporarily set display: block to the slot element to compute its position, and then reverts to the previous computed style.

const elComputedStyle = wt.getComputedStyle(domElement, null);
const elComputedDisplay = elComputedStyle.display || 'block';
const mustDisplayElement = elComputedDisplay === 'none';
if (mustDisplayElement) {
domElement.style = domElement.style || {};
domElement.style.display = 'block';
box = domElement.getBoundingClientRect();
domElement.style.display = elComputedDisplay;
}

This moves the style from the stylesheet to the elements and away from the control of the website to switch classes.

This causes problems when ad slots have placed that switch between display states.
For example, when a ad slot is placed in a collapsible sidebar or when content and an ad slot are switched by some condition.

In this situation, the ad slot itself or the page part containing the ad slot may have display: none at bidding time and continue to retain the style by Adagio adapter.

To avoid this problem, the adapter should revert to the previous style of the element itself, rather than computed style.

Steps to reproduce

Occurs when SafeFrame is not used and window.top is accessible.

} else if (canAccessTopWindow()) {

1. Starts with hidden ad slots

<style>
  .is-hidden { display: none; }
</style>
<div id="slot1" class="is-hidden"></div>

<div id="container" class="is-hidden">
  <div id="slot2"></div>
</div>

2. Adapter calculates slot positions

<style>
  .is-hidden { display: none; }
</style>
<div id="slot1" class="is-hidden" style="display: block;"></div>

<div id="container" class="is-hidden">
  <div id="slot2" style="display: block;"></div>
</div>

3. Adapter reverts styles to computed

<style>
  .is-hidden { display: none; }
</style>
<div id="slot1" class="is-hidden" style="display: none;"></div>

<div id="container" class="is-hidden">
  <div id="slot2" style="display: none;"></div>
</div>

4. Publisher site attempts to display by removing class

<style>
  .is-hidden { display: none; }
</style>
<div id="slot1" class="" style="display: none;"></div>

<div id="container" class="">
  <div id="slot2" style="display: none;"></div>
</div>

Test page

Expected results

Actual results

Platform details

Other information

When working with Google Ad Manager, gpt.js (pubads_impl_*.js) may eventually work correctly by updating the styles of elements (removing display: none for ad slot).

However, problems occur when using other ad servers.

It is better not to depend on the gpt.js behavior. And the fix is easy.

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

Successfully merging a pull request may close this issue.

1 participant