Last Comment Bug 748276 - Navigation Timing treats javascript: loads as starting a new navigation, which affects all timing for the next navigation in the browsing context
: Navigation Timing treats javascript: loads as starting a new navigation, whic...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Document Navigation (show other bugs)
: 11 Branch
: x86 Windows XP
: -- normal (vote)
: mozilla15
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-24 01:41 PDT by peter.de.keer
Modified: 2012-07-03 03:39 PDT (History)
5 users (show)
bzbarsky: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Don't record a navigationStart for javascript: URI loads that don't produce a document. (1.67 KB, patch)
2012-05-22 21:28 PDT, Boris Zbarsky [:bz] (still a bit busy)
bugs: review+
Details | Diff | Splinter Review

Description peter.de.keer 2012-04-24 01:41:04 PDT
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 2012-04-24 04:44:33 PDT
Error in my description

Expected results:

*Time* spent on the page should not be added to Navigation Timing.
Comment 2 peter.de.keer 2012-05-03 12:03:43 PDT
created video to try to explain the issue better!
http://www.youtube.com/watch?v=1dvZA1ngAc4
Comment 3 peter.de.keer 2012-05-12 12:18:51 PDT
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 [:bz] (still a bit busy) 2012-05-22 21:15:59 PDT
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 [:bz] (still a bit busy) 2012-05-22 21:18:38 PDT
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 [:bz] (still a bit busy) 2012-05-22 21:20:14 PDT
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 [:bz] (still a bit busy) 2012-05-22 21:28:21 PDT
Created attachment 626317 [details] [diff] [review]
Don't record a navigationStart for javascript: URI loads that don't produce a document.
Comment 8 Boris Zbarsky [:bz] (still a bit busy) 2012-05-23 09:47:24 PDT
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] 2012-05-24 10:46:46 PDT
https://hg.mozilla.org/mozilla-central/rev/7e85b068db34
Comment 10 peter.de.keer 2012-06-05 01:17:21 PDT
Is there a browser version of firefox in wich we can test the solution?
Nightly? Beta?
Comment 11 Boris Zbarsky [:bz] (still a bit busy) 2012-06-05 09:03:06 PDT
Any nightly, for now.  In a few days, also Aurora.
Comment 12 peter.de.keer 2012-07-03 03:39:27 PDT
OK, thanks, I tested it and it works great!
Now I discovered a new bug!
https://bugzilla.mozilla.org/show_bug.cgi?id=770463

Note You need to log in before you can comment on or make changes to this bug.