540248 - Race condition when updating the window title during tab preview

Status: RESOLVED FIXED

(Firefox :: Shell Integration, defect)

Description

[Rob Arnold [:robarnold]]

Attachment: Don't set the title in previewmode

There seems to be a race condition in the Windows taskbar. If the main window updates its title during a preview, that title is drawn instead of the focused preview's title. In tabbrowser.xml we are intentionally changing the title but one of the possible solutions in bug 520807 would cause the DWM to no longer draw the titlebar. This patch removes that change in title while in previewmode. This method is not perfect though - any update to the actual window title (say via some JS on a timer) overrides the title drawn by the DWM during preview. This issue also affects IE8.

Comment 1

[Dão Gottwald [:dao]]

So as long as we draw the title bar, this means we would display the title for the tab that was selected beforehand? I don't think we want this.

Comment 2

[Rob Arnold [:robarnold]]

(In reply to comment #1)
> So as long as we draw the title bar, this means we would display the title for
> the tab that was selected beforehand? I don't think we want this.

No. If we ask the DWM to draw the frame, then the title is always displayed correctly but the rest of the frame is not for maximized windows (bug 502807).

If we don't ask the DWM to draw the frame, then it will still draw the title of the previewed tab for us (though over the last-seen image of the window so there will be a frame). It will not, however, draw the rest of the frame.

We could avoid this whole issue (including DHTML changes to the title) if we drew *everything* about the window but that's not slated until the Firefox 4.0 timeframe so this is a temporary fix in a sense.

Comment 3

[Dão Gottwald [:dao]]

Comment on attachment 422062 [details] [diff] [review]
Don't set the title in previewmode

>             // Don't switch the fast find - this tab switch is temporary

You should update this comment and add a reference to this bug.

Comment 4

[Rob Arnold [:robarnold]]

Attachment: Don't set the title in previewmode

Comment 5

[Rob Arnold [:robarnold]]

Attachment: Don't set the title in previewmode

Oops, updated the wrong comment.

Comment 6

[Dão Gottwald [:dao]]

Comment on attachment 422097 [details] [diff] [review]
Don't set the title in previewmode

>+            if (!this._previewMode) {
>+              // Don't switch the fast find - this tab switch is temporary
>               this._fastFind.setDocShell(this.mCurrentBrowser.docShell);

This doesn't make sense now: "Don't" and "this tab switch is temporary" refer to the preceding line, contrary to how code comments are usually written.

>+              // XXX Bug 540248 - don't update the titlebar during preview
>+              this.updateTitlebar();

Same here.

Comment 7

[Rob Arnold [:robarnold]]

Attachment: Don't set the title in previewmode

(In reply to comment #6)
> (From update of attachment 422097 [details] [diff] [review])
> >+            if (!this._previewMode) {
> >+              // Don't switch the fast find - this tab switch is temporary
> >               this._fastFind.setDocShell(this.mCurrentBrowser.docShell);
> 
> This doesn't make sense now: "Don't" and "this tab switch is temporary" refer
> to the preceding line, contrary to how code comments are usually written.
> 
> >+              // XXX Bug 540248 - don't update the titlebar during preview
> >+              this.updateTitlebar();
> 
> Same here.

Ah right. Was not fully awake when I edited this. Better?

Comment 8

[Dão Gottwald [:dao]]

That's certainly better.

Comment 9

[Rob Arnold [:robarnold]]

Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/d39bcd1d6ff8

Comment 10

[Rob Arnold [:robarnold]]

Comment on attachment 422109 [details] [diff] [review]
Don't set the title in previewmode

Requesting approval so that bug 520807 can be backported to 1.9.2.

Comment 11

[LegNeato]

Comment on attachment 422109 [details] [diff] [review]
Don't set the title in previewmode

a=LegNeato for 1.9.2.5. Please ONLY land this on mozilla-1.9.2 default, as we are still working on 1.9.2.4 on the relbranch

Comment 12

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/4a05e2566508