Adsense won't load with turbolinks #151

Closed
chengguangnan opened this Issue Dec 29, 2012 · 18 comments

Comments

Projects
None yet
8 participants
@chengguangnan

Google adsense won't appear if you get to the page through a turbolink.

I have no idea but perhaps it's due to the dom onload event?

JFYI.

@davydotcom

This comment has been minimized.

Show comment
Hide comment
@davydotcom

davydotcom Jan 11, 2013

Contributor

Can you paste a sample of the adsense code?

Contributor

davydotcom commented Jan 11, 2013

Can you paste a sample of the adsense code?

@chengguangnan

This comment has been minimized.

Show comment
Hide comment
@chengguangnan

chengguangnan Jan 14, 2013

I've setup an rails app that demonstrate this. I will put the link here later, maybe tomorrow.

I've setup an rails app that demonstrate this. I will put the link here later, maybe tomorrow.

@chengguangnan

This comment has been minimized.

Show comment
Hide comment
@chengguangnan

chengguangnan Jan 16, 2013

@davydotcom , check out this.

You may not see any ads because your domain name isn't allowed, but you can see a blank box.

Now click to anther page, the box is gone, although the new page has adsense. The expected result is a blank box always be there.

Screen Shot 2013-01-16 at 8 36 02 PM

@davydotcom , check out this.

You may not see any ads because your domain name isn't allowed, but you can see a blank box.

Now click to anther page, the box is gone, although the new page has adsense. The expected result is a blank box always be there.

Screen Shot 2013-01-16 at 8 36 02 PM

@caarlos0

This comment has been minimized.

Show comment
Hide comment
@caarlos0

caarlos0 Feb 12, 2013

Same issue here.

Same issue here.

@reed

This comment has been minimized.

Show comment
Hide comment
@reed

reed Feb 14, 2013

Collaborator

I found this article:

http://www.redkart.com/google_adsense.html

...and used the basic premise to come up with this solution, which appears to work on the sample app you provided:

  1. Add the following script tag to the head, above the application.js file:

    <script src="//www.google.com/jsapi"></script>
  2. Define these classes somewhere in your application's JS:

    class Adsense
      constructor: (@ad_client) ->
        if google?
          google.load 'ads', '1'
          google.setOnLoadCallback @initPage
          @ads = {}
          $(document).on 'page:fetch', =>
            @clearAds()
          $(document).on 'page:load', =>
            @initPage()
    
      initPage: =>
        ad.load() for id, ad of @ads
    
      clearAds: ->
        @ads = {}
    
      newAd: (container, options) ->
        id = options.format + '_' + container.id
        @ads[id] = new Ad @, id, container, options
    class Ad
      constructor: (@adsense, @id, @container, @options) ->
    
      load: ->
        if @ad_object? then @refresh() else @create()
    
      refresh: ->
        @ad_object.refresh()
    
      create: ->
        @ad_object = new google.ads.Ad @adsense.ad_client, @container, @options
  3. Add this line to your application's JS to initialize adsense

    window.MyAdsense = new Adsense "ca-pub-4379359399658390" # your google_ad_client id
  4. Create a container with an ID for each ad on the page

    <body>
      <%= yield %>
      <div id="my_ad"></div>
    </body>
  5. Add a script tag to the body of the page that will initialize the ad:

    <script>
    window.MyAdsense.newAd(document.getElementById('my_ad'), {
      'format': google.ads.LEADERBOARD,
      'ad_slot': "4746598348",
      'ad_width': 300,
      'ad_height': 250
    });
    </script>

And you should be set.

@guangnan @caarlos0 Can you guys try this out and see if it works?

Collaborator

reed commented Feb 14, 2013

I found this article:

http://www.redkart.com/google_adsense.html

...and used the basic premise to come up with this solution, which appears to work on the sample app you provided:

  1. Add the following script tag to the head, above the application.js file:

    <script src="//www.google.com/jsapi"></script>
  2. Define these classes somewhere in your application's JS:

    class Adsense
      constructor: (@ad_client) ->
        if google?
          google.load 'ads', '1'
          google.setOnLoadCallback @initPage
          @ads = {}
          $(document).on 'page:fetch', =>
            @clearAds()
          $(document).on 'page:load', =>
            @initPage()
    
      initPage: =>
        ad.load() for id, ad of @ads
    
      clearAds: ->
        @ads = {}
    
      newAd: (container, options) ->
        id = options.format + '_' + container.id
        @ads[id] = new Ad @, id, container, options
    class Ad
      constructor: (@adsense, @id, @container, @options) ->
    
      load: ->
        if @ad_object? then @refresh() else @create()
    
      refresh: ->
        @ad_object.refresh()
    
      create: ->
        @ad_object = new google.ads.Ad @adsense.ad_client, @container, @options
  3. Add this line to your application's JS to initialize adsense

    window.MyAdsense = new Adsense "ca-pub-4379359399658390" # your google_ad_client id
  4. Create a container with an ID for each ad on the page

    <body>
      <%= yield %>
      <div id="my_ad"></div>
    </body>
  5. Add a script tag to the body of the page that will initialize the ad:

    <script>
    window.MyAdsense.newAd(document.getElementById('my_ad'), {
      'format': google.ads.LEADERBOARD,
      'ad_slot': "4746598348",
      'ad_width': 300,
      'ad_height': 250
    });
    </script>

