Closed Bug 624363 Opened 14 years ago Closed 13 years ago

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

Categories

(Firefox :: Tabbed Browser, defect)

Product:
Firefox
Component:
Tabbed Browser
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 9
Tracking Flags:
Tracking Status
blocking2.0 --- -

People

(Reporter: george.carstoiu, Assigned: tabutils+bugzilla)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

patch v2
1.94 KB, patch
dao
: review+
Details | Diff | Splinter Review 
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
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?
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.
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
This problem is minor. While the tabs are reloading I see progress icons spinning in the tabs, which give enough indication of reload progress.
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.
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.
Attached patch patch (obsolete) — — Splinter Review 

        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-

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch v2
        — 
          — Splinter Review
      

    


  
(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...

    
  

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

    
  
Assignee: nobody → ithinc
Whiteboard: [4b9] → [inbound]

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/73296f31e968
Status: NEW → RESOLVED
Closed: 13 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.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: