710978 - Title attribute of a window is not updated in nsXULWindow::SetTitle

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Geoff Lankow (:darktrojan)]

When a window's title is update by calling SetTitle, the DOM's title does not change. For example, in a view source window, the document element's title attribute is always "Nightly".

I've got a patch on Try now.

Comment 1

[Geoff Lankow (:darktrojan)]

Attachment: patch

Appears to have made it through try without breaking anything, although only Linux64 opt has finished.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 581890 [details] [diff] [review]
patch

Hmm.  I'm not sure what the right behavior is here.  Neil?

Comment 3

[Geoff Lankow (:darktrojan)]

Attachment: patch v2

Gah, forgot about the OS X quirk.

Comment 4

[[e-mail]]

Comment on attachment 581911 [details] [diff] [review]
patch v2

Sorry, but nsXULWindow::SetTitle is (eventually) how setting a XUL window element's title attribute updates the window's title, so it's fortunate that you were saved from an infinite loop.

If you really want the source window's chrome document to have the same title as the window, you need to find the code that's setting the window's title directly and change it to set the document's title instead. (And not by setting some random attribute either.)

And then SeaMonkey can remove its title setting hack in its tabbrowser.

Comment 5

[Geoff Lankow (:darktrojan)]

Attachment: patch v3

Is this more like it?

Comment 6

[[e-mail]]

(In reply to comment #4)
> And then SeaMonkey can remove its title setting hack in its tabbrowser.
Which this patch will in fact break...

Comment 7

[[e-mail]]

(In reply to comment #6)
> (In reply to comment #4)
> > And then SeaMonkey can remove its title setting hack in its tabbrowser.
> Which this patch will in fact break...
Quite badly, in fact :-(

Comment 8

[[e-mail]]

(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #4)
> > > And then SeaMonkey can remove its title setting hack in its tabbrowser.
> > Which this patch will in fact break...
> Quite badly, in fact :-(
So, what happens is that this call to SetTitle is asynchronous because it happens at an unsafe time. The subsequent call in SeaMonkey's tabbrowser to set the XUL window title is still synchronous, so it gets clobbered by the asynchronous title update that this patch generates. Fixing tabbrowser to set the document title will simply coalesce the asynchronous title updates.

Comment 9

[[e-mail]]

(In reply to comment #8)
> So, what happens is that this call to SetTitle is asynchronous because it
> happens at an unsafe time.
Actually that's not true either. Calls to nsXULWindow::SetTitle synchronously update the title (but there's no way of knowing what it is set to, except by the user looking at the screen). Calls to nsDocument::SetTitle synchronously update its title, but then the chrome/content tree owner gets notified asynchronously, and then a DOM title changed event is also fired. The chrome tree owner simply updates the window title, while the content tree owner calls through into this code to set the chrome document's title.

Comment 10

[[e-mail]]

Comment on attachment 582785 [details] [diff] [review]
patch v3

r=me if you give me a chance to fix SeaMonkey when you check this in.

Comment 11

[Serge Gautherie (:sgautherie)]

Comment on attachment 582785 [details] [diff] [review]
patch v3

Review of attachment 582785 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/viewsource/test/browser/browser_bug699356.js
@@ +10,5 @@
>      let gBrowser = aWindow.gBrowser;
>  
>      is(gBrowser.contentDocument.title, source, "Correct document title");
> +    is(aWindow.document.documentElement.getAttribute("title"),
> +      "Source of: " + source + ("nsILocalFileMac" in Components.interfaces ? "" : " - Nightly"),

Isn't "Nightly" Firefox specific?

I suggest code (taken from bug 729474 patch Av1) using indexOf() like
+        isnot(title.indexOf(aEvent.accessible.name), -1,
+              "Window title contains the name of active tab document" +
+              " (Is '" + aEvent.accessible.name + "' in '" + title + "'?)");

Comment 12

[Geoff Lankow (:darktrojan)]

Attachment: patch v4

This is more like it. Tested it on Try.

Comment 13

[Geoff Lankow (:darktrojan)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bb48e7c6aef1

Comment 14

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/bb48e7c6aef1

Comment 15

[Jens Hatlak (:InvisibleSmiley)]

(In reply to [e-mail] from comment #10)
> r=me if you give me a chance to fix SeaMonkey when you check this in.

Two questions:
1. where (bug #) will that happen?
2. will this bug fix finally fix the nasty bug where every browser window in DOMI is called "SeaMonkey"? I thought it was somehow impossible to fix, but maybe this bug made the difference.

Comment 16

[Jens Hatlak (:InvisibleSmiley)]

I'll answer them myself...

(In reply to Jens Hatlak (:InvisibleSmiley) from comment #15)
> Two questions:
> 1. where (bug #) will that happen?

Already happened, and this bug was referenced:
http://hg.mozilla.org/comm-central/rev/61a16304321b

> 2. will this bug fix finally fix the nasty bug where every browser window in
> DOMI is called "SeaMonkey"?

Yes, this bug fixed that! :-)

Comment 17

[Serge Gautherie (:sgautherie)]

(In reply to Jens Hatlak (:InvisibleSmiley) from comment #16)
> http://hg.mozilla.org/comm-central/rev/61a16304321b

V.Fixed wrt SeaMonkey part only, per bug 729474 comment 7 :-)