593623 - Don't dispatch DOMTitleChanged if the title hasn't changed

Status: NEW

(Core :: DOM: Core & HTML, defect, P5)

Description

[Dão Gottwald [:dao]]

Attachment: patch

This patch removes the workaround that has been added in bug 593447.

Comment 1

[Dão Gottwald [:dao]]

Attachment: patch

stripped the tabbrowser changes

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

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

Does this work also with hidden iframes?

Comment 3

[Dão Gottwald [:dao]]

As a side note, (In reply to comment #2)
> Comment on attachment 472237 [details] [diff] [review]
> patch
> 
> Does this work also with hidden iframes?

What kind of failure should I be looking for?

By the way, the event isn't actually useful for sub frames, fwiw.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

(In reply to comment #3) 
> What kind of failure should I be looking for?
If there is a no-op (old and new title are equal) title change in the hidden 
iframe, is the event dispatch?

> By the way, the event isn't actually useful for sub frames, fwiw.
Why it isn't useful?

Comment 5

[Dão Gottwald [:dao]]

(In reply to comment #4)
> > By the way, the event isn't actually useful for sub frames, fwiw.
> Why it isn't useful?

The event is supposed to help the browser keep up with title changes. However, iframe titles aren't displayed. tabbrowser.xml returns early if it gets the event from an iframe: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/tabbrowser.xml?mark=2451-2453#2446

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

That is how firefox UI uses the event, but I don't see how it is not useful
for sub frames in general.

Comment 7

[Dão Gottwald [:dao]]

Chances are that every singe Gecko browser out there either does it this way or unintentionally updates the main title due to a title change in a sub document (e.g. http://mxr.mozilla.org/comm-central/source/mail/base/content/specialTabs.js#396). I don't really see a use case for a browser to keep up with a sub document's title.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

The point is that the event should be fired consistently, whether or not
the document is in an iframe.

Comment 9

[Dão Gottwald [:dao]]

If it should fire on iframes, then it should of course fire consistently with regards to the title being set to the same value it had already. The thing is that I don't think it should fire on iframes at all. The use case this was added for in bug 100433 doesn't want it.

Anyway, I won't be able to spend more time on this in the near future.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Well, <browser> is a type of iframe.
And DOMTitleChanged is dispatched even for the top level xul document
(and there are extensions which use that event).

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

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

Actually, I think there is another problem, a bit tricky one.
What if the document is in bfcache and someone changes its title.
That shouldn't change docshellWin's title.
So, unfortunately we need some kind of check whether the document is the
current document in the docshell. And in that case we can
also get the docshell from, document's container. No need for shell->GetPresContext().
So only change the title if the document has container, and the document
is the current document in the container (docshell) and the current title
in the docshell isn't the one just set to document.

Comment 12

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.