175231 - Add back the page load time to the status bar

Status: RESOLVED WONTFIX

(SeaMonkey :: UI Design, enhancement)

Description

[Russell Odom]

Per bug 48436 comment 46, I'd like the option to turn this display back on! We
use it a lot (at work) for just getting *approximate* figures for the
performance of our web pages, and really missed it when it went.

I do agree that it shouldn't necessarily be on by default (it's not very useful
to the majority of users), so perhaps allow an entry in user.js to turn it back
on (doesn't need a preference in the UI - technical users who are likely to use
it should be able to work it out).

As an aside, if there is concern that the value that was available before is
inaccurate, display to fewer decimal places is fine for me - no need to
implement some complex timing code, just use the old stuff.

Comment 1

[Ben Howe]

I second the motion.

Comment 2

[Doug Turner (:dougt)]

we can offer that support if it was important to the front end UI.

Comment 3

[Byron]

The option to turn this feature on and off would be nice. I tend to agree that
it can be somewhat useful since it also can give a user an idea how their
internet connection is working too. Netscape 6.2.3(Gecko/20020508) has this
feature. Unless the code for this function creates a major problem then it
should be added either as an option or permenant part of the browser. Accuracy
aside it does provide some help for web page developers. No need for a fine
tuned number just 3 digits(i.e 12.5secs or .356 secs, whatever speed your
internet connection is) would work. Also seeing the word "Done" seems so
mundaine for a browser of this quality.

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

to xpapps: gui fetures.

Comment 5

[andrej koelewijn]

Yes, can we please have the time display back. There are developers here where i
work, that only use mozilla instead of msie because it displays the page load
time. And they refuse to upgrade to version 1.2 because it has been removed.

Comment 6

[Andreas Liebig]

Same here: I switched back to 1.1 because of the missing load time. 
I don't care if it is not 100% accurate: a ballpark figure measured by the
browser is still better than using a stop watch.
I agree that it shouldn't necessarily be on by default, but there should be some
way to turn it back on (editing *.js is ok with me)

Comment 7

[Andrew Hagen]

Since this feature has already been implemented, adding it back in shouldn't be
difficult. As long the feature is only accessible through editing user.js, it
shouldn't create any bloat.

Comment 8

[Andrea Aime]

I've just been asked by some colleagues of mine
why the page load time has disapperead... they're
not happy about the removal... An option to enable
it again would be welcomed :-)

Comment 9

[Jo Hermans]

*** Bug 184667 has been marked as a duplicate of this bug. ***

Comment 10

[John P. Bindel]

I won't upgrade without it.

Comment 11

[bitlogic]

I just upgraded to version 1.2.1-9 and I was really suprised that It didn't show
the page load time information.
I'm a web developer and I used a lot to see if my developed pages take to long
or not. 
I don't care if it's really accurate or not. If I have some time information,
it's more easy to identify which pages are slow and what's the time that they
take to load.
I'll like to see an option to enable time load.

Comment 12

[[e-mail]]

*** This bug has been marked as a duplicate of 88982 ***

Comment 13

[Andreas Liebig]

Shawn, I don't think this is a duplicate of 88982. This is about diplaying the
load time AFTER loading has finished; 88982 is about displaying information
while loading. 

Comment 14

[Per Ångström]

My conclusion is exactly the same as that of Andreas: it's not a duplicate.
Reopening.

Comment 15

[dbkeon]

Please, please make this an optional feature instead of just taking it away!  I
look at the download times frequently when doing development.  This is one of
those nice little extras that make Mozilla so great.  I recently upgraded from
1.1 to 1.3a and couldn't believe it was gone.  I don't care that it's not
perfectly accurate.  As others have written, it at least allows one to make a
good quick baseline comparison.  If it's not made into an option I'll go back to
1.1.

Thanks.

Comment 16

[NorthMan]

I really missed this feature, although it did take me a while to notice it was
gone.  Now that it has been gone a while, I have began missing it.  If we are
going to go ahead and put it back in, it should have a UI pref, probably under
Advanced or sommat.

Comment 17

[Sander]

No it shouldn't. Such a pref would serve no purpose whatsoever. Are there people
who so badly _don't_ want to see estimates for pageload time that they'd turn
the pref off? Enough of them for it to be worth the added work to continuously
test that both states of the pref will keep working? And to give up valuable
space in the already overloaded pref panels?

Comment 18

[Eamonn Maher]

I too would like to say that it is an accassionally useful feature to be able to
see the download times and would like to see the feature it re-instated.

Comment 19

[Ben Howe]

Attachment: Adds page time back to status bar in browser

I don't expect that this patch would be incorporated into Mozilla just yet (per
bug 48436), but for anybody that wants it, here it is.	Incorporating the patch
does not require a recompile because the files are jar'ed in the chrome
directory.  nsBrowserStatusHandler.js is in comm.jar/content/navigator and
navigator.properties is in en-US.jar/locale/en-US/navigator.

Perhaps somebody who knows a bit more about coding with advanced preferences
can switch this feature on and off via a browser.something.something
preference.  If that gets done, then we might have a chance at getting it
incorporated back into the trunk.

Comment 20

[Ben Howe]

Attachment: Adds page load time to browser status bar with advanced pref

This patch adds the advanced preference "browser.show_load_time".  When set to
true, then the page load time is shown in the status bar.  When false
(default), it just displays "Done".  As this is my first patch, I don't know
how to get it into the trunk, so somebody will have to help me out with that.

Comment 21

[Russell Odom]

Suggestion on this patch:

Instead of adding the "(%elapsed% secs)" to nv_done in navigator.properties,
then removing it later if not required, instead create a new value (e.g.
nv_elapsed) that is appened to msg if the pref says so, i.e.
nv_elapsed= (%elapsed% secs)

