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

isSafeFrame detection not detecting AMP forced SafeFrame #242

Open
Sir-Will opened this issue Oct 6, 2024 · 4 comments
Open

isSafeFrame detection not detecting AMP forced SafeFrame #242

Sir-Will opened this issue Oct 6, 2024 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@Sir-Will
Copy link
Contributor

Sir-Will commented Oct 6, 2024

Describe the bug
While testing an integration in AMP environment, I encountered the issue of it not resizing the iframe. It turns out that the isSafeFrame check does not work for the forced SafeFrame by AMP.

The moment SafeFrame is being enabled in GAM, the detection works.

export function isSafeFrame(win) {
return !!(win.$sf && win.$sf.ext);
}

The only workaround seems to be to create dedicated line items and creatives for AMP, which has SafeFrame enabled in GAM.

It might make sense to always send the resize message for AMP, if there is no possibility without SafeFrame.

if (isSafeFrame(window)) {

To Reproduce
Steps to reproduce the behavior:

  1. AMP environment page
  2. Serve GAM creative with SafeFrame off
  3. Set breakpoint on isSafeFrame, in amp.js
  4. See it resulting in false and iframe src containing "safeframe" URL

Expected behavior
Outgoing AMP embed-siz message when the creative does not have SafeFrame checked.

@patmmccann
Copy link
Contributor

related: #107

the solution would close both i imagine

@dgirardi
Copy link
Collaborator

dgirardi commented Jan 13, 2025

#249 should produce the expected behavior in the OP:

Outgoing AMP embed-size message when the creative does not have SafeFrame checked.

However, I cannot get it to actually resize the ad. I don't know if that's what it's supposed to do; in the safeframe case the resizing is done by the $sf API calls which are not available in the forced safeframe case. The only embed-size documentation I can find are related to the amp-iframe tag and specify some requirements for it (resizable attribute & allow-same-origin), but just translating those into amp-ad does not appear to work.

Also, I could not find any way to detect an AMP environment. Our logic always runs in what looks like any other safeframe.

@patmmccann
Copy link
Contributor

Also, I could not find any way to detect an AMP environment. Our logic always runs in what looks like any other safeframe.

did #107 or ampproject/amphtml#6829 not help?

@dgirardi
Copy link
Collaborator

No, window.name is empty so that snippet's

var version = JSON.parse(decodeURI(window.name)).ampcontextVersion;

does not work. I suspect there's something I don't know regarding how to set up the AMP page, the creative, or both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Ready for Dev
Development

No branches or pull requests

3 participants