Closed Bug 593626 Opened 14 years ago Closed 14 years ago

setting the tab title in onStatusChange possibly prevents the title bar from being updated

Categories

(Firefox :: Tabbed Browser, defect)

Product:
Firefox
Component:
Tabbed Browser
Platform:
x86
Windows 7
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 4.0b7

People

(Reporter: jdm, Assigned: dao)

Details

(Keywords: regression)

Attachments

(2 files)

patch
2.25 KB, patch
Gavin
: review+
Details | Diff | Splinter Review
alternative patch
3.11 KB, patch
Gavin
: review+
Gavin
: approval2.0+
Details | Diff | Splinter Review 
This is potentially just an unfiled intermittent orange, but it suspiciously occurred in Dao's push which involved a change to window title behaviour.

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1283626668.1283628429.15127.gz
Rev3 WINNT 6.1 mozilla-central opt test mochitest-other on 2010/09/04 11:57:48


s: talos-r3-w7-033
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_windowtitle.js | The window title for http://mochi.test:8888/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_windowtitle_page.html is correct (outside private browsing mode) - Got Minefield, expected Test title - Minefield
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_windowtitle.js | The window title for about:privatebrowsing is correct (outside private browsing mode) - Got Minefield, expected Would you like to start Private Browsing? - Minefield
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_windowtitle.js | The window title for http://mochi.test:8888/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_windowtitle_page.html is correct (inside private browsing mode) - Got Minefield (Private Browsing), expected Test title - Minefield (Private Browsing)
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_windowtitle.js | The window title for about:privatebrowsing is correct (inside private browsing mode) - Got Minefield (Private Browsing), expected Private Browsing - Minefield (Private Browsing)
Blocks: 593447
Component: Private Browsing → Tabbed Browser
QA Contact: private.browsing → tabbed.browser
OS: Linux → Windows 7
Summary: Various failures in browser_privatebrowsing_windowtitle.js → Title bar not always updated when loading a page (various failures in browser_privatebrowsing_windowtitle.js)
Attached patch patch — — Splinter Review 
DOMTitleChanged is asynchronous, so it's racing with onStateChange; when DOMTitleChanged loses, it won't update the title bar.
Assignee: nobody → dao
Status: NEW → ASSIGNED
Attachment #472236 - Flags: review?(gavin.sharp)
(In reply to comment #9)
> DOMTitleChanged is asynchronous, so it's racing with onStateChange; when
> DOMTitleChanged loses, it won't update the title bar.

I don't understand how winning/losing a race with onStatusChange would affect the the DOMTitleChanged handler's behavior. Why is it's call to updateTitleBar not sufficient?
(In reply to comment #38)
> (In reply to comment #9)
> > DOMTitleChanged is asynchronous, so it's racing with onStateChange; when
> > DOMTitleChanged loses, it won't update the title bar.
> 
> I don't understand how winning/losing a race with onStatusChange would affect
> the the DOMTitleChanged handler's behavior. Why is it's call to updateTitleBar
> not sufficient?

Because it doesn't call updateTitlebar when it loses. (setTabTitle returns false and the DOMTitleChanged handler assumes it has nothing to do.)
Keywords: regression
Whiteboard: [orange] → [orange][perma-orange]
How is onStatusChange related in any way to the title setting code?

(I wish we could have this discussion in a bug that wasn't being spammed by tbplbot...)
(In reply to comment #59)
> How is onStatusChange related in any way to the title setting code?

Well it does this:

if (this.mTab.label == this.mTabBrowser.mStringBundle.getString("tabs.loading"))
  this.mTabBrowser.setTabTitle(this.mTab);

So if it sets the tab title, a DOMTitleChanged event following that won't update the title bar. That's more or less what I said in the previous comment, though, so maybe I don't understand your question.

However, since this test is failing on Windows 7, the more immediately relevant cause is very likely bug 593836.
No longer blocks: 593447
Depends on: 593836
(In reply to comment #61)
> (In reply to comment #59)
> > How is onStatusChange related in any way to the title setting code?
> 
> Well it does this:
> 
> if (this.mTab.label ==
> this.mTabBrowser.mStringBundle.getString("tabs.loading"))
>   this.mTabBrowser.setTabTitle(this.mTab);
> 
> So if it sets the tab title, a DOMTitleChanged event following that won't
> update the title bar.

The sequence would be:
- content sets document.title (aboutPrivateBrowsing.xhtml does this)
- document finishes loading
- tabbrowser.xml gets the pending DOMTitleChanged event
No longer blocks: 438871
No longer depends on: 593836
Summary: Title bar not always updated when loading a page (various failures in browser_privatebrowsing_windowtitle.js) → setting the tab title in onStatusChange possibly prevents the title bar from being updated
Whiteboard: [orange][perma-orange]
Attachment #472236 - Flags: review?(gavin.sharp) → review+
It looks like _replaceLoadingTitle in nsSessionStore has the same issue?
_replaceLoadingTitle is called when a window or tab closes, so it doesn't look like it would matter. That said, maybe setTabTitle should just call updateTitlebar.

        Attachment #473559 -
        Flags: review?(gavin.sharp)

        Attachment #473559 -
        Flags: review?(gavin.sharp)

        Attachment #473559 -
        Flags: review+

        Attachment #473559 -
        Flags: approval2.0+

    
    

    
  
http://hg.mozilla.org/mozilla-central/rev/7daa437a5fff
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b6

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: