Summary: | Document.title setter does not work for SVG documents | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||
Component: | DOM | Assignee: | Chris Dumez <cdumez> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cdumez, darin, rniwa, sabouhallawa, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar, WebExposed | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Chris Dumez
2016-07-06 19:30:50 PDT
Created attachment 282983 [details]
Patch
Comment on attachment 282983 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=282983&action=review > Source/WebCore/dom/Document.cpp:1660 > + if (is<SVGSVGElement>(documentElement())) > + m_titleElement = childrenOfType<SVGTitleElement>(*documentElement()).first(); > + else { > + if (m_titleElement) { > + if (isHTMLDocument() || isXHTMLDocument()) > + m_titleElement = descendantsOfType<HTMLTitleElement>(*this).first(); > + } else > + m_titleElement = newTitleElement; > + } Why don't we fix SVGTitleElement::insertedInto and SVGTitleElement::removedFrom so that we can use the same code path for SVG and HTML documents? We already have code to call titleElementAdded/titleElementRemoved so we just need to check that its parent is the document element of the SVG document. > Source/WebCore/svg/SVGTitleElement.cpp:77 > + // which causes SVGTitleElement::childrenChanged(), which ends up Document::setTitle(). ends up *calling* Document::setTitle() or *invoking* Comment on attachment 282983 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=282983&action=review >> Source/WebCore/dom/Document.cpp:1660 >> + } > > Why don't we fix SVGTitleElement::insertedInto and SVGTitleElement::removedFrom so that we can use the same code path for SVG and HTML documents? > We already have code to call titleElementAdded/titleElementRemoved so we just need to check that its parent is the document element of the SVG document. Sadly, this is not that easy. I tried to do what you suggest but I cannot get all the tests to pass. Here one issue: - When adding a HTMLTitleElement to a SVGDocument's whose document element is a SVGSVGElement, we would expect this title to be ignored. However, if we move the is<SVGSVGElement>(documentElement()) from Document::updateTitleElement() to SVGTitleElement::insertedInto(), it no longer gets ignored. We could add a !is<SVGSVGElement>(documentElement()) check to HTMLTitleElement::insertedInto() but this would be uglier IMHO. Note that I also tried adding a (isHTMLDocument() || isXHTMLDocument()) check to HTMLTitleElement::insertedInto() but this caused another test to fail because the HTMLTitleElement should not get ignored if added to a SVGDocument whose document element is not a SVGSVGElement. Anyway, I suggest landing the patch as is for now. We can think if there is a better way to factor this later as it does not seem trivial. Created attachment 282991 [details]
Patch
Comment on attachment 282991 [details] Patch Clearing flags on attachment: 282991 Committed r202895: <http://trac.webkit.org/changeset/202895> All reviewed patches have been landed. Closing bug. |