748276 - Navigation Timing treats javascript: loads as starting a new navigation, which affects all timing for the next navigation in the browsing context

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[peter.de.keer]

User Agent: Mozilla/5.0 (Windows NT 5.1) AppleWebKit/536.5 (KHTML, like Gecko) Chrome/19.0.1084.30 Safari/536.5

Steps to reproduce:

Install bookmarktlet from http://kaaes.github.com/timing/
Go to webpage (to keep it simple) http://www.mozilla.org/robots.txt
Click on bookmarktlet => all is good!
Wait 5 seconds on page and reload (CTRL+F5, ENTER in url bar)
Click on bookmarktlet => wait time on the page is added to Navigation Timing (fetchStart)



Expected results:

Page spent on the page should not be added to Navigation Timing
See behaviour in Google Chrome.

These numbers are sent to Google Analytics, and it is there that I viewed strange numbers first. (https://groups.google.com/a/googleproductforums.com/d/topic/analytics/IdxoGvoSDzM/discussion)

Comment 1

[peter.de.keer]

Error in my description

Expected results:

*Time* spent on the page should not be added to Navigation Timing.

Comment 2

[peter.de.keer]

created video to try to explain the issue better!
http://www.youtube.com/watch?v=1dvZA1ngAc4

Comment 3

[peter.de.keer]

The developer of the 'bookmarklet' confirmed it's a bug in firefox and not in her code!
https://github.com/kaaes/timing/issues/1
https://skitch.com/kaaes/83aq6/dock

Comment 4

[Boris Zbarsky [:bzbarsky]]

All that's happening here is that the time when the javascript: load starts is being counted as the navigationStart.

Olli, Igor, how is this stuff supposed to work, exactly?  It looks to me like we're not tracking on the docshell's timing object which channel its doing timing for, which we probably should...

Comment 5

[Boris Zbarsky [:bzbarsky]]

Looking at nsDocShell::InternalLoad, a similar issue would arise if a beforeunload handler disallowed leaving the page.  We'd record the navigationStart at the point before running that handler, and then if we managed to leave the page sometime use _that_ navigationStart...

I had been going to move the MaybeInitTiming call to the point where we create the channel for the navigation (and thus know whether it's a background channel or not, which would fix the javascript: case), but the beforeunload thing makes that ... complicated.

I think I can still fix the javascript: issue, but the beforeunload part will still be broken.

Comment 6

[Boris Zbarsky [:bzbarsky]]

But I still think the right design for the timing stuff would have it know which exact channel it's doing timing for....

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Don't record a navigationStart for javascript: URI loads that don't produce a document.

Comment 8

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7e85b068db34

This could really use a test, but testing timing stuff is a PITA.  :(

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7e85b068db34

Comment 10

[peter.de.keer]

Is there a browser version of firefox in wich we can test the solution?
Nightly? Beta?

Comment 11

[Boris Zbarsky [:bzbarsky]]

Any nightly, for now.  In a few days, also Aurora.

Comment 12

[peter.de.keer]

OK, thanks, I tested it and it works great!
Now I discovered a new bug!
https://bugzilla.mozilla.org/show_bug.cgi?id=770463