Closed Bug 47909 Opened 24 years ago Closed 15 years ago

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

Categories

(SeaMonkey :: UI Design, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: hsivonen, Assigned: mbockelkamp)

References

(Blocks 1 open bug)

Details

(Keywords: helpwanted, Whiteboard: [fixedBeonex][geekweb-p2] need help to back out 235879)

Attachments

(7 files, 11 obsolete files)

(deleted), image/png
Details
(deleted), patch
neil
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review
(deleted), image/jpeg
Details
(deleted), patch
neil
: review+
Bienvenu
: superreview+
benjamin
: approval1.8b4+
Details | Diff | Splinter Review
(deleted), patch
neil
: review+
Bienvenu
: superreview+
benjamin
: approval1.8b4+
Details | Diff | Splinter Review
(deleted), patch
mnyromyr
: review+
Bienvenu
: superreview+
Details | Diff | Splinter Review
(deleted), patch
mnyromyr
: review+
Bienvenu
: superreview+
Details | Diff | Splinter Review
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.
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.
OS: Mac System 8.5 → All
Hardware: Macintosh → All
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)
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).
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.
Reassigning from bdonohoe to hangas
Assignee: bdonohoe → hangas
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...
QA Contact: mpt → zach
updating to new owner. sorry for the spam.
Assignee: hangas → mpt
Depends on: 79992
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.
Assignee: mpt → blaker
Component: User Interface Design → XP Apps: GUI Features
QA Contact: zach → paw
Summary: Recycling progress meter screen real estate → Browser status bar should only have progress meter while loading stuff
taking
Assignee: blaker → cbiesinger
per comment 8, removing dependency. this bug is no longer about moving the statusbar, only about hiding/showing it
No longer depends on: 79992
Attached patch patch (obsolete) (deleted) — Splinter Review
Attached patch Better patch (obsolete) (deleted) — Splinter Review
Attachment #92263 - Attachment is obsolete: true
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?
>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
Status: NEW → ASSIGNED
Now I do see the progress-meter working with a new profile. Boh... Sorry for the false alarm.
Target Milestone: --- → mozilla1.2beta
still have to check for other occurences
Comment on attachment 93111 [details] [diff] [review] mailnews part ah... this includes the fix for another bug... ignore the clearly unrelated lines :)
ok, I think I got them all now. ready for review.
Comment on attachment 92269 [details] [diff] [review] Better patch r=chanial@noos.fr (new mail address) I don't feel myself comfortable to review the Mail/News pathces since I never touched them
Attachment #92269 - Flags: review+
sspitzer, could you review the two mailnews patches on this bug?
bienvenu, could you review the two mailnews patches on this bug?
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.
Attached patch mailnews main window part (obsolete) (deleted) — Splinter Review
now rediffed to not include the unrelated changes (which have been checked in in the meantime)
Attachment #93111 - Attachment is obsolete: true
Christian, can you attach some screen shots? cc jglick, as we'll need her approval since this is a UI change.
note, we've also got a progress meter in the compose window, advanced message search, advanced addressbook search.
Attached image screenshot of mail main window (deleted) —
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?
sspitzer, hm, right; looks like my lxr search was incomplete. I'll make a patch for them too... but tomorrow.
you might want to wait for jglick to comment before doing that, to see if she agrees with this change to the UI.
>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.
removing browser from summary, as this affects mailnews as well.
Summary: Browser status bar should only have progress meter while loading stuff → status bar should only have progress meter while loading stuff
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)
Attached patch message search window (obsolete) (deleted) — Splinter Review
this should be the last dialog, pending a reply to comment 34; sspitzer, could you review the patches here now?
>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.
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
*** Bug 79926 has been marked as a duplicate of this bug. ***
Whiteboard: fixedBeonex
-> default owner sspitzer, I gave up caring about my UI patches... so there's no need to review this anymore.
Assignee: cbiesinger → blaker
Status: ASSIGNED → NEW
Whiteboard: fixedBeonex → [fixedBeonex][geekweb-p2]
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...
nsbeta1- per the nav triage team.
Keywords: nsbeta1nsbeta1-
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
Summary: status bar should only have progress meter while loading stuff → status bar should only have progress meter while loading stuff (hide when not needed, only show when needed)
Assignee: blake → neil.parkwaycc.co.uk
Assignee: neil.parkwaycc.co.uk → cbiesinger
Attachment #93842 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #95312 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #95316 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #95716 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #95721 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #95316 - Flags: review?(neil.parkwaycc.co.uk)
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.
Depends on: 229818
Comment on attachment 95716 [details] [diff] [review] msg compose window Nits: use .hidden instead of setAttribute, and hide the panel before resetting the progressmeter.
Attachment #95716 - Flags: review?(neil.parkwaycc.co.uk) → review+
Comment on attachment 95721 [details] [diff] [review] message search window Bug 229818 will obsolete this patch too.
Comment on attachment 93842 [details] [diff] [review] subscribe dialog Patch bitrotted.
Attachment #93842 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 95721 [details] [diff] [review] message search window Patch has bitrotted.
Attachment #95721 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 95312 [details] [diff] [review] mailnews main window part Patch has "bitrotted".
Attachment #95312 - Flags: review?(neil.parkwaycc.co.uk) → review-
Attached patch mailnews main window, unbitrotted (obsolete) (deleted) — Splinter Review
Attachment #95312 - Attachment is obsolete: true
Attachment #138749 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 92269 [details] [diff] [review] Better patch hmm, this totally does not apply anymore...
Attachment #92269 - Attachment is obsolete: true
Attachment #140177 - Flags: review?(neil.parkwaycc.co.uk)
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.
Attachment #140177 - Flags: review?(neil.parkwaycc.co.uk) → review+
Attachment #140177 - Flags: superreview?(bz-vacation)
Comment on attachment 140177 [details] [diff] [review] browser patch (checked in) sr=bzbarsky with neil's nit addressed
Attachment #140177 - Flags: superreview?(bz-vacation) → superreview+
Attachment #140177 - Attachment description: browser patch → browser patch (checked in)
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
Target Milestone: mozilla1.2beta → mozilla1.8alpha
Oops, attachment 144572 [details] is a PSP file (default setting of Paint Shop Pro 8) Here's an JPG version of the same image.
Attachment #144572 - Attachment is obsolete: true
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?
Target Milestone: mozilla1.8alpha → mozilla1.2beta
(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.
Attachment #144573 - Attachment is obsolete: true
(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.
Keywords: nsbeta1-
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.
neeed to look at backing this out on the 1.7 branch....
Flags: blocking1.7+
need to look at backing this out on the 1.7 branch....
Whiteboard: [fixedBeonex][geekweb-p2] → [fixedBeonex][geekweb-p2] need help to back out 235879
can someone put together a patch to back out on the 1.7 branch?
> 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).
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?
(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?
Target Milestone: mozilla1.2beta → mozilla1.8beta
>> 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?
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
Flags: blocking1.7+
Comment on attachment 138749 [details] [diff] [review] mailnews main window, unbitrotted With .hidden
Attachment #138749 - Flags: review?(neil.parkwaycc.co.uk) → review+
-> default owner, doesn't look like I'm going to get to updating the mailnews patches anytime soon...
Assignee: cbiesinger → guifeatures
Keywords: helpwanted
Priority: P3 → --
QA Contact: pawyskoczka
Target Milestone: mozilla1.8beta → ---
Product: Core → Mozilla Application Suite
Attached patch mailnews main window, unbitrotted (obsolete) (deleted) — Splinter Review
Assignee: guifeatures → mbockelkamp
Attachment #138749 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #189104 - Flags: superreview?(bienvenu)
Attachment #189104 - Flags: review?(neil.parkwaycc.co.uk)
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.
Attachment #189104 - Flags: review?(neil.parkwaycc.co.uk) → review-
Comment on attachment 189104 [details] [diff] [review] mailnews main window, unbitrotted OK. I did not notice that comment.
Attachment #189104 - Flags: superreview?(bienvenu)
set/removeAttribute() -> .hidden
Attachment #189104 - Attachment is obsolete: true
Attachment #189176 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #189176 - Flags: review?(neil.parkwaycc.co.uk) → review+
Attachment #189176 - Flags: superreview?(bienvenu)
Attachment #189176 - Flags: superreview?(bienvenu) → superreview+
Attachment #189176 - Flags: approval1.8b4?
Attachment #189176 - Flags: approval1.8b4? → approval1.8b4+
Can someone check attachment 189176 [details] [diff] [review] in? Neil?
Attachment #189176 - Attachment description: Mailnews main window, v2 → Mailnews main window, v2 (checked in)
Pretty sure this broke the Subscribe window (IMAP/NNTP): Error: this.statusPanel has no properties Source File: chrome://messenger/content/mailWindow.js Line: 435
Attached patch Fix for regression (checked in) (deleted) — Splinter Review
Attachment #189537 - Flags: superreview?(bienvenu)
Attachment #189537 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #189537 - Flags: approval1.8b4?
Comment on attachment 189537 [details] [diff] [review] Fix for regression (checked in) why not hidden="true"?
Attachment #189537 - Flags: review?(neil.parkwaycc.co.uk) → review+
Attachment #189537 - Flags: superreview?(bienvenu) → superreview+
(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...)
Attachment #95716 - Attachment is obsolete: true
Attachment #189803 - Flags: superreview?(bienvenu)
Attachment #189803 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #95721 - Attachment is obsolete: true
Attachment #189805 - Flags: superreview?(bienvenu)
Attachment #189805 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 93842 [details] [diff] [review] subscribe dialog This will be fixed by Attachment 189537 [details] [diff]
Attachment #93842 - Attachment is obsolete: true
Attachment #189803 - Flags: superreview?(bienvenu) → superreview+
Attachment #189537 - Flags: approval1.8b4? → approval1.8b4+
what makes us show the search window progress bar, i.e., remove the hidden attribute?
@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?
Attachment #189805 - Flags: superreview?(bienvenu) → superreview+
Attachment #189537 - Attachment description: Fix for regression → Fix for regression (checked in)
*** Bug 301585 has been marked as a duplicate of this bug. ***
Attachment #189803 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #189803 - Flags: review+
Attachment #189803 - Flags: approval1.8b4?
Comment on attachment 189805 [details] [diff] [review] Message search window (checked in) Okay, looks good. Let's get these two into the lizard, too...
Attachment #189805 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #189805 - Flags: review+
Attachment #189805 - Flags: approval1.8b4?
Attachment #189803 - Flags: approval1.8b4? → approval1.8b4+
Attachment #189805 - Flags: approval1.8b4? → approval1.8b4+
Attachment #189803 - Attachment description: Msg compose window, v2 → Msg compose window, v2 (checked in)
Attachment #189805 - Attachment description: Message search window → Message search window (checked in)
Is there more work to do, here? If not, let's resolve this.
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">
> /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.
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?
Matthias, Are you still working on this ?
Component: XP Apps: GUI Features → UI Design
This has been mostly fixed a long time ago, any remaining issues should be filed as followup bugs.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: