ampproject / amphtml

The AMP web component framework.
https://amp.dev
Apache License 2.0
14.89k stars 3.89k forks source link

Add marginheight=0 and marginwidth=0 to FIE #24019

Closed calebcordry closed 5 years ago

calebcordry commented 5 years ago

The amp runtime contains CSS that will set body: margin 0 !important. These styles used to also apply to FIEs rendered by amp-ad as well as inabox.

Advertisers had valid complaints about this logic, as there was no way for the creative to override this rule.

A few months ago, as part of the amp inabox CSS refactor we changed this logic so that we no longer are injecting the unnecessary amp runtime styles into creatives. Specifically the body margin rule was eliminated in #22274

This has altered the presentation of some number creatives that were reliant on the the body margin rule. There are also complications around some sell side tools that will not let you include CSS targeting <body> to override browser defaults (DV360 in particular).

I am proposing that we add two new attributes to FIEs created by amp marginheight=0 and marginwidth=0. This would change the default styling back to the way it was before the CSS refactoring, but would allow advertisers to easily override this behavior if they desire.

My understanding is that GPT is already doing this, so it would be moving our implementation closer to its existing behavior. We also spot checked a few other ad networks on non-amp pages, and this seem be common across ad networks.

calebcordry commented 5 years ago

to/ @jeffkaufman @keithwrightbos @ampproject/wg-ads Does anyone have questions/concerns about doing this?

calebcordry commented 5 years ago

cc/ @sushan04