And you should be set.

@guangnan @caarlos0 Can you guys try this out and see if it works?

@chengguangnan

This comment has been minimized.

Show comment
Hide comment
@chengguangnan

chengguangnan Feb 27, 2013

Hi @reed ,

I removed the google.ads.LEADERBOARD line and it worked. Thank you.

My changes:

-   <script src="//www.google.com/jsapi"></script>
+   <script src="https://www.google.com/jsapi"></script>
...
-    id = options.format + '_' + container.id
+    id = 'ads_' + container.id
...
-  'format': google.ads.LEADERBOARD,

Hi @reed ,

I removed the google.ads.LEADERBOARD line and it worked. Thank you.

My changes:

-   <script src="//www.google.com/jsapi"></script>
+   <script src="https://www.google.com/jsapi"></script>
...
-    id = options.format + '_' + container.id
+    id = 'ads_' + container.id
...
-  'format': google.ads.LEADERBOARD,
@reed

This comment has been minimized.

Show comment
Hide comment
@reed

reed Feb 27, 2013

Collaborator

Cool, thanks for the feedback. I'm glad it worked for you.

Collaborator

reed commented Feb 27, 2013

Cool, thanks for the feedback. I'm glad it worked for you.

@reed reed closed this Feb 28, 2013

@caarlos0

This comment has been minimized.

Show comment
Hide comment
@caarlos0

caarlos0 Apr 5, 2013

@reed your solution mixed with @guangnan tips worked here =D

Thank you both.

caarlos0 commented Apr 5, 2013

@reed your solution mixed with @guangnan tips worked here =D

Thank you both.

@chengguangnan

This comment has been minimized.

Show comment
Hide comment
@chengguangnan

chengguangnan May 28, 2013

Hi guys, further updates are moved to here.

Hi guys, further updates are moved to here.

@AstonJ

This comment has been minimized.

Show comment
Hide comment
@AstonJ

AstonJ Aug 10, 2013

Hi @reed @guangnan

Thanks for this solution! However there is one drawback, it adds 3 render-blocking javascript files which can slow the site down. I go from a pagespeed score of 93 to 84 when I add adsense via this method.

What about doing something as simple as this? (There would be no need to add the script tag to the head then - which is what causes the slow down.)

adOne = ''
#// Adsense
$(document).on 'page:fetch', ->
  adOne = $('#ad_one').html()

$(document).on 'page:change', ->
   $('#ad_one').html(adOne)

Where you have a div with and id of 'ad_one' that contains the adsense code.

Any thoughts on this? I'm a JS noob so I might be missing something.

AstonJ commented Aug 10, 2013

Hi @reed @guangnan

Thanks for this solution! However there is one drawback, it adds 3 render-blocking javascript files which can slow the site down. I go from a pagespeed score of 93 to 84 when I add adsense via this method.

What about doing something as simple as this? (There would be no need to add the script tag to the head then - which is what causes the slow down.)

adOne = ''
#// Adsense
$(document).on 'page:fetch', ->
  adOne = $('#ad_one').html()

$(document).on 'page:change', ->
   $('#ad_one').html(adOne)

Where you have a div with and id of 'ad_one' that contains the adsense code.

Any thoughts on this? I'm a JS noob so I might be missing something.

@leods92

This comment has been minimized.

Show comment
Hide comment
@leods92

leods92 Oct 14, 2014

I think I've developed a good solution for the newest adsbygoogle.js script.
Feel free to refine it.
Basically I'm reseting all global variables set and used by Google AdSense script.
If one doesn't use this, after some pages being loaded ads will stop being shown. This is a must for websites running AdSense.
As I don't display ads on all my website's pages I opted for loading Google's script at the end of <body> instead of <head>. You might consider putting in <head> if your whole website is covered by it to better take advantage of Turbolinks. Moving to <head> might cause side effects my solution doesn't cover.

MyApp.Turbolinks.GoogleAds = {
  reset: function() {
    if (typeof adsbygoogle !== 'undefined') {
      // It's okay to use forEach since browsers that don't support it
      // don't support Turbolinks anyway. If browser doesn't support Turbolinks,
      // its events (hence this) won't be triggered.
      [
        "adk2_experiment",
        "async_config",
        "correlator",
        "exp_persistent",
        "iframe_oncopy",
        "jobrunner",
        "num_0ad_slots",
        "num_ad_slots",
        "num_reactive_ad_slots",
        "num_sdo_slots",
        "num_slot_to_show",
        "num_slots_by_channel",
        "onload_fired",
        "persistent_language",
        "persistent_state",
        "persistent_state_async",
        "prev_ad_formats_by_region",
        "prev_ad_slotnames_by_region",
        "pstate_expt",
        "pstate_rc_expt",
        "reactive_ads_global_state",
        "top_js_status",
        "unique_id",
        "viewed_host_channels"
      ].forEach(function(val) {
        delete window["google_" + val];
      });

      delete window.adsbygoogle;
    }
  },

  enable: function() {
    $(document).on("page:fetch", this.reset);
  }
};

