Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Unload code not called on iPhone/iPad #43

Open
solarice opened this Issue May 14, 2012 · 5 comments

Comments

Projects
None yet
2 participants

Unload code is not called on iPhone/iPad and there is no navigation timing API currently supported.

IPad/Iphone uses the “pagehide” event instead of “unload” event.

http://www.webkit.org/blog/516/webkit-page-cache-ii-the-unload-event/

I tested this and it works. I'm new to git, but I'll try to send the code updates later today.

Still figuring out GIT. Somehow my branch is not 'public' so I cannot see my local checkins on this web-site in order to issue a pull request...?

Here's the code I added to boomerang.js. "pagehide" for iPhone/iPad and "onunload" for blackberry....

        impl.addListener(w, "pagehide",
                    function() {
                        if(fn) {
                            fn.call(cb_scope, null, cb_data);
                        }
                        fn=cb_scope=cb_data=null;
                    }
                );
        impl.addListener(w, "onunload",
                    function() {
                        if(fn) {
                            fn.call(cb_scope, null, cb_data);
                        }
                        fn=cb_scope=cb_data=null;
                    }
Contributor

bluesmoon commented Jun 12, 2012

Hi @solarice thanks for the patch. We've moved development to https://github.com/lognormal/boomerang/

I'll merge your changes in there and give you credit.

Regarding the change you made for BlackBerry, don't we already listen to the unload event? Or does BlackBerry do something weird with event names?

Yes, sort of. Exiting code had "unload" but blackberry uses "onunload".

    // Add "pagehide" for support on iPad/iPhone
    // Add "onunload" for Blackberry
    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() {
                        if(fn) {
                            fn.call(cb_scope, null, cb_data);
                        }
                        fn=cb_scope=cb_data=null;
                    }
                );
        impl.addListener(w, "pagehide",
                    function() {
                        if(fn) {
                            fn.call(cb_scope, null, cb_data);
                        }
                        fn=cb_scope=cb_data=null;
                    }
                );
        impl.addListener(w, "onunload",
                    function() {
                        if(fn) {
                            fn.call(cb_scope, null, cb_data);
                        }
                        fn=cb_scope=cb_data=null;
                    }
                );
    }

The impl.addListener method adds the "on" prefix when calling "attachEvent", but that's not getting used....

addListener: function(el, sType, fn, capture) {
    if(el.addEventListener) {
        el.addEventListener(sType, fn, (capture));
    }
    else if(el.attachEvent) {
        el.attachEvent("on" + sType, fn);
    }
}

http://docs.blackberry.com/en/developers/deliverables/11848/HTML_ref_event_attributes_600908_11.jsp

P.S. Thanks for adding in the iPhone/iPad portion. I'll try syncing up again as there's other fixes/enhancements I'd like to add.

@solarice solarice closed this Jun 12, 2012

Contributor

bluesmoon commented Jun 12, 2012

wouldn't we also need to do this for the onload event in that case?

@solarice solarice reopened this Jun 12, 2012

That's a good question. We were getting data in our tests and they seemed sufficiently accurate. I'll investigate some more to understand why it was working.

@bluesmoon bluesmoon added a commit to bluesmoon/boomerang that referenced this issue Apr 2, 2015

@bluesmoon bluesmoon 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:
yahoo#43
and the Surfin Safari blog:
http://www.webkit.org/blog/516/webkit-page-cache-ii-the-unload-event/
a463afe

@bluesmoon bluesmoon added a commit to SOASTA/boomerang that referenced this issue Aug 8, 2015

@bluesmoon bluesmoon 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:
yahoo#43
and the Surfin Safari blog:
http://www.webkit.org/blog/516/webkit-page-cache-ii-the-unload-event/
d579368

@bluesmoon bluesmoon pushed a commit to bluesmoon/boomerang that referenced this issue Mar 8, 2017

@nicjansma nicjansma Merge pull request #43 from SOASTA/restiming-noscript
Change optimized trie to avoid string that trigger a XSS warning from NoScript
4b0c1e1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment