Closed
Bug 169380
Opened 22 years ago
Closed 20 years ago
RFE: Add LED to tab labels to indicate tab load status
Categories
(SeaMonkey :: Tabbed Browser, enhancement)
SeaMonkey
Tabbed Browser
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: moz-spam-filtered.20.nickj, Assigned: neil)
References
(Blocks 1 open bug)
Details
Attachments
(7 files, 3 obsolete files)
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.1) Gecko/20020826 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.1) Gecko/20020826 When tabs load, it would be useful if they gave a colour indication of how loaded they are (like a very simple reflection of the progress bar that shows in the status bar). So when a group is opened, initially every tab opened could start out with a red block in it's tab. As a particular tab loaded, it could go yellow (perhaps with 2 or 3 substeps of yellow), and then when it is totally finished go green. Currently tabs indicate they are either loaded or loading; The extra information would be useful. It would for example allow a bookmark group to be opened when using a dial-up modem, and for the user to then prioritise for reading only those tabs that have either finished loading or that are quite close to being fully loaded. Reproducible: Always Steps to Reproduce: There's an example of a 3-tier traffic light system for tab status shown in a dialog box in point 22 of http://multizilla.mozdev.org/features.html ; Around two more tiers to indicate various states of 'pending' would be ideal.
Comment 1•22 years ago
|
||
This would be an alternative to Bug 117161.
Reporter | ||
Comment 2•22 years ago
|
||
I hadn't found bug 117161 previously, so thank you for mentioning that, and yes, this is an alternative approach. The screenshots attached to that bug are quite helpful, so to illustrate I've taken one of those and modified it to show what this idea could look like, and I'm attaching this now.
Reporter | ||
Comment 3•22 years ago
|
||
There are a few differences to this approach to the one mentioned in 117161, namely: 1) It doesn't use the background of the tab to illustrate load progress. This is because the size of each tab fluctuates as tabs are opened and closed, so you can't just look at each progress bar, you have to compare it to the size of the tab, and secondly this is probably too much information - the user just wants to know roughly how loaded something is (or not). 2) This does away with the separate page painting progress bar (probably too much information again, user just wants to know roughly how loaded a page is). 3) This does away with the spinning arrows, because they don't add anything extra, and thus this suggestion doesn't use any extra space. 4) This also uses the traffic light colours to help convey the information as to how loaded the page is.
Updated•22 years ago
|
QA Contact: sairuh → pmac
Comment 4•20 years ago
|
||
this should be marked wontfix. we're not going to put a progress indicator in each tab. where's our UI czar?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 5•20 years ago
|
||
I could live with adding the minimum backend to allow a coloured spinning arrows extension, i.e. it would be possible to install a small extension that would override the current spinning arrows to colour-code the loading progress.
Reporter | ||
Comment 6•20 years ago
|
||
A minimal change to allow an extension that does progress indication sounds ideal. The simplest interface I can conceive of is something that gets passed a tab-load percentage, and that returns a tab-icon url, along these lines: // Default implementation, just returns B&W spinning arrows getLoadingTabIcon : function(percentage) { return url("chrome://global/skin/icons/Loading.gif"); } // versus an extension that shows colour-coded tab-load progress getLoadingTabIcon : function(percentage) { if (percentage <= 0) return url("chrome://global/skin/icons/Loading-1.gif"); else if (percentage <= 20) return url("chrome://global/skin/icons/Loading-2.gif"); else if (percentage <= 40) return url("chrome://global/skin/icons/Loading-3.gif"); else if (percentage <= 60) return url("chrome://global/skin/icons/Loading-4.gif"); else if (percentage <= 80) return url("chrome://global/skin/icons/Loading-5.gif"); else return url("chrome://global/skin/icons/Loading-6.gif"); }
Assignee | ||
Comment 7•20 years ago
|
||
Heh, I was thinking of something simpler than that, instead of our current busy="true" implementation, use busy="<percentage value 00-99>" then you can use CSS to style the busy image e.g. tab[busy^="5"] would style 50-59%.
Reporter | ||
Comment 8•20 years ago
|
||
Ahh - now that's good - and definitely simpler! So a load-progress extension could be done with CSS + a few icon images, but when no such extension was installed then a loading tab would look the same as it does at the moment (i.e. the current spinning arrows icon would match all busy percentages). Yes, that's much better.
Assignee | ||
Comment 9•20 years ago
|
||
The changes to browser.css are needed for consistency with Modern. The changes to tabbrowser.xml do the work that an extension can hook into.
Reporter | ||
Comment 10•20 years ago
|
||
I'm trying to get the load progress icons to work, and not having much luck, so I strongly suspect I'm doing something wrong. Here's what I've done: I've made the patch changes to tabbrowser.xml (extracted toolkit.jar, modified, and rezipped up back into toolkit.jar). With the browser.css, I'm running Firefox 0.8, and I couldn't find an exact match for the browser.css changes. (May be that you're running a more recent build?). The closest match was in classic.jar, namely classic\skin\classic\global\browser.css. The middle of this file contained this: ========= .tab-icon { margin-top: 1px; margin-right: 3px; width: 16px; height: 16px; list-style-image: url("chrome://global/skin/icons/folder-item.png"); -moz-image-region: rect(0px, 16px, 16px, 0px); } tab[busy] > .tab-icon { list-style-image: url("chrome://global/skin/icons/Loading.gif"); } ======== So couldn't see anything that exactly matched this change (was close, but the file path was different, and the '-moz-image-region' line was gone): ======== margin-right: 3px; width: 16px; height: 16px; +} + +tab { list-style-image: url("chrome://communicator/skin/bookmarks/bookmark-item.gif"); } ======== Consequently I didn't make this change. I tried making this change: ======== -tab[busy] > .tab-icon { +tab[busy] { ======== However restarting the browser with this resulted in no spinning-arrows icon showing up when the page was loading. I wasn't sure whether this was good or bad, but I figured it was a case of the user's custom CSS overriding the inbuilt CSS, so figured it probably should work as per normal if there was no user-customised CSS. Conseqently I undid this change, so overall there were no changes to the browser.css file. I then made some very basic 16x16 GIF icons, and a very basic userChrome.css. I'm attaching a ZIP file containing these files. The icons I put into the classic.jar file, under classic\skin\classic\global\icons, which is also where the current spinning arrows icon (Loading.gif) is located. When I restarted the browser though, it had the standard spinning arrows icon, not the progressive loading icons, so I've done something wrong. The tabbrowser.xml changes were straightforward, and the Gif icons seem fine, so that leaves as the most probable mistakes I've made: * the browser.css changes that I didn't make. * the userChrome.css, where I'm making a best guess at what the correct syntax should be. Is there anything obviously wrong in what I'm doing? Any pointers would be most appreciated.
Assignee | ||
Comment 11•20 years ago
|
||
I noticed a bug because I don't enforce two digits :-/ Also I made it easier to customize, there was an issue with the last patch. +} + +tab { This bit goes after the height line. Perhaps you were hit by bug 192022 which "disabled" the progress bar?
Attachment #145200 -
Attachment is obsolete: true
Assignee | ||
Comment 12•20 years ago
|
||
I have this working in conjunction with attachment 145327 [details] [diff] [review].
Reporter | ||
Comment 13•20 years ago
|
||
Superb! Thank you! I got it running using a nightly build of Firefox, and it works great - screenshot is attached. The only small thing I wondered about was setting 'value' to be equal to zero when nothing has loaded. Currently it goes: spinning arrows, then the various tab load icons, then the web page's icon. Basically I wondered if it would be possible to override the spinning arrows altogether, and just have the value^="0" case match when nothing has loaded. I'm guessing that the reason this doesn't happen currently is because 'onProgressChange' does not get called until some data get loaded, so it doesn't get the chance to set the value to zero. I tried doing this ~line 200 of tabbrowser.xml: =============================================== if (aWebProgress.DOMWindow == this.mBrowser.contentWindow) this.mBrowser.userTypedValue = null; if (!this.mBlank) { + this.mTab.setAttribute("value", "00"); this.mTab.setAttribute("busy", "true"); this.mTab.label = this.mTabBrowser.mStringBundle.getString("tabs.loading"); =============================================== This seems to work OK, but my knowledge of Mozilla's internals is pretty scant, so I'm not sure if I'm making that change in the right place, or if it will have any unintended consequences.
Assignee | ||
Comment 14•20 years ago
|
||
Perhaps this would also work for you? .tabbrowser-tab[busy="true"] { list-style-image: url("Loading-00.gif") !important; }
Reporter | ||
Comment 15•20 years ago
|
||
Yep, that works great, and I'd much rather have it in CSS than modify the code. Thank you. It's all working great now, so that by default it looks exactly the same as before, or with some small additions to the userChrome the tab load indicators appear. Good stuff. Hopefully the patch will be included in time for the next release.
Assignee | ||
Comment 16•20 years ago
|
||
Well if you want it in FireFox your best bet is to file a new bug under that component and attach your version of the patch there.
Assignee | ||
Comment 17•20 years ago
|
||
Comment on attachment 145327 [details] [diff] [review] Better patch As for Mozilla, let's ask for moa first...
Attachment #145327 -
Flags: superreview?(jag)
Comment 18•20 years ago
|
||
Comment on attachment 145327 [details] [diff] [review] Better patch I like this idea. I'm not sure if "value" is the best attribute name for this though. Why not call it "progress"? Could this attribute (for a very short time) be set to "100", causing the rule for 10% to match? There might be some flicker as it goes from 90% to 100% to the site icon / bookmark icon.
Assignee | ||
Comment 19•20 years ago
|
||
>+ this.mTab.setAttribute("value", ("0" +
Math.floor(aCurTotalProgress * 100 / aMaxTotalProgress)).slice(-2));
It occurred to me that the second digit is probably unnecessary anyway; how
about if I change the calculation to this.mTab.setAttribute("progress",
Math.floor(aCurTotalProgress * 9.9 / aMaxTotalProgress)); ?
Reporter | ||
Comment 20•20 years ago
|
||
I found the best way to really test the corner cases of tab load indicators was to load between 30 and 50 tabs at once, which causes everything to go much slower by saturating my network link, and then leave it until everything finished loading, and see what happens. The update in the comment above is definitely better - without this it would occasionally show the nothing-load-state for a tab that had finished loading - I'm guessing that the progress value had come out as "100", and then I guess that became "00" after the "slice(-2)" took effect, resulting in the icon becoming the 0% nothing-is-loaded icon. That doesn't happen any more with the change in the above comment. There is something I noticed though in the process of this stress-testing: There were a very small number of pages that seem to end up with their tab icon stuck on the state just before the fully loaded state (i.e. matches the 90-99% loaded case). They stay in this state, even after the page has completed fully loading (i.e. the top right of the browser has stopped spinning for that page, and all network activity has stopped). I have a sneaking suspicion though that this may be a problem with some other part of the browser, and not with this patch. In other words, a case of this change exposing some other pre-existing problem, rather than causing the problem. I say this for 2 reasons: 1) I couldn't get it to go away by adjusting the patch (such as not setting the progress value if it was > 99%). 2) It seems to happen particularly on certain web pages, and other pages are fine. After testing several hundred bookmarks, I was able to make a short list of some pages that tended to have this problem, namely: http://h71000.www7.hp.com/wizard/wiz_5486.html http://www.ibj.usyd.edu.au/ http://www.abc.net.au/science/k2/stn/default.htm http://www.measureup.com/Cart/more_product_infoMUP.asp?id=478 http://www.askmecorp.com/Notfound.asp?404;http://www.askmecorp.com/BusinessSplash.asp http://www.telusplanet.net/public/sparkman/netcalc.htm http://www.pbs.org/wgbh/pages/frontline/shows/dotcon/historical/bubbles.html http://www.jvc-australia.com/products/_products_sub_group_details.asp?ID=1083 http://www.maxtor.com/en/support/service/index.htm http://websupport.wdc.com/websupport/clearexp_scripts/warrantystart.asp I've attached an image to this comment of what it looks like when just these problematic pages have finished loading. Should I maybe be filing a separate bug report for this?
Reporter | ||
Comment 21•20 years ago
|
||
Doh! I take it back about it being unrelated - and it can be fixed with a very small addition to the 'if' condition in the patch, like so: ====================== { + if (aMaxTotalProgress > 0 && this.mTab.hasAttribute("busy")) + this.mTab.setAttribute("progress", Math.floor(aCurTotalProgress * 9.9 / aMaxTotalProgress)); if (!this.mBlank && this.mTabBrowser.mCurrentTab == this.mTab) { ====================== With this, I couldn't get any pages to stuff up, even the ones listed above that previously were reproducibly causing problems. Sorry for the confusion!
Assignee | ||
Comment 22•20 years ago
|
||
Comment on attachment 145327 [details] [diff] [review] Better patch >@@ -229,6 +231,7 @@ > this.mBlank = false; > > this.mTab.removeAttribute("busy"); >+ this.mTab.removeAttribute("value"); > > var location = aRequest.QueryInterface(nsIChannel).URI; > if (this.mIcon) { Nick, I hope you changed this to say "progress" instead of "value"... jag, assuming Nick did, then it might just be easier to require the extension to style .tabbrowser-tab[busy="true"][progress="N"] and not bother about removing the progress attribute?
Reporter | ||
Comment 23•20 years ago
|
||
> Nick, I hope you changed this to say "progress" instead of "value"... Yes, I did. I know that's weird though, because it suggests that "onProgressChange" gets called after busy=false, which I wouldn't expect in theory, but it seems to happen on some pages in practice. > require the extension to style .tabbrowser-tab[busy="true"][progress="N"] I've tried this, and it seems to works fine on all sites, including the ones that were causing problems, and it's also fine with heaps of tabs loading at once. I.e.: userChrome.css contains lines for the various cases, like this one: .tabbrowser-tab[busy="true"][progress="0"] { list-style-image: url("Loading-00.gif") !important; } And the tabbrowser.xml contains additions like so in the appropriate places: + if (aMaxTotalProgress > 0) + this.mTab.setAttribute("progress", Math.floor (aCurTotalProgress * 9.9 / aMaxTotalProgress)); Plus: + this.mTab.removeAttribute("progress"); Plus: + t.className = "tabbrowser-tab"; And the browser.css changes as per standard. With this setup it all works fine, no problems.
Assignee | ||
Comment 24•20 years ago
|
||
>+ this.mTab.removeAttribute("progress");
Actually the point is that having the [busy="true"] in the CSS means that we
don't have to remove the progress attribtue any more.
Reporter | ||
Comment 25•20 years ago
|
||
> don't have to remove the progress attribute any more.
OK. I've just tested this though, and I'm sorry to say it doesn't work so well
if the progress attribute is not removed.
The best way to see why is with the attached screenshot. To create it, I loaded
about 20 tab at once. I let that completely finish. Then I loaded about 40 tabs
at once, overwriting the previous 20 tabs, and soon as that started, I took a
screenshot. What happens is that the first 20 tabs still have their progress
value set to around 90% (because the attribute was not removed), so those first
20 tabs show as nearly fully loaded, and then they jump back down to 10 or 20%
loaded, as data starts to arrive for those tabs. The last 20 tabs start at 0%
progress, as expected.
Attachment #145584 -
Attachment is obsolete: true
Reporter | ||
Comment 26•20 years ago
|
||
Actually, I've just realised that something like the above happens with those 10 pages listed before that were still displaying the load indicators after finishing loading. This can happens even if you *do* remove the progress attribute, because there's something weird about those 10 pages that causes their progress to get set again even *after* busy=false. So, in this test scenario, you load the 10 fussy pages, and it looks normal. Then clear the cache. Then load the same 10 pages again. Immediately take a screenshot. They start at 90% loaded (image attached), because their progress value got set again (after it was removed) when they were loading the first time. The easiest way to fix this seems to be to explicitly initialize progress to zero when the tab starts loading. [The hard way is to work out why 'onProgressChange' gets called after busy=false, but that's way beyond the scope of this bug.] Also, it's probably best to initialize progress before setting busy=true as well, for good measure. I.e. make an additional modification to 'onStateChange' like so: ========================================================== if (!this.mBlank) { + this.mTab.setAttribute("progress", "0"); this.mTab.setAttribute("busy", "true"); this.mTab.label = this.mTabBrowser.mStringBundle.getString("tabs.loading"); ========================================================== Note that this is a pretty nasty and non-obvious test case, which is why I didn't find it before. Works fine now though if progress is initialized to zero.
Assignee | ||
Comment 27•20 years ago
|
||
Ok, so again this is with the .tabbrowser[busy="true"][progress="N"] style, but now this removes the progress attribute before setting busy="true", which is a minor variation on Nick's idea because I want the spinning arrows while we're trying to establish a connection. Nick can always change his CSS to .tabbrowser-tab[busy="true"] { list-style-image: url(progress0.gif); } if he never wants the spinning arrows.
Attachment #145327 -
Attachment is obsolete: true
Assignee | ||
Updated•20 years ago
|
Attachment #145734 -
Flags: superreview?(jag)
Attachment #145734 -
Flags: review?(bugs4hj)
Comment 28•20 years ago
|
||
Comment on attachment 145734 [details] [diff] [review] New approach sr=jag You know someone's gonna file a bug on the inaccurracy of that progress * 9.9 thing ;-)
Attachment #145734 -
Flags: superreview?(jag) → superreview+
Reporter | ||
Comment 29•20 years ago
|
||
Confirming that new approach patch works great, including for all previously described test cases, and that no problems were encountered. Excellent work!
Assignee | ||
Updated•20 years ago
|
Attachment #145734 -
Flags: review?(bugs4hj) → review?(bugs)
Assignee | ||
Updated•20 years ago
|
Attachment #145734 -
Flags: review?(bugs) → review?(mconnor)
Comment 30•20 years ago
|
||
Comment on attachment 145734 [details] [diff] [review] New approach r=mconnor@myrealbox.com
Attachment #145734 -
Flags: review?(mconnor) → review+
Assignee | ||
Comment 31•20 years ago
|
||
Fix checked in to seamonkey.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Updated•20 years ago
|
Attachment #145327 -
Flags: superreview?(jag)
Comment 32•19 years ago
|
||
*** Bug 271665 has been marked as a duplicate of this bug. ***
Updated•16 years ago
|
Product: Core → SeaMonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•