Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Reloading all tabs does not show progress in List all tabs pop-up

RESOLVED FIXED in Firefox 9

Status

()

Firefox
Tabbed Browser
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: George Carstoiu, Assigned: ithinc)

Tracking

({regression})

Trunk
Firefox 9
regression
Points:
---
Bug Flags:
in-litmus ?

Firefox Tracking Flags

(blocking2.0 -)

Details

Attachments

(1 attachment, 1 obsolete attachment)

1.94 KB, patch
dao
: review+
Details | Diff | Splinter Review
(Reporter)

Description

7 years ago
User-Agent:       Mozilla/5.0 (Windows NT 6.1; rv:2.0b9pre) Gecko/20110109 Firefox/4.0b9pre
Build Identifier: Mozilla/5.0 (Windows NT 6.1; rv:2.0b9pre) Gecko/20110109 Firefox/4.0b9pre

When reloading all tabs, in the pop-up from "List all tabs", progress is shown only for the websites that do not have a favicon.

Reproducible: Always

Steps to Reproduce:
1.Open browser with two tabs (ex: www.nypost.com, www.2advanced.com)
2.Right click on any tab and select "Reload all tabs" from context menu
3.Press "List all tabs" button before the website finish to reload.

Actual Results:  
The site that has a favicon (in our case www.nypost.com) does not show a progress icon.

Expected Results:  
All websites have the progress icon displayed while data is being reloaded.
Regression from Firefox 3.6. Dao, do you have an idea which fix could have been regressed this? Otherwise we will check for the regression range.
Status: UNCONFIRMED → NEW
blocking2.0: --- → ?
Ever confirmed: true
Keywords: regression, regressionwindow-wanted
Hardware: x86 → All
Version: unspecified → Trunk
(Reporter)

Updated

7 years ago
Whiteboard: [4b9]
This does not seem like a blocker, just a visual annoyance. Blocking-.
blocking2.0: ? → -
George, could you check older beta releases and nightly builds of Firefox 4, so we get the information when this has been regressed?
For the moment I have disabled the Litmus test:
https://litmus.mozilla.org/show_test.cgi?id=9988
Flags: in-litmus?
Regression Window
last good:http://hg.mozilla.org/mozilla-central/rev/198709160138

first bad:http://hg.mozilla.org/mozilla-central/rev/eec9a82ad740

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=198709160138&tochange=eec9a82ad740
Dao, do you see a changeset which could be related?
nope
Alex, this regression range is wrong. I exercised a hg bisect and none of those listed changeset have been caused this regression. Can you or George please re-check? Thanks.

Comment 9

6 years ago
Regression window:
Works:
http://hg.mozilla.org/mozilla-central/rev/78c90846f8c7
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100923 Firefox/4.0b7pre ID:20100923215709
Fails:
http://hg.mozilla.org/mozilla-central/rev/b82884bf3be7
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100924 Firefox/4.0b7pre ID:20100924013821
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=78c90846f8c7&tochange=b82884bf3be7
Suspected bug:
32f406acbb45	Dão Gottwald — Bug 597673 - Kill updateIcon. r=gavin
Blocks: 597673
Keywords: regressionwindow-wanted
This problem is minor. While the tabs are reloading I see progress icons spinning in the tabs, which give enough indication of reload progress.

Comment 11

6 years ago
Related problems: 

1) It happens by reloading only one tab, not necessarily all.

2) When first loading a new tab, the throbber appears, but only during the "connecting" stage. It doesn't appears on subsequent reloads or loads of a new page on the same tab. 

All the problems appeared on the build informed by Comment #9

On the default configuration, the only way to know if all your tabs have finished loading when they have overflown, is through the "list all tabs" button.

Let me know if I should file separate bugs for these problems.
(Assignee)

Comment 12

6 years ago
In Firefox 3.6 we remove "image" attribute when "busy" is set.

http://mxr.mozilla.org/mozilla1.9.2/source/browser/base/content/tabbrowser.xml#690

690       <method name="updateIcon">
691         <parameter name="aTab"/>
692         <body>
693           <![CDATA[
694             var browser = this.getBrowserForTab(aTab);
695             if (!aTab.hasAttribute("busy") && browser.mIconURL)
696               aTab.setAttribute("image", browser.mIconURL);
697             else
698               aTab.removeAttribute("image");
699           ]]>
700         </body>
701       </method>

Now we don't. "image" attribute on menuitem will override "list-style-image" css rule. We may handle this in _setMenuitemAttributes.
(Assignee)

Updated

6 years ago
Duplicate of this bug: 685829
(Assignee)

Comment 14

6 years ago
Created attachment 559717 [details] [diff] [review]
patch
Attachment #559717 - Flags: review?(dao)
Comment on attachment 559717 [details] [diff] [review]
patch

>--- a/browser/base/content/tabbrowser.xml
>+++ b/browser/base/content/tabbrowser.xml
>@@ -560,16 +578,17 @@
>                     this.mTabBrowser.useDefaultIcon(this.mTab);
>                 }
>
>                 if (this.mBlank)
>                   this.mBlank = false;
>
>                 this.mTab.removeAttribute("busy");
>                 this.mTab.removeAttribute("progress");
>+                this.mTabBrowser._tabAttrModified(this.mTab);

I think it's possible that the "busy" attribute was already absent here, in which case _tabAttrModified shouldn't be called. Also, some lines above, we seem to erroneously not call it for this.mTab.setAttribute("busy", "true").
Attachment #559717 - Flags: review?(dao) → review-
(Assignee)

Comment 16

6 years ago
Created attachment 560481 [details] [diff] [review]
patch v2

(In reply to Dão Gottwald [:dao] from comment #15)
> I think it's possible that the "busy" attribute was already absent here, in
> which case _tabAttrModified shouldn't be called.

Addressed.

> Also, some lines above, we
> seem to erroneously not call it for this.mTab.setAttribute("busy", "true").

setAttribute("busy", "true") is accompanied by setTabTitleLoading, within which _tabAttrModified is called. Needs better organizing?
Attachment #559717 - Attachment is obsolete: true
Attachment #560481 - Flags: review?(dao)
(In reply to ithinc from comment #16)
> > Also, some lines above, we
> > seem to erroneously not call it for this.mTab.setAttribute("busy", "true").
> 
> setAttribute("busy", "true") is accompanied by setTabTitleLoading, within
> which _tabAttrModified is called. Needs better organizing?

Not sure how else we'd organize this. Hopefully it's at least covered by a test, but I guess it's actually not...

Updated

6 years ago
Attachment #560481 - Flags: review?(dao) → review+

Updated

6 years ago
Assignee: nobody → ithinc
Whiteboard: [4b9] → [inbound]
https://hg.mozilla.org/mozilla-central/rev/73296f31e968
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → Firefox 9
You need to log in before you can comment on or make changes to this bug.