YahooArchive / boomerang

End user oriented web performance testing and beaconing
http://lognormal.github.com/boomerang/doc/
Other
1.18k stars 448 forks source link

Unload code not called on iPhone/iPad #43

Open solarice opened 12 years ago

solarice commented 12 years ago

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.

solarice commented 12 years ago

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;
                    }
bluesmoon commented 12 years ago

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?

solarice commented 12 years ago

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.

bluesmoon commented 12 years ago

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

solarice commented 12 years ago

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.