Open Bug 593623 Opened 12 years ago Updated 3 years ago

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

Categories

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

defect

Tracking

()

People

(Reporter: dao, Unassigned)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch patch (obsolete) — Splinter Review
This patch removes the workaround that has been added in bug 593447.
Attachment #472186 - Flags: review?(Olli.Pettay)
Attached patch patchSplinter Review
stripped the tabbrowser changes
Attachment #472186 - Attachment is obsolete: true
Attachment #472237 - Flags: review?(Olli.Pettay)
Attachment #472186 - Flags: review?(Olli.Pettay)
Comment on attachment 472237 [details] [diff] [review]
patch

Does this work also with hidden iframes?
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.
(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?
(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
That is how firefox UI uses the event, but I don't see how it is not useful
for sub frames in general.
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.
The point is that the event should be fired consistently, whether or not
the document is in an iframe.
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.
Assignee: dao → nobody
Status: ASSIGNED → NEW
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 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.
Attachment #472237 - Flags: review?(Olli.Pettay) → review-
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.
Priority: -- → P5
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.