Bug 148851

Summary: script.text shouldn't include text from non-direct children of the script element
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: DOMAssignee: Keith Rollin <krollin>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, cdumez, commit-queue, esprehn+autocc, kangil.han, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews102 for mac-mavericks
none
Archive of layout-test-results from ews106 for mac-mavericks-wk2
none
Patch
none
Patch
none
Patch none

Description Ryosuke Niwa 2015-09-04 16:49:20 PDT
See https://html.spec.whatwg.org/multipage/scripting.html#dom-script-text

HTMLScriptElement.prototype.text must return the concatenation of the data of Text nodes that are its direct children,
and must not include texts of other node types or non-direct child Text nodes.

This bug was found by the newly added test:
LayoutTests/http/tests/w3c/html/semantics/scripting-1/the-script-element/script-text.html
Comment 1 Radar WebKit Bug Importer 2015-09-04 16:49:36 PDT
<rdar://problem/22587759>
Comment 2 Keith Rollin 2015-10-07 11:35:38 PDT
Created attachment 262618 [details]
Patch
Comment 3 Chris Dumez 2015-10-07 11:46:29 PDT
Comment on attachment 262618 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=262618&action=review

> Source/WebCore/ChangeLog:5
> +

We usually put the radar link under the bugzilla one when there is one.

> Source/WebCore/ChangeLog:8
> +        Don't include text from non-direct children in script.text.

Please provide a link to the spec that says that it is the right thing to do. Also please indicate the behavior of Chrome and Firefox to make sure this change is web-compatible.

> Source/WebCore/dom/ScriptElement.cpp:386
> +    for (Text* text = TextNodeTraversal::firstChild(m_element); text; text = TextNodeTraversal::nextSibling(*text))

auto*

> LayoutTests/imported/w3c/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=148851

We usually put the radar link under the bugzilla one when there is one.
Comment 4 Build Bot 2015-10-07 12:08:22 PDT
Comment on attachment 262618 [details]
Patch

Attachment 262618 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/254840

New failing tests:
fast/dom/script-getElementById-during-insertion.html
Comment 5 Build Bot 2015-10-07 12:08:26 PDT
Created attachment 262622 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 6 Build Bot 2015-10-07 12:14:04 PDT
Comment on attachment 262618 [details]
Patch

Attachment 262618 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/254854

New failing tests:
fast/dom/script-getElementById-during-insertion.html
Comment 7 Build Bot 2015-10-07 12:14:08 PDT
Created attachment 262623 [details]
Archive of layout-test-results from ews106 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 8 Keith Rollin 2015-10-07 15:56:51 PDT
Created attachment 262650 [details]
Patch
Comment 9 Chris Dumez 2015-10-07 16:24:00 PDT
Comment on attachment 262650 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=262650&action=review

r=me with nit fix.

> LayoutTests/fast/dom/script-subtext-in-script-elements.html:11
> +    <div id="result">PASS</div>

nit: I would mark this as FAIL initially...

> LayoutTests/fast/dom/script-subtext-in-script-elements.html:17
> +        script.appendChild(document.createTextNode(""));

... then set it to "PASS" here to make sure the script is executed.
Comment 10 Keith Rollin 2015-10-07 16:59:28 PDT
Created attachment 262658 [details]
Patch
Comment 11 Chris Dumez 2015-10-07 17:01:26 PDT
Comment on attachment 262658 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=262658&action=review

> LayoutTests/fast/dom/script-subtext-in-script-elements.html:16
> +        document.getElementById('result').innerHTML = 'PASS';

Er, I meant in the script you're inserting.

> LayoutTests/fast/dom/script-subtext-in-script-elements.html:19
> +        script.appendChild(document.createTextNode(""));

script.appendChild(document.createTextNode("document.getElementById('result').innerHTML = 'PASS'"));
Comment 12 Keith Rollin 2015-10-07 17:30:45 PDT
Created attachment 262660 [details]
Patch
Comment 13 WebKit Commit Bot 2015-10-07 19:26:45 PDT
Comment on attachment 262660 [details]
Patch

Clearing flags on attachment: 262660

Committed r190703: <http://trac.webkit.org/changeset/190703>
Comment 14 WebKit Commit Bot 2015-10-07 19:26:52 PDT
All reviewed patches have been landed.  Closing bug.