MyApp.Turbolinks.GoogleAds.enable();

leods92 commented Oct 14, 2014

I think I've developed a good solution for the newest adsbygoogle.js script.
Feel free to refine it.
Basically I'm reseting all global variables set and used by Google AdSense script.
If one doesn't use this, after some pages being loaded ads will stop being shown. This is a must for websites running AdSense.
As I don't display ads on all my website's pages I opted for loading Google's script at the end of <body> instead of <head>. You might consider putting in <head> if your whole website is covered by it to better take advantage of Turbolinks. Moving to <head> might cause side effects my solution doesn't cover.

MyApp.Turbolinks.GoogleAds = {
  reset: function() {
    if (typeof adsbygoogle !== 'undefined') {
      // It's okay to use forEach since browsers that don't support it
      // don't support Turbolinks anyway. If browser doesn't support Turbolinks,
      // its events (hence this) won't be triggered.
      [
        "adk2_experiment",
        "async_config",
        "correlator",
        "exp_persistent",
        "iframe_oncopy",
        "jobrunner",
        "num_0ad_slots",
        "num_ad_slots",
        "num_reactive_ad_slots",
        "num_sdo_slots",
        "num_slot_to_show",
        "num_slots_by_channel",
        "onload_fired",
        "persistent_language",
        "persistent_state",
        "persistent_state_async",
        "prev_ad_formats_by_region",
        "prev_ad_slotnames_by_region",
        "pstate_expt",
        "pstate_rc_expt",
        "reactive_ads_global_state",
        "top_js_status",
        "unique_id",
        "viewed_host_channels"
      ].forEach(function(val) {
        delete window["google_" + val];
      });

      delete window.adsbygoogle;
    }
  },

  enable: function() {
    $(document).on("page:fetch", this.reset);
  }
};

MyApp.Turbolinks.GoogleAds.enable();
@jpstokes

This comment has been minimized.

Show comment
Hide comment
@jpstokes

jpstokes Oct 23, 2014

@leods92 Is the code you provided the whole solution? When I add it to my site I still don't get the ads to show up consistently. I've inserted it in the head after application.js is loaded.

@leods92 Is the code you provided the whole solution? When I add it to my site I still don't get the ads to show up consistently. I've inserted it in the head after application.js is loaded.

@jpstokes

This comment has been minimized.

Show comment
Hide comment
@jpstokes

jpstokes Oct 23, 2014

Well, looks like I was getting an error says undefined MyApp. I changed "MyApp.Turbolinks.GoogleAds" to "AdsenseFix" and it seems to be working...I guess all the '.' was throwing it off.

Well, looks like I was getting an error says undefined MyApp. I changed "MyApp.Turbolinks.GoogleAds" to "AdsenseFix" and it seems to be working...I guess all the '.' was throwing it off.

@leods92

This comment has been minimized.

Show comment
Hide comment
@leods92

leods92 Oct 23, 2014

@jpstokes It's just a namespace. I like to have a single namespace for the whole app and I also have another one for Turbolinks fixes. To fix your issue you just need to define those namespaces, for instance MyApp = { TurboLinks: {} }

leods92 commented Oct 23, 2014

@jpstokes It's just a namespace. I like to have a single namespace for the whole app and I also have another one for Turbolinks fixes. To fix your issue you just need to define those namespaces, for instance MyApp = { TurboLinks: {} }

@jpstokes

This comment has been minimized.

Show comment
Hide comment
@jpstokes

jpstokes Oct 23, 2014

Cool...thx.

Cool...thx.

@tomasgregor

This comment has been minimized.

Show comment
Hide comment
@tomasgregor

tomasgregor Sep 3, 2015

leods92 - thank you for the fix. To keep code structure clean I placed your code to a file required by application.js which is set download asynchronously and it works as well. Is there any reason to put the code in or ?

leods92 - thank you for the fix. To keep code structure clean I placed your code to a file required by application.js which is set download asynchronously and it works as well. Is there any reason to put the code in or ?

@leods92

This comment has been minimized.

Show comment
Hide comment
@leods92

leods92 Sep 6, 2015

@tomasgregor Sorry for the delay in answering. Could you elaborate? I don't understand your question.

leods92 commented Sep 6, 2015

@tomasgregor Sorry for the delay in answering. Could you elaborate? I don't understand your question.

@tomasgregor

This comment has been minimized.

Show comment
Hide comment
@tomasgregor

tomasgregor Sep 7, 2015

@leods92 I meant you placed your script in the tag and elaborated on placing it in the as another option. In my Rails project I put the code in a separate file turoblinks-settings.js which is required by application.js. My question was whether there's a reason to put the script directly in the body or head tag or it can be uploaded with javascript_inlcude_tag.

@leods92 I meant you placed your script in the tag and elaborated on placing it in the as another option. In my Rails project I put the code in a separate file turoblinks-settings.js which is required by application.js. My question was whether there's a reason to put the script directly in the body or head tag or it can be uploaded with javascript_inlcude_tag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment