Setting document.title reuses <title>'s textnode child

[Boris Zbarsky]

STEPS TO REPRODUCE:
1)  Load attached testcase
2)  See which alert comes up

EXPECTED RESULTS:
"textnode is not being reused"

ACTUAL RESULTS:
"Reusing textnode on title set"

The relevant spec draft text is at http://www.whatwg.org/specs/web-apps/current-work/multipage/dom.html#document.title step 4 of the "On setting" algorithm.

Opera and Gecko both follow the HTML5 draft here.

[Mark Rowe (bdash)]

There's no test case attached.  Boris, can you please attach the test case you refer to in your description of the bug?

[Boris Zbarsky]

Created attachment 38858 [details]
Testcase

Ah, sorry.  Not sure how I failed to attach that.

[Mark Rowe (bdash)]

<rdar://problem/7186473>

[Adam Barth]

You're blowing my mind Boris.  Will fix.

[Adrien Loison]

Created attachment 137235 [details]
Patch

[WebKit Review Bot]

Comment on attachment 137235 [details]
Patch

Attachment 137235 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12406665

New failing tests:
fast/dom/document-set-title-mutation-crash.html

[WebKit Review Bot]

Created attachment 137236 [details]
Archive of layout-test-results from ec2-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-02  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Kent Tamura]

Comment on attachment 137235 [details]
Patch

r- because of the EWS failure

[Chris Dumez]

Looks like this still applies according to the latest specification:
- https://html.spec.whatwg.org/multipage/dom.html#document.title
- https://dom.spec.whatwg.org/#dom-node-textcontent

This is caused by the following "optimization" in HTMLTitleElement::setText():
    if (!value.isEmpty() && hasOneChild() && is<Text>(*firstChild())) {
        downcast<Text>(*firstChild()).setData(value);
        return;
    }

[Chris Dumez]

Chrome and Firefox seem to agree with the specification.

[Chris Dumez]

Created attachment 283292 [details]
WIP Patch

[Chris Dumez]

Created attachment 283293 [details]
WIP Patch

[Chris Dumez]

Created attachment 283295 [details]
Patch

[Benjamin Poulain]

Comment on attachment 283295 [details]
Patch

Please also include a test for the SVG title element. We always find fishy bugs with SVG.
Bonus point for a test that raise the exception on setTitle() (I guess messing up with the tree in response to mutation events).

[Chris Dumez]

Created attachment 283298 [details]
Patch

[Chris Dumez]

(In reply to comment #14)
> Comment on attachment 283295 [details]
> Patch
> 
> Please also include a test for the SVG title element. We always find fishy
> bugs with SVG.
> Bonus point for a test that raise the exception on setTitle() (I guess
> messing up with the tree in response to mutation events).

I added a test for svn title before landing. I agree we should cover the exception case, I'll look into it tomorrow and we can land the test in a follow-up.

[WebKit Commit Bot]

Comment on attachment 283298 [details]
Patch

Clearing flags on attachment: 283298

Committed r203047: <http://trac.webkit.org/changeset/203047>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.