47909 - status bar should only have progress meter while loading stuff (hide when not needed, only show when needed)

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, enhancement)

Description

[Henri Sivonen (:hsivonen)]

Meta data display (bug 1995) needs space in the status bar. Meta data display
needs to function fully when a page has been fully loaded. The progess meter is
only needed when the page hasn't been fully loaded. Therefore, reusing the space
taken by the progress meter for meta data display seems like an obvious choice.

Curretly the progess meter is on the left in the status bar. If it appeared and
disappeared there, it would cause the primary status bar text being drawn is
different locations depending on the visibility of the progress meter. That
would make the UI look unstable.

I suggest moving the progress meter to the right in the status bar. That way it
can vanish without causing any repositioning of the primary status bar text.

Comment 1

[Matthew T (active 1999-2002)]

I agree on the first part. The progress meter space should be reused once the 
page is loaded.

But putting it on the right would break the general left-to-right flow of status 
from general to specific. See:
___________________________________________________________
 X [#####():::] 13 kb of 18 kb read, 3 seconds remaining
"|""|""""""""""""|"""""""""""""""""""""""""""""""""""""""""
 |  |            +- status text (specific status)
 |  +-------------- progress meter (general status)
 +----------------- hourglass(/throbber?) (very general status)

Then when the page is loaded, we could still reuse the space:
___________________________________________________________
[] http://bugzilla.mozilla.org/votehelp.html   [English :^]
"|""|"""""""""""""""""""""""""""""""""""""""""""|""""""""""
 |  |                                           +- metadata
 |  +--------------------------------------------- more metadata
 +------------------------------------------------ document validity icon

Is it acceptable to require the user to wait for the page to finish loading (or 
for them to press Stop), before we show metadata? If so, then the progress meter 
should probably stay where it is.

Comment 2

[David Hallowell]

Can we not just have the progress meter similar to Netscape 4.x on the Mac where
the progress meter takes up the entire status bar and lives under the status
text (can't really describe it but ask someone who has a mac if you can take a
look and see what I mean)

Comment 3

[Matthew T (active 1999-2002)]

I don't like Mac 4.x's approach. It makes status text quite difficult to see, 
especially when progress is indeterminate (you're trying to read black text on an 
animated barbershop of light and dark gray).

Comment 4

[Henri Sivonen (:hsivonen)]

Left-to-right flow from general to specific makes some sense, but if the progress meter 
vanishes in that scenario, the position of the metadata display moves and makes the UI 
look unstable.

No, I don't think meta data should only be displayed once the page has fully loaded. If 
meta data is displayed with more general data on the left, a progress meter on the right 
would obscure only less important meta data while the page is loading. After loading the 
progress meter could vanish leaving room to more meta data and at the same time the 
general meta data display wouldn't need to move.

Comment 5

[leger]

Reassigning from bdonohoe to hangas

Comment 6

[Zach Lipton [:zach]]

Chaning the qa contact on these bugs to me. MPT will be moving to the 
owner of this component shortly. I would like to thank him for all his hard 
work as he moves roles in mozilla.org...Yada, Yada, Yada...

Comment 7

[Asa Dotzler [:asa]]

updating to new owner. sorry for the spam.

Comment 8

[Matthew T (active 1999-2002)]

I was wrong, Henri was right. Having the progress bar on the left would make
status text jump around between loading and not-loading states. 

So, the progress bar should stay where it is, but it should disappear completely
(as in {display: none}) when nothing is loading. This will make more space for
status text, and make the browser window less cluttered. --> XP Apps: GUI.

Comment 9

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

taking

Comment 10

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

per comment 8, removing dependency. this bug is no longer about moving the
statusbar, only about hiding/showing it

Comment 11

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

Attachment: patch

Comment 12

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

Attachment: Better patch

Comment 13

[Pierre Chanial]

Comment on attachment 92269 [details] [diff] [review]
Better patch

After applying this patch, the progressmeter does not progress anymore. You can
fix it by removing the hidden="true" attribute in <statusbarpanel>
I have no idea why xul is behaving that way but I have not investigated
further.

In addition, Mail/News also uses the statusbar progress meter. I think you
should apply the same fix for consistency. Could you check all the places where
it is used?

Comment 14

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

>After applying this patch, the progressmeter does not progress anymore. 

Can't reproduce that here. For me, the progressbar behaves the same with or
without the patch (other than that it's hidden if unused with the patch)...

Mailnews: Hm... I'll look into that

Comment 15

[Pierre Chanial]

Now I do see the progress-meter working with a new profile. Boh...
Sorry for the false alarm.

Comment 16

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

Attachment: mailnews part

Comment 17

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

still have to check for other occurences

Comment 18

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

Comment on attachment 93111 [details] [diff] [review]
mailnews part

ah... this includes the fix for another bug... ignore the clearly unrelated
lines :)

Comment 19

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

Attachment: subscribe dialog

Comment 20

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

ok, I think I got them all now. ready for review.

Comment 21

[Pierre Chanial]

Comment on attachment 92269 [details] [diff] [review]
Better patch

r=[e-mail] (new mail address)
I don't feel myself comfortable  to review the Mail/News pathces since I never
touched them

Comment 22

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

sspitzer, could you review the two mailnews patches on this bug?

Comment 23

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

bienvenu, could you review the two mailnews patches on this bug?

Comment 24

[David :Bienvenu]

I'd need to run with this patch to make sure everything is working OK for
mail/news, and I don't have a working tree at the moment. I'll try to get to it
as soon as I can, but it might be a few days.

Comment 25

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

Attachment: mailnews main window part

now rediffed to not include the unrelated changes (which have been checked in
in the meantime)

Comment 26

[(not reading, please use [e-mail] instead)]

Christian, can you attach some screen shots?

cc jglick, as we'll need her approval since this is a UI change.

Comment 27

[(not reading, please use [e-mail] instead)]

note, we've also got a progress meter in the compose window, advanced message
search, advanced addressbook search.

Comment 28

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

Attachment: screenshot of mail main window

notice the missing progressmeter

if, otoh, the progressmeter is in use, the window looks as before

want a screenshot of the subscribe window as well?

Comment 29

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

sspitzer, hm, right; looks like my lxr search was incomplete. I'll make a patch
for them too... but tomorrow.

Comment 30

[(not reading, please use [e-mail] instead)]

you might want to wait for jglick to comment before doing that, to see if she 
agrees with this change to the UI.

Comment 31

[jglick]

>So, the progress bar should stay where it is, but it should disappear 
>completely (as in {display: none}) when nothing is loading. This will make more 
>space for status text...

I agree. This change is ok with me.

Comment 32

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

removing browser from summary, as this affects mailnews as well.

Comment 33

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

Attachment: msg compose window

Comment 34

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

that's interesting. advanced addressbook search does not seem to ever use the
progressmeter, even though it is shown.

jglick: is it therefore ok to always hide it? it does not serve a function
anyway. (this is easier to code. if it will ever be working, it can be shown again)

Comment 35

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

Attachment: message search window

this should be the last dialog, pending a reply to comment 34; sspitzer, could
you review the patches here now?

Comment 36

[jglick]

>advanced addressbook search does not seem to ever use the progressmeter, even 
>though it is shown. is it therefore ok to always hide it? it does not serve a 
>function anyway.

Yes. Would be nice to have it working at some point though.

Comment 37

[(not reading, please use [e-mail] instead)]

sorry for the delay.  I'll do some reviews and testing over the next few days. 
I think this will make it into mozilla 1.2

Comment 38

[Matthew T (active 1999-2002)]

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

Comment 39

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

-> default owner

sspitzer, I gave up caring about my UI patches... so there's no need to review
this anymore.

Comment 40

[[e-mail]]

At one point the progress meter was allowed as a direct child of the status bar.
That would have allowed this bug to be fixed with a line of CSS...

Comment 41

[Paul Wyskoczka]

nsbeta1- per the nav triage team.

Comment 42

[Harvey Savage]

issue 2002120408, progress bar at bottom right of browser window remains filled
in a great majority of the time, you get no true indication of page loading
progress.

Tme morphing Mozilla icon at the upper right doesn't morph any longer, just sits
there, a dull blue-gray rectangle.

harvey r. savage

Comment 43

[[e-mail]]

Comment on attachment 93842 [details] [diff] [review]
subscribe dialog

If subscribe had been correctly written to use StartMeteors and StopMeteors
which attachment 95312 [details] [diff] [review] fixes then this wouldn't be necessary.

Comment 44

[[e-mail]]

Comment on attachment 95716 [details] [diff] [review]
msg compose window

Nits: use .hidden instead of setAttribute, and hide the panel before resetting
the progressmeter.

Comment 45

[[e-mail]]

Comment on attachment 95721 [details] [diff] [review]
message search window

Bug 229818 will obsolete this patch too.

Comment 46

[[e-mail]]

Comment on attachment 93842 [details] [diff] [review]
subscribe dialog

Patch bitrotted.

Comment 47

[[e-mail]]

Comment on attachment 95721 [details] [diff] [review]
message search window

Patch has bitrotted.

Comment 48

[[e-mail]]

Comment on attachment 95312 [details] [diff] [review]
mailnews main window part

Patch has "bitrotted".

Comment 49

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

Attachment: mailnews main window, unbitrotted

Comment 50

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

Comment on attachment 92269 [details] [diff] [review]
Better patch

hmm, this totally does not apply anymore...

Comment 51

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

Attachment: browser patch (checked in)

Comment 52

[[e-mail]]

Comment on attachment 140177 [details] [diff] [review]
browser patch (checked in)

>+      // Show the progress meter
>+      this.statusPanel.removeAttribute("hidden");
>       // Turn the progress meter and throbber off.
>+      this.statusPanel.setAttribute("hidden", "true");
Nit: Please use .hidden here.

Comment 53

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 140177 [details] [diff] [review]
browser patch (checked in)

sr=bzbarsky with neil's nit addressed

Comment 54

[HJ]

On WinXP, with mozilla 1.7b, there is no progressmeter in modern and in classic
the height is 1px off (see next attachment).

I've updated the target milestone to 1.8a, because mozilla 1.2 is long past and
we need this for the 1.8 cycle and removed keyword nsbeta1-

"nsbeta1-   	This keyword will be added to bugs which the Netscape evaluation
team has rejected for the next Netscape release."

There will be no future Netscape release, so it is now useless to still have
such keyword anyway.

Christian, my Q for you is: "Are you still working on this bug?" 
If yes, why is the bug status set to NEW and if not, why is it (still) assigned
to you?

Cheers,
/HJ

Comment 55

[HJ]

Attachment: Screenshot (WinXP/Mozilla 2004031616/classic theme) with both a displayed and hidden progressmeter

Comment 56

[HJ]

Attachment: Screenshot (WinXP/Mozilla 2004031616/classic theme) with both a displayed and hidden progressmeter

Oops, attachment 144572 [details] is a PSP file (default setting of Paint Shop Pro 8)
Here's an JPG version of the same image.

Comment 57

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

I'd appreciate it if people would not change the target milestone of my bugs


I'll need to look at the mailnews patches again (browser part is checked in),
and I intend to do that at some time, yes.

What do you mean with "both a hidden and displayed progressmeter"? it is
obviously displayed in that screenshot?

Comment 58

[HJ]

(In reply to comment #57)
> I'd appreciate it if people would not change the target milestone of my bugs

Sorry Biesi, but I am missing the logic of this. What is your point? What is the
logic of using mozilla 1.2beta? Mozilla 1.2beta is ancient history by now and
developers, like me, can hijack your non resolved bugs, if they want to! 

> I'll need to look at the mailnews patches again (browser part is checked in),
> and I intend to do that at some time, yes.

Guess you don't need help with this bug, but that was my point, but I'll rather
pass now.

> What do you mean with "both a hidden and displayed progressmeter"? it is
> obviously displayed in that screenshot?

True, not a perfect screenshot, but it is an combined screenshot. Just look at
the off-line icon. Look at the shading, obviously one pixel off.

Comment 59

[HJ]

Attachment: Here is a better screenshot (500% enlarged)

Comment 60

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

(btw, you didn't remove nsbeta1- earlier, doing that now)

(In reply to comment #58)
> Sorry Biesi, but I am missing the logic of this. What is your point? What is the
> logic of using mozilla 1.2beta? Mozilla 1.2beta is ancient history by now and
> developers, like me, can hijack your non resolved bugs, if they want to! 

it's a way to organize my buglist... ok well, right now it's somewhat
unorganized, but anyway :)

> Guess you don't need help with this bug, but that was my point, but I'll rather
> pass now.

Oh, if you want to take a look at updating the mailnews patches, that would be
cool. I don't know when I'll find the time for this. I am sorry for assuming
that you just wanted to know the state of this bug, rather than work on it :)

as for why this bug is NEW... I like to use NEW vs ASSIGNED as a way to keep
track of whether the bug already has a mostly-ready patch or not. this one hasn't :)

> True, not a perfect screenshot, but it is an combined screenshot. Just look at
> the off-line icon. Look at the shading, obviously one pixel off.

Ahh. I finally see your point :) it would be nice though not to make this bug a
general dumping ground for progressmeter issues :) as in, your bug would be a
different bug I'd say. hm... it may have been caused by the patch here, because
we now change whether progressbar appears. but anyway... please file a new bug,
cc me, and mention the bug# here. thanks.

sorry if comment 57 was sounding impolite/rude, I certainly didn't mean it to be.

Comment 61

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I think this caused regression bug 235871.  In particular, in some themes, the
progress meter was the tallest thing in the status bar.  With this patch, this
means the window resizes when the status bar appears and disappears.

Comment 62

[chris hofmann]

neeed to look at backing this out on the 1.7 branch....

Comment 63

[chris hofmann]

need to look at backing this out on the 1.7 branch....

Comment 64

[chris hofmann]

can someone put together a patch to back out on the 1.7 branch?

Comment 65

[(not reading, please use [e-mail] instead)]

> can someone put together a patch to back out on the 1.7 branch?

I can look into doing that.

I'm just looking at bonsai now to see what was landed (browser and mailnews, or
just browser).

Comment 66

[(not reading, please use [e-mail] instead)]

christian, should the TM be set to 1.8a, since I'm about to back this out of
1.7?  (see bug #235871)

also, it doesn't look like you checked in the mail parts of this patch, just the
mozilla/xpfe parts.

is that correct?

Comment 67

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

(In reply to comment #66)
> christian, should the TM be set to 1.8a, since I'm about to back this out of
> 1.7?  (see bug #235871)

ok, well, setting to something by which I maybe finish this up....

> also, it doesn't look like you checked in the mail parts of this patch, just the
> mozilla/xpfe parts.
> 
> is that correct?

Yes. The mailnews part don't have reviews yet, and some need updating.
they wouldn't need to be backed out anyway I think, since the problem is just
that webpages are continuously reloading, right?

Comment 68

[(not reading, please use [e-mail] instead)]

>> also, it doesn't look like you checked in the mail parts of this patch, just the
>> mozilla/xpfe parts.
>> 
>> is that correct?
>
>Yes. The mailnews part don't have reviews yet, and some need updating.
>they wouldn't need to be backed out anyway I think, since the problem is just
>that webpages are continuously reloading, right?

you might be right about that.  but let's be safe and keep the mailnews ones for
1.8 and not land them in 1.7 final.

if you have cycles, can you also review (I already have r=neil) my backout patch
in bug #235871?

Comment 69

[(not reading, please use [e-mail] instead)]

christian, thanks for the review.

I've backed out this fix from the 1.7 branch, but the trunk still has it so the
bug remains open.

I've asked asa to figure out if we should fix mozilla or let all theme
developers know they need to fix their themes.

clearning the blocker 1.7 as this is not part of 1.7

Comment 70

[[e-mail]]

Comment on attachment 138749 [details] [diff] [review]
mailnews main window, unbitrotted

With .hidden

Comment 71

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

-> default owner, doesn't look like I'm going to get to updating the mailnews
patches anytime soon...

Comment 72

[Matthias Bockelkamp]

Attachment: mailnews main window, unbitrotted

Comment 73

[[e-mail]]

Comment on attachment 189104 [details] [diff] [review]
mailnews main window, unbitrotted

This appears to be the same as attachment 138749 [details] [diff] [review] and as per comment 70 I prefer
.hidden instead of set or removeAttribute.

Comment 74

[Matthias Bockelkamp]

Comment on attachment 189104 [details] [diff] [review]
mailnews main window, unbitrotted

OK. I did not notice that comment.

Comment 75

[Matthias Bockelkamp]

Attachment: Mailnews main window, v2 (checked in)

set/removeAttribute() -> .hidden

Comment 76

[Matthias Bockelkamp]

Can someone check attachment 189176 [details] [diff] [review] in? Neil?

Comment 77

[Stephen Donner [:stephend] Not actively reading bugmail]

Pretty sure this broke the Subscribe window (IMAP/NNTP):

Error: this.statusPanel has no properties
Source File: chrome://messenger/content/mailWindow.js
Line: 435

Comment 78

[Matthias Bockelkamp]

Attachment: Fix for regression (checked in)

Comment 79

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

Comment on attachment 189537 [details] [diff] [review]
Fix for regression (checked in)

why not hidden="true"?

Comment 80

[[e-mail]]

(In reply to comment #79)
>(From update of attachment 189537 [details] [diff] [review] [edit])
>why not hidden="true"?
It sets shown during startup anyway (although for news it loads synchronously so
you don't notice...)

Comment 81

[Matthias Bockelkamp]

Attachment: Msg compose window, v2 (checked in)

Comment 82

[Matthias Bockelkamp]

Attachment: Message search window (checked in)

Comment 83

[Matthias Bockelkamp]

Comment on attachment 93842 [details] [diff] [review]
subscribe dialog

This will be fixed by Attachment 189537 [details] [diff]

Comment 84

[David :Bienvenu]

what makes us show the search window progress bar, i.e., remove the hidden
attribute?

Comment 85

[Matthias Bockelkamp]

@bienvenu: it's the conde in mailWindow.js, which also does this for the
subscribe dialog in the regression fix

@neil: could you check in the regression fix?

Comment 86

[Stephen Donner [:stephend] Not actively reading bugmail]

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

Comment 87

[Karsten Düsterloh]

Comment on attachment 189805 [details] [diff] [review]
Message search window (checked in)

Okay, looks good. Let's get these two into the lizard, too...

Comment 88

[Stephen Donner [:stephend] Not actively reading bugmail]

Is there more work to do, here?  If not, let's resolve this.

Comment 89

[Matthias Bockelkamp]

There is more work:

/editor/ui/composer/content/TextEditorAppShell.xul, line 162 -- <statusbarpanel
class="statusbarpanel-progress">

/extensions/inspector/resources/content/statusbarOverlay.xul, line 8 --
<statusbarpanel class="statusbarpanel-progress">

/mailnews/base/search/resources/content/FilterListDialog.xul, line 225 --
<statusbarpanel class="statusbarpanel-progress">

Comment 90

[Karsten Düsterloh]

> /editor/ui/composer/content/TextEditorAppShell.xul, line 162 -- 
> <statusbarpanel class="statusbarpanel-progress">

AFAICT, this is only used in a (mostly defunct) debug test page?

> /extensions/inspector/resources/content/statusbarOverlay.xul, line 8 --
> <statusbarpanel class="statusbarpanel-progress">

This file is referenced by DOMI's jar.mn only, so it's unused and could be
removed anyway?

> /mailnews/base/search/resources/content/FilterListDialog.xul, line 225 --
> <statusbarpanel class="statusbarpanel-progress">

True.

Comment 91

[Matthias Bockelkamp]

I didn't check that any further yet. CCing the module owners of composer and
inspector.

daniel,caillon: Can the progressmeters in the last comment be removed?

Comment 92

[Serge Gautherie (:sgautherie)]

Matthias,
Are you still working on this ?

Comment 93

[Robert Kaiser]

This has been mostly fixed a long time ago, any remaining issues should be filed as followup bugs.