Bug 159503 – Document.title setter does not work for SVG documents

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Chris Dumez]

Document.title setter does not work for SVG documents:
https://html.spec.whatwg.org/multipage/dom.html#document.title

It works in Firefox and Chrome.

Test case:
http://w3c-test.org/html/dom/documents/dom-tree-accessors/document.title-09.html

[Radar WebKit Bug Importer]

<rdar://problem/27212313>

[Chris Dumez]

Created attachment 282983 [details]
Patch

[Ryosuke Niwa]

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*

[Chris Dumez]

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.

[Chris Dumez]

Created attachment 282991 [details]
Patch

[Chris Dumez]

Comment on attachment 282991 [details]
Patch

Clearing flags on attachment: 282991

Committed r202895: <http://trac.webkit.org/changeset/202895>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.