Created attachment 383812 [details] Repro input Document::setTitle may append a new title element if none currently exists. If this triggers an event handler which deletes this new title element, the call to setTextContent will cause a crash.
<rdar://problem/57305579>
<rdar://56626071>
This is not a security bug.
Created attachment 383816 [details] Patch
WebKit's behavior of document.title's setter matches the current spec: https://html.spec.whatwg.org/multipage/dom.html#document.title which says: > Let element be the result of creating an element given the document element's node document, title, and the HTML namespace. > > Append element to the head element. Because the spec doesn't have mutation events they don't have to deal with the situation in which appending the element to the head element results in its removal :(
Comment on attachment 383816 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383816&action=review > Source/WebCore/dom/Document.cpp:1601 > + // Another null check since insertBefore may have triggered event that deletes m_titleElement This is a bit verbose. I think we can just say: "insertBefore above may have ran scripts which removed m_titleElement" > Source/WebCore/dom/Document.cpp:1613 > + // Another null check since appendChild may have triggered event that deletes m_titleElement > + if (m_titleElement) Ditto. > LayoutTests/fast/dom/Document/title-property-set-with-dom-event.html:1 > +<html> Can we add another test case with a SVG document since you're also making the code change to SVG code path?
I guess one thing we should check is what other browsers do. Do they set the value of title element despite of the fact they're no longer in the document? What do they do if they insert the removed element into a new document (e.g. into an iframe's contentDocument).
Firefox does update the text value, but Chrome does not. So this patch will align with Chrome's behavior.
(In reply to Sunny He from comment #8) > Firefox does update the text value, but Chrome does not. So this patch will > align with Chrome's behavior. I don't think Firefox updates the text value per se. I think the behavior we observed is that Firefox would set the text content value of HTMLTitleElement before DOMNodeInserted event fires. In neither browsers, document's title was updated because the title element is ultimately removed.
Created attachment 383904 [details] Patch
Comment on attachment 383904 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383904&action=review > LayoutTests/fast/dom/Document/title-property-set-with-dom-event-svg.html:5 > + if (window.testRunner) { WebKit style is to not use curly braces around a single line statement but it’s okay to keep it like this for tests.
Comment on attachment 383904 [details] Patch Clearing flags on attachment: 383904 Committed r252667: <https://trac.webkit.org/changeset/252667>
All reviewed patches have been landed. Closing bug.
Comment on attachment 383904 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383904&action=review > Source/WebCore/dom/Document.cpp:1601 > + // insertBefore above may have ran scripts which removed m_titleElement ...may have run scripts... Also period at the end (we like comments to be full sentences). > Source/WebCore/dom/Document.cpp:1612 > + // appendChild above may have ran scripts which removed m_titleElement Same.
Committed r253096: <https://trac.webkit.org/changeset/253096>