clean up Document's handling of title changes
Created attachment 87488 [details] Patch
Ryosuke, let me know if this is what you were suggesting I do.
Comment on attachment 87488 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=87488&action=review > Source/WebCore/dom/Document.cpp:1354 > + if (m_titleElement) Okay, so this code is valid because only HTMLTitleElement and SVGTitleElement calls setTitleElement with an arbitrary element but that's not entirely obvious to me. Please assert that m_titleElement->hasTagName(titleTag). But I'd also prefer to have the condition in the if statement as well because otherwise we'll be introducing a security vulnerability if we ever hit the assertion.
Committed r82422: <http://trac.webkit.org/changeset/82422>
http://trac.webkit.org/changeset/82422 might have broken Windows XP Debug (Tests)
+ if (!isHTMLDocument()) + m_titleElement = 0; What happens in the case of XHTML documents?
I'm not sure if your comment is about the prior code or the new code. I tried pretty hard to not change the behavior of the code, so please let me know if I got that wrong. As for the prior code: For some context, m_titleElement is attempting to track the element that we are deriving the Document's title from. That is important so its contents can be updated if JS does 'document.title="foo"'. We only learn about these title elements if the title element notifies the Document via setTitle("...", this) (this is the function I renamed to setTitleElement in this change). It seems isHTMLDocument() is false for XHTML documents, but they still use HTMLTitleElement? So in that case, the code you quoted clears the reference from Document to the element, and so document.title=... doesn't keep it in sync. I just verified this with a local xhtml file. That is to say, the following does something different in XHTML vs HTML. <head><title id='t'>test</title></head> <body> <script>document.title = 'foo'; console.log(document.getElementById('t').innerText);</script> Thankfully, Ryosuke had me add an extra bit of verification before I landed the patch, so at least I didn't make it worse. I will open a bug about it. https://bugs.webkit.org/show_bug.cgi?id=57537