ampproject / amphtml

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

$sf is not defined within the Google safeFrame #19466

Closed github-nourdine-dahmani closed 4 years ago

github-nourdine-dahmani commented 5 years ago

Sorry for reopening https://github.com/ampproject/amphtml/issues/19139 More an more publishers are complaining.

Problem: $sf is not defined within the Google safeFrame. image URL: https://calcio.fanpage.it/amichevole-italia-ucraina-le-ultime-sulle-formazioni-le-quote-e-dove-vederla-in-tv/amp/?google_preview=VR1kgoPGP1MYh5_33QUwh7us5QWIAYCAgKDb8MT0SQ&iu=1010633&gdfp_req=1&lineItemId=4816432979&creativeId=138246844693

Same issue here: https://www.freundin.de/beauty-foehn-im-test?amp&google_preview=vQ8hGQ9h31MY1Ifb3wUw1KOQ5wWIAYCAgKDbkYXoCw&iu=40748507&gdfp_req=1&lineItemId=4870567891&creativeId=138251967304

We tried data-force-safeframe=true, no success. Thanks for your help!

aghassemi commented 5 years ago

to @keithwrightbos cc @lannka

jeffkaufman commented 5 years ago

Reading https://github.com/ampproject/amphtml/blob/master/extensions/amp-ad-network-doubleclick-impl/safeframe.md this looks like it should be working. I made a test page, https://www.jefftk.com/amp-demo/amp-doubleclick-fsf that is configured to serve a test creative that verifies safeframe is configured properly, but it will take some time before my test creative is serving live.

jeffkaufman commented 5 years ago

The test page with safeframe test creative, https://www.jefftk.com/amp-demo/amp-doubleclick-fsf , is working now:

screen shot 2018-11-27 at 2 50 30 pm screen shot 2018-11-27 at 2 50 39 pm

The test creative is:

<div id="container">
    <style type="text/css">
        #container {
          background-color: #EEF;
          width: 500px;
          height: 500px;
        }
    </style>
    <span>
        <strong>$sf.ext.register and $sf.ext.geom</strong>
        <br>
        Geometry info should be updated on scroll/browser resize.
    </span>
    <pre id="geometry"></pre>
    <script>
      function updateGeometry() {
        var el = document.getElementById('geometry');
        var geom = $sf.ext.geom();
        el.innerHTML = JSON.stringify(geom, null, '  ');
      }

      $sf.ext.register(500, 500, function(status, data) {
        if (status === 'geom-update') {
          updateGeometry();
        }
      });

      updateGeometry();
    </script>
</div>

It looks like this is all working correctly?

jeffkaufman commented 5 years ago

Testing the two referenced urls I'm not seeing this either. No reference error, and the ads look good.

jeffkaufman commented 5 years ago

@ndahmani

github-nourdine-dahmani commented 5 years ago

Hi @jeffkaufman,

Thanks a lot for your investigation, much appreciated. However, I'm not sure you checked the right ad in the URL I provided.

Seeing your test page, you rightly use $sf to get geom data.

In the page I provided, $sf is not even defined into the safeframe scope. Which is my problem, how $sf could not be defined even though the publisher used data-force-safeframe. Our framework needs geom data in order to expand the container when we have an ad to display (and before the user reaches it).

Please, could you have a second look here (mobile): https://kurier---at---doh-342-safeframe-4-k32e7yy-rltdq4mpxfdic.eu.platform.sh/amp/kultur/wanda-live-in-der-arena-himmlisches-ueberraschungskonzert/400351735?google_preview=yWBpxYCfNcEYgNDI4AUwgOz95wWIAYCAgKDb--D_pQE&iu=39025507&gdfp_req=1&lineItemId=4733082881&creativeId=138252023189

Looking for data-slot = "/39025507/amp_banner2" you'll find the amp-ad I'm talking about. You'll see that $sf is not defined (we have many other example if needed):

capture d ecran 2018-12-17 a 16 25 35

Many thanks,

jeffkaufman commented 5 years ago

@ndahmani Thanks for the follow-up!

When I visit https://kurier---at---doh-342-safeframe-4-k32e7yy-rltdq4mpxfdic.eu.platform.sh/amp/kultur/wanda-live-in-der-arena-himmlisches-ueberraschungskonzert/400351735?google_preview=yWBpxYCfNcEYgNDI4AUwgOz95wWIAYCAgKDb--D_pQE&iu=39025507&gdfp_req=1&lineItemId=4733082881&creativeId=138252023189 on mobile I don't see any $sf warning on the console. Does this consistently fire for you?

github-nourdine-dahmani commented 5 years ago

@jeffkaufman I apologize, I forgot to mention, the warning screened above got after manually access $sf into the console. For testing purpose, I usually access $sf this way. I only have to be under the safeframe scope.

However, our ad script uses the same property to test the safeframe context. If I'm correct, this command should not fire any warning.

Thanks a lot for checking!

jeffkaufman commented 5 years ago

Looking on your test page, I don't see ext.js getting loaded, which does look wrong.

github-nourdine-dahmani commented 5 years ago

Thanks for raising this up @jeffkaufman I don't understand why sometimes ext.js is loaded and sometimes not I don't think the publisher is responsible for loading this library since he only set a DoubleClick amp-ad element.

However, even with ext.js loaded $sf still not defined under the safeframe scope.

capture d ecran 2018-12-17 a 18 33 36
olivier-gnagora commented 5 years ago

Hi all, any update on this issue? Is there none workaround we can use? thanks.

olivier-gnagora commented 5 years ago

Now that I can confirm that our simple solution for this problem is working I want to share this in case someone else has the same troubles.

For your information. We use DoubleClick as Ad Server.

Problem: $sf is not defined error message within the Google safeFrame even though “data-force-safeframe=true” is set

Solution (which worked for us): We added the ext.js script (which is necessary for the $sf functions) in our creative code snippet. Maybe not the best solution since we load the same script 2 times but it worked for us.

screen-ext-js-code-snippet

calebcordry commented 5 years ago

Closing this as it seems the original reporter has fixed? Feel free to reopen.

jeffkaufman commented 4 years ago

Sorry for dropping this!

The workaround described above of manually modifying the creative to include ext.js should work as a temporary fix, but over time could have problems as the version of ext.js gets out of sync with the version of the host.

I think the root problem is that Google AdManager is not injecting ext.js into the creative when serving to safeframes on AMP pages the way it does when it serves to safeframes on non-AMP pages. It's not an AMP problem, but an AdManager issue. I'll look at fixing it.

jeffkaufman commented 4 years ago

Sorry for the long wait, but this should be fixed now: AdManager should injects ext.js when serving to SafeFrame on AMP pages.