From 7a271416c199ac20fd42430442d84a7002e5d1aa Mon Sep 17 00:00:00 2001 From: Philip Tellis Date: Tue, 12 Jun 2012 13:59:06 -0400 Subject: [PATCH 1/2] Use pageshow/pagehide instead of load/unload Browsers that use a page cache may not always fire onunload when a user navigates away from a page but doesn't close the window. They fire the pagehide event instead, and the pageshow event when the user returns to the page. It's better for us to use these events on browsers that support it. pageshow/pagehide also fire when onload/unload would normally have fired. Thanks to github user @solstice for the info in issue 43: https://github.com/yahoo/boomerang/issues/43 and the Surfin Safari blog: http://www.webkit.org/blog/516/webkit-page-cache-ii-the-unload-event/ --- boomerang.js | 46 +++++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/boomerang.js b/boomerang.js index af103b75e..b403b05ce 100644 --- a/boomerang.js +++ b/boomerang.js @@ -249,11 +249,15 @@ boomr = { // The developer can override onload by setting autorun to false if(!("autorun" in config) || config.autorun !== false) { - impl.addListener(w, "load", - function() { - impl.fireEvent("page_ready"); - } - ); + var load_handler = function() { + impl.fireEvent("page_ready"); + }; + if("onpagehide" in w) { + impl.addListener(w, "pageshow", load_handler); + } + else { + impl.addListener(w, "load", load_handler); + } } impl.addListener(w, "DOMContentLoaded", function() { impl.fireEvent("dom_loaded"); }); @@ -270,8 +274,13 @@ boomr = { else if(d.visibilityState) impl.addListener(d, "visibilitychange", fire_visible); - // This must be the last one to fire - impl.addListener(w, "unload", function() { w=null; }); + if(!("onpagehide" in w)) { + // This must be the last one to fire + // We only clear w on browsers that don't support onpagehide because + // those that do are new enough to not have memory leak problems of + // some older browsers + impl.addListener(w, "unload", function() { w=null; }); + } return this; }, @@ -308,22 +317,21 @@ boomr = { // support it. This allows us to fall back to onunload when onbeforeunload // isn't implemented if(e_name === 'page_unload') { - impl.addListener(w, "unload", - function() { - if(fn) { - fn.call(cb_scope, null, cb_data); - } - fn=cb_scope=cb_data=null; - } - ); - impl.addListener(w, "beforeunload", - function() { + var unload_handler = function() { if(fn) { fn.call(cb_scope, null, cb_data); } fn=cb_scope=cb_data=null; - } - ); + }; + // pagehide is for iOS devices + // see http://www.webkit.org/blog/516/webkit-page-cache-ii-the-unload-event/ + if("onpagehide" in w) { + impl.addListener(w, "pagehide", unload_handler); + } + else { + impl.addListener(w, "unload", unload_handler); + impl.addListener(w, "beforeunload", unload_handler); + } } return this; From ea09ed9dc6cb2c707eebd372f405339ae68a965d Mon Sep 17 00:00:00 2001 From: Philip Tellis Date: Tue, 12 Jun 2012 14:14:43 -0400 Subject: [PATCH 2/2] Use BOOMR.page_ready instead of new handler Our load handler does exactly what BOOMR.page_ready does, so just call BOOMR.page_ready. BOOMR.page_ready never refers to `this`, so we don't have a problem with bad dereferencing. --- boomerang.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/boomerang.js b/boomerang.js index b403b05ce..6d01a71b6 100644 --- a/boomerang.js +++ b/boomerang.js @@ -249,14 +249,11 @@ boomr = { // The developer can override onload by setting autorun to false if(!("autorun" in config) || config.autorun !== false) { - var load_handler = function() { - impl.fireEvent("page_ready"); - }; if("onpagehide" in w) { - impl.addListener(w, "pageshow", load_handler); + impl.addListener(w, "pageshow", BOOMR.page_ready); } else { - impl.addListener(w, "load", load_handler); + impl.addListener(w, "load", BOOMR.page_ready); } }