Amphtml: amp-ad 'Ad' attribution does not collapse if no ad returned

[safster]

What's the issue?

amp-ad 'Ad' attribution does not collapse when no ad returned

Briefly describe the bug/feature request.
amp-ad allows publishers to collapse the iframe if no ad is returned. However, the 'Ad' attribution element does not collapse with the iframe, thus leaving an ad attribution with no ad in the page.

How do we reproduce the issue?

The below URLs have an amp-ad setup that if no ad is returned (refresh several times until no ad is returned) still show the 'Anzeige' (localised version of 'Ad' in German) attribution:
https://www.google.de/amp/amp.handelsblatt.com/stromnetze-in-deutschland-energieverband-warnt-vor-zu-langsamen-ausbau-/19708306.html
https://www.google.de/amp/amp.wiwo.de/der-tag-nach-der-frankreich-wahl-dax-erklimmt-nach-abstimmung-rekordhoch-/19706872.html
https://www.google.de/amp/m.faz.net/aktuell/gesellschaft/tiere/jagd-auf-zugvoegel-80-euro-pro-portion-rotkehl

If this is a feature request you can use this section to point to a prototype/mockup that will help us understand the request.

What browsers are affected?

All

Which AMP version is affected?

Is this a new issue? Or was it always broken? Paste your AMP version. You can find it in the browser dev tools.
New issue following from the recent ad placeholder release.

[jridgewell]

/to @zhouyx

[jasti]

@zhouyx Can you think of a way to allow a publisher provided "ad label" that can be loaded as part of the same iframe that collapses when there is no ad?
@safster Thanks for the issue - we'll prioritize this.

[zhouyx]

With our current code, I can't think of a good way to collapse an element in response to another element collapses. 😢

[jridgewell]

Use a amp-ad:before selector? When the ad collapses, the before will too.

[TimLohmann]

EDIT: I checked the pages again and you are right, that the sites don't use the "Anzeige" on the amp-ad:before but on a parent div element. But I am pretty sure that this won't change the effect because of the height:0. I will retest the issue.

When the ad collapses the height will be set to 0. This means amp-ad:before will still be shown.
It would be better if amd-ad is set to display:none. Then amp-ad:before will be invisible, too.
Is it possible to change the collapsing behaviour to display:none instead of height:0?

In addition to the ticket information: The used adserver on this sites is "doubleclick"

[TimLohmann]

@jridgewell
Sorry, I think I was mistaken by an earlier test. I have retested my amp-ad implementation with you suggestion and it works.
Sorry for my wrong comment before.
I will notify our clients to change the ad attribution to amp-ad:before

Thank you for your help! In my opinion the ticket is resolved.

[zhouyx]

Thanks @jridgewell for the great suggestion.
I don't know about the :before selector, just played with it. One question I have is with it would the ad label be placed inside the <amp-ad> element?

[jridgewell]

Technically, yes. But you can move the psuedo-element with absolute positioning, etc.

[zhouyx]

Hmmm, I do believe amp-ad has overflow: hidden though

[TimLohmann]

@zhouyx
Yes, you are right. amp-ad receives a class i-amphtml-layout-size-defined which has the css statement overflow:hidden !important;

In my test I was able to overwrite it with giving the amp-ad an id-attribute and setting overflow:visible !important; to that id.

I don't know if this breaks any amp rules and should be avoided. In this case I would be happy if someone can clarify this for me.

So my quick and dirty example:

<!-- somewhere in the page -->
<style>
amp-ad#ad1 { overflow:visible !important;}
</style>

<!-- somewhere else in the page -->
<amp-ad id="ad1" ...>...</amp-ad>

This fixed the problem for me and I was able to setup an "Ad" attribution to my ads in the page like @jridgewell suggested.

I hope this helps in your case,too.

Sorry for commenting in the closed ticket, but I thought the last question was unanswered and I wanted to provide a solution.

[zhouyx]

@TimLohmann !important will make the page an invalid AMP page. Please refer to doc

[TimLohmann]

@zhouyx
Thank you for pointing me to the specific rule!

@topic
In this case there will be no quick solution without altering some code. May be it is possible to add an attribute to amp-ad which contains the text for the specific advertisment-label which will be positioned above the amp-ad?

In Germany such labels are necessary for identifying ads to the users. I think many websites (inlcuding NY Times) have such labels on their sites, too.

[zhouyx]

cc @jasti

[jasti]

In AMP, we'll create a class name that ad wrappers can append to themselves. Therefore, when amp-ad collapses, the runtime will automatically try to collapse the wrapper class too. The requirement for an ad wrapper to be collapsed is that the ad wrapper has to be the direct parent of amp-ad.
Will be picked up in the next few sprints.

[ampprojectbot]

This issue hasn't been updated in awhile. @jasti Do you have any updates?

[ilovric00]

cc @calebcordry

[ampprojectbot]

This issue hasn't been updated in awhile. @jasti Do you have any updates?

[jasti]

@lannka candidate for fix-it sprint?

[zhouyx]

@jasti This issue is not a quick fix. We need agree on a design.

One idea is to have the <amp-ad> component create a div as its child and allow publisher to customize that div. The drawback is the visibility tracking can be not that accurate in that case, because we track the visibility of the parent <amp-ad> component, not the actually ad iframe.

We get multiple requests for the feature. Should considering prioritizing it.

[calebcordry]

+1 For prioritizing. I have had several people ask for this.

[jasti]

I had talked to @lannka a while ago about this and we arrived at this solution for implementation: https://github.com/ampproject/amphtml/issues/8941#issuecomment-331550222
@zhouyx could you connect with @lannka to see if that is still feasible?
We can prioritize it for the next sprint.

[zhouyx]

We will be reusing the <amp-layout> component as a container of the ad, and then ask publisher to reference the parent container within the <amp-ad> element.

<amp-layout id='ad-container'>
  Advertisement
  <amp-ad container-id='ad-container'></amp-ad>
</amp-layout>

A summary of approaches we decide not to proceed:

1. Using :before
   Not applicable because visibility:hidden on <amp-ad>

2. Create one extra layer of container div within <amp-ad>

<amp-ad>
  <div></div>
  <iframe></iframe>
</amp-ad>

Not applicable because in this case the iframe can't fill the <amp-ad> element. And visibility tracking and responsiveness will break

1. Allow publishers to reference the indicator element to the <amp-ad> element

<div id='ind'>Advertisement</div>
<amp-ad indicator='ind'></amp-ad>

A collapse to the indicator element after the collapse of the <amp-ad> element can potentially cause page jump.

1. Allow publisher to put a div container around ad, and reference the container div

<div id='wrapper'>
Advertisement
<amp-ad wrapper='wrapper'></amp-ad>
</div>

The AMP runtime infrastructure requires an AMP element to perform attemptCollapse to best determine the outcome of the collapse and whether the attempt can be fulfilled

1. Introduce a <amp-ad-container> element that ships with the amp-ad-0.1.js. When parent <amp-ad-container> exists collapse the container instead of the <amp-ad> element.

<amp-ad-container><amp-ad></amp-ad></amp-ad-container>

Kind of confusing due to the existence of two ad containers <amp-sticky-ad>``<amp-flying-carpet> Since <amp-layout> has already provided similar functionalities, decide to save one more component name.

cc @lannka @aghassemi