That way, the code in nsBrowserStatusHandler.js is independent of the value of
nv_done, and your code won't break if somebody changes it.

Also, maybe format the time so it's always to e.g. 3 decimal places.

Comment 22

[Ben Howe]

Thanks for the suggestions about the patch.  I agree with your comments. 
However, in order to format the time to 3 decimal points regardless of trailing
zeros would require an ugly code hack since JavaScript (to my knowledge) doesn't
have a flexible float to string conversion (like sprintf).  Would it still be
worth it to add in 2 extra JavaScript multiplications?  I know that's not much,
but if we are trying to reduce bloat as much as possible, it would not be worth
it IMO.  Here's the best way to do it that I can think of:

msg += " (" + Math.floor(elapsed) + "." + elapsed*1000-1000*Math.floor(elapsed)
+ " secs)";

There might be a better way to do it, but I don't know - I am very new to
JavaScript.

Comment 23

[Ben Howe]

Attachment: Patch with better implementation

I have incorporated the changes suggested in comment #21, but I left it such
that the page load time is displayed in 3 digits always.  For example, if the
load time is 1.140 seconds, "Done (1.14 secs)" will be displayed.  Any way we
can get this into 1.5 alpha?

Comment 24

[Ben Howe]

Sorry, last comment should say that page load time is NOT always displayed in 3
digits.

Comment 25

[Russell Odom]

+            // Record page loading time (in milliseconds).

Minor niggle - this is actually *calculating* the load time not recording it.

+            msg += " " + gNavigatorBundle.getString("nv_elapsed");

Put the space on the start of nv_elapsed rather than adding it here. This means
that the *whole* message is in navigator.properties, e.g. if we wanted
nv_elapsed changing to ", in NN seconds".

A thought about the 3 decimal place thing: convert the value to a string and
append trailing 0s, then trim it off 3 chars after the ".". Watch for the case
where the time is an exact number of seconds though; it won't have a trailing
decimal point in this case. Not sure if it'd be worth the effort!

Comment 26

[Ben Howe]

> Minor niggle - this is actually *calculating* the load time not recording it.

I think it is recording it - it records it into a variable.  The milliseconds
thing is wrong, though, and will be changed.

> A thought about the 3 decimal place thing: convert the value to a string and
> append trailing 0s, then trim it off 3 chars after the ".". Watch for the case
> where the time is an exact number of seconds though; it won't have a trailing
> decimal point in this case. Not sure if it'd be worth the effort!

Doesn't seem worth it to me.  I know that 2 secs = 2.000 secs :)  Any objections
from anybody?

> Put the space on the start of nv_elapsed rather than adding it here. This
> means that the *whole* message is in navigator.properties, e.g. if we wanted
> nv_elapsed changing to ", in NN seconds".

I put it as is because we could relocate it later on if we wanted (like to a
different place in the status bar (like right justified or something)), but now
that I think about it, that probably wouldn't happen, so I will change it as
requested.

Comment 27

[Ben Howe]

nv_elapsed cannot begin with a space.  The space is ignored and the status bar
ends up looking like this: "Done(0.586 secs)".  I think that it is OK to just
set nv_elapsed=(%elapsed% secs).  That way, the load time display is more
independent of where it is used.  Perhaps it *should* be up to the calling
JavaScript code to place the strings where appropriate and pad with spaces if
necessary.

Comment 28

[Ben Howe]

Attachment: Changed "millisecond" comment to be correct

This should get the job done.

Comment 29

[Ben Howe]

Well, I've tried to get this patch reviewed by the proper people, but nobody
seems interested in reviewing it, so it looks like this bug is going to die a
slow death unless somebody else wants to look more into getting this patch into
the trunk.  I've spent more time now looking for reviewers than actually writing
the patch, so I give up :(  It's a shame this is such a pain becuase it is a
simple patch!

Comment 30

[Alec Flett]

Comment on attachment 127014 [details] [diff] [review]
Changed "millisecond" comment  to be correct

did anyone measure the performance impact of this change? makes me wonder if
Date.now() ends up making a system call, which would slow down pageloading perf

Comment 31

[Ben Howe]

> makes me wonder if
> Date.now() ends up making a system call, which would slow down pageloading perf

Assuming it did, which would be better?  Just calling Date.now(), or doing a
Mozilla pref lookup and then calling Date.now (if necessary)?

Comment 32

[jag (Peter Annema)]

This isn't localizable. The correct way is to have two "done" messages:

 nv_done=Done
+nv_done_elapsed=Done (%S secs)

and then use one or the other. Also, instead of manually replacing %elapsed% use
gNavigatorBundle.getFormattedString.

Note that this might affect page load times.

Also, see bug 88982 where it's suggested in addition to time a few other metrics
are added.

Comment 33

[Alec Flett]

Comment on attachment 127014 [details] [diff] [review]
Changed "millisecond" comment  to be correct

*sigh* 
PLEASE use stringbundle.formatStringFromName()	- its the preferred method for
the translators.

Comment 34

[R.K.Aa.]

*** Bug 238630 has been marked as a duplicate of this bug. ***

Comment 35

[Russell Odom]

No activity on this bug for a while! Still missing the feature; anyone out there
want to pick this up?

Comment 36

[zug_treno]

*** Bug 334621 has been marked as a duplicate of this bug. ***

Comment 37

[Russell Odom]

I think with extensions such as LORI and FasterFox, the need for this has gone away.

Comment 38

[hhaamu]

(In reply to comment #37)
> I think with extensions such as LORI and FasterFox, the need for this has gone
> away.

Just reminding you that neither of those extensions work with the suite, against which this bug is still filed.