Match newly-clarified spec on textarea defaultValue/value/child text content
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Aug 2017 20:49:52 +0000 (20:49 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Aug 2017 20:49:52 +0000 (20:49 +0000)
https://bugs.webkit.org/show_bug.cgi?id=173878

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

Re-sync WPT test from upstream and rebaseline to improve test coverage.

* web-platform-tests/html/semantics/forms/the-textarea-element/value-defaultValue-textContent-expected.txt:
* web-platform-tests/html/semantics/forms/the-textarea-element/value-defaultValue-textContent.html:

Source/WebCore:

Update HTMLTextArea.defaultValue to match align with other browsers and match the
latest HTML specification:
- https://html.spec.whatwg.org/#dom-textarea-defaultvalue

The defaultValue getter should return the child text content:
- https://dom.spec.whatwg.org/#concept-child-text-content
Our code was traversing all Text descendants, not just the children.

The defaultValue setter should act as the setter of the Element's textContent
IDL attribute. Previously, we had a custom logic that was only removing the
text children.

Test: imported/w3c/web-platform-tests/html/semantics/forms/the-textarea-element/value-defaultValue-textContent.html

* dom/ScriptElement.cpp:
(WebCore::ScriptElement::scriptContent const):
* dom/TextNodeTraversal.cpp:
(WebCore::TextNodeTraversal::childTextContent):
* dom/TextNodeTraversal.h:
* html/HTMLTextAreaElement.cpp:
(WebCore::HTMLTextAreaElement::defaultValue const):
(WebCore::HTMLTextAreaElement::setDefaultValue):
* html/HTMLTitleElement.cpp:
(WebCore::HTMLTitleElement::text const):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@220290 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/resources/import-expectations.json
LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/the-textarea-element/value-defaultValue-textContent-expected.txt
LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/the-textarea-element/value-defaultValue-textContent-xhtml-expected.txt [new file with mode: 0644]
LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/the-textarea-element/value-defaultValue-textContent-xhtml.xhtml [new file with mode: 0644]
LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/the-textarea-element/value-defaultValue-textContent.html
LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/the-textarea-element/w3c-import.log
Source/WebCore/ChangeLog
Source/WebCore/dom/ScriptElement.cpp
Source/WebCore/dom/TextNodeTraversal.cpp
Source/WebCore/dom/TextNodeTraversal.h
Source/WebCore/html/HTMLTextAreaElement.cpp
Source/WebCore/html/HTMLTitleElement.cpp

index 34e0919..93789af 100644 (file)
@@ -1,3 +1,15 @@
+2017-08-04  Chris Dumez  <cdumez@apple.com>
+
+        Match newly-clarified spec on textarea defaultValue/value/child text content
+        https://bugs.webkit.org/show_bug.cgi?id=173878
+
+        Reviewed by Darin Adler.
+
+        Re-sync WPT test from upstream and rebaseline to improve test coverage.
+
+        * web-platform-tests/html/semantics/forms/the-textarea-element/value-defaultValue-textContent-expected.txt:
+        * web-platform-tests/html/semantics/forms/the-textarea-element/value-defaultValue-textContent.html:
+
 2017-08-04  Brady Eidson  <beidson@apple.com>
 
         Enable ServiceWorkers at runtime for WebKitTestRunner.
index c8d9510..2633974 100644 (file)
     "web-platform-tests/html/semantics/embedded-content/the-object-element/object-events.html": "skip", 
     "web-platform-tests/html/semantics/forms/textfieldselection/select-event.html": "skip", 
     "web-platform-tests/html/semantics/forms/textfieldselection/textfieldselection-setRangeText.html": "skip", 
+    "web-platform-tests/html/semantics/forms/the-textarea-element": "import", 
     "web-platform-tests/html/semantics/interactive-elements/the-details-element/toggleEvent.html": "skip", 
     "web-platform-tests/html/semantics/interactive-elements/the-summary-element": "skip", 
     "web-platform-tests/html/semantics/links/links-created-by-a-and-area-elements": "skip", 
index be75c35..6290f75 100644 (file)
@@ -1,7 +1,14 @@
 
 PASS defaultValue and value are the empty string by default 
 PASS defaultValue and value are affected by setting textContent 
+PASS defaultValue and value are affected by setting nodeValue on a child text node 
+PASS defaultValue and value are affected by setting data on a child text node 
 PASS defaultValue and value are affected by textContent in combination with appending a text node 
+PASS defaultValue and value are affected by textContent in combination with appending a DocumentFragment 
+PASS defaultValue and value reflect child text content, not textContent 
+PASS Setting defaultValue wipes out any children, including elements (just like setting textContent) 
 PASS defaultValue and value treat CRLF differently 
+PASS value normalizes CRLF even spread over multiple text nodes 
 PASS tests for the value setter 
+PASS tests for U+0000 NULL 
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/the-textarea-element/value-defaultValue-textContent-xhtml-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/the-textarea-element/value-defaultValue-textContent-xhtml-expected.txt
new file mode 100644 (file)
index 0000000..fd1be19
--- /dev/null
@@ -0,0 +1,3 @@
+
+PASS defaultValue and value include CDATASection Text nodes 
+
diff --git a/LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/the-textarea-element/value-defaultValue-textContent-xhtml.xhtml b/LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/the-textarea-element/value-defaultValue-textContent-xhtml.xhtml
new file mode 100644 (file)
index 0000000..9462e94
--- /dev/null
@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="utf-8"?>
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+<title>textarea element value/defaultValue/textContent functionality</title>
+<link rel="author" title="Domenic Denicola" href="mailto:d@domenic.me"/>
+<link rel="help" href="https://html.spec.whatwg.org/multipage/#dom-textarea-value"/>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+</head>
+<body>
+<script>
+"use strict";
+
+test(() => {
+
+  const textarea = document.createElement("textarea");
+
+  textarea.appendChild(document.createCDATASection("foo bar baz"));
+  assert_equals(textarea.defaultValue, "foo bar baz", "the defaultValue should reflect the textContent");
+  assert_equals(textarea.value, "foo bar baz",
+    "changing the child text content should change the raw value, and subsequently the api value");
+
+}, "defaultValue and value include CDATASection Text nodes");
+</script>
+</body>
+</html>
index fa197c3..a1a405f 100644 (file)
@@ -1,5 +1,5 @@
 <!DOCTYPE HTML>
-<title>textarea element select() functionality</title>
+<title>textarea element value/defaultValue/textContent functionality</title>
 <link rel="author" title="Domenic Denicola" href="mailto:d@domenic.me">
 <link rel="help" href="https://html.spec.whatwg.org/multipage/#dom-textarea-value">
 <script src="/resources/testharness.js"></script>
@@ -20,8 +20,8 @@ test(() => {
 test(() => {
 
   const textarea = document.createElement("textarea");
-  textarea.textContent = "foo bar";
 
+  textarea.textContent = "foo bar";
   assert_equals(textarea.defaultValue, "foo bar", "the defaultValue should reflect the textContent");
   assert_equals(textarea.value, "foo bar",
     "changing the textContent should change the raw value, and subsequently the api value");
@@ -31,9 +31,33 @@ test(() => {
 test(() => {
 
   const textarea = document.createElement("textarea");
+
+  textarea.textContent = "some text";
+  textarea.firstChild.nodeValue = "foo bar";
+  assert_equals(textarea.defaultValue, "foo bar", "the defaultValue should reflect the textContent");
+  assert_equals(textarea.value, "foo bar",
+    "changing the textContent should change the raw value, and subsequently the api value");
+
+}, "defaultValue and value are affected by setting nodeValue on a child text node");
+
+test(() => {
+
+  const textarea = document.createElement("textarea");
+
+  textarea.textContent = "some text";
+  textarea.firstChild.data = "foo bar";
+  assert_equals(textarea.defaultValue, "foo bar", "the defaultValue should reflect the textContent");
+  assert_equals(textarea.value, "foo bar",
+    "changing the textContent should change the raw value, and subsequently the api value");
+
+}, "defaultValue and value are affected by setting data on a child text node");
+
+test(() => {
+
+  const textarea = document.createElement("textarea");
+
   textarea.textContent = "foo bar";
   textarea.appendChild(document.createTextNode(" baz"));
-
   assert_equals(textarea.defaultValue, "foo bar baz", "the defaultValue should reflect the textContent");
   assert_equals(textarea.value, "foo bar baz",
     "changing the textContent should change the raw value, and subsequently the api value");
@@ -43,8 +67,62 @@ test(() => {
 test(() => {
 
   const textarea = document.createElement("textarea");
-  textarea.textContent = "foo\r\nbar\rbaz\nqux";
+  textarea.textContent = "foo bar";
+
+  const frag = document.createDocumentFragment();
+  frag.appendChild(document.createTextNode(" baz"));
+  const el = document.createElement("span");
+  el.appendChild(document.createTextNode("qux?"));
+  frag.appendChild(el);
+  frag.appendChild(document.createTextNode(" fizz"));
+  textarea.appendChild(frag);
+
+  textarea.appendChild(document.createTextNode(" whee"));
+  assert_equals(textarea.defaultValue, "foo bar baz fizz whee", "the defaultValue should reflect the textContent");
+  assert_equals(textarea.value, "foo bar baz fizz whee",
+    "changing the textContent should change the raw value, and subsequently the api value");
+
+}, "defaultValue and value are affected by textContent in combination with appending a DocumentFragment");
+
+test(() => {
+
+  const textarea = document.createElement("textarea");
+  textarea.appendChild(document.createTextNode("foo bar"));
+
+  const child = document.createElement("span");
+  child.textContent = "baz";
+  textarea.appendChild(child);
+
+  assert_equals(textarea.textContent, "foo barbaz", "the textContent should have *all* the text content");
+  assert_equals(textarea.defaultValue, "foo bar", "the defaultValue should reflect the child text content");
+  assert_equals(textarea.value, "foo bar",
+    "changing the child text content should change the raw value, and subsequently the api value");
+
+}, "defaultValue and value reflect child text content, not textContent");
+
+test(() => {
+
+  const textarea = document.createElement("textarea");
+  textarea.appendChild(document.createTextNode("foo bar"));
+
+  const child = document.createElement("span");
+  child.textContent = "baz";
+  textarea.appendChild(child);
+
+  textarea.defaultValue = "foo";
+
+  assert_equals(textarea.childNodes.length, 1, "Only one child node should exist");
+  assert_equals(textarea.defaultValue, "foo", "the defaultValue should be the new text");
+  assert_equals(textarea.value, "foo", "the api value should be the new text");
+  assert_equals(textarea.textContent, "foo", "the textContent should be the new text");
 
+}, "Setting defaultValue wipes out any children, including elements (just like setting textContent)");
+
+test(() => {
+
+  const textarea = document.createElement("textarea");
+
+  textarea.textContent = "foo\r\nbar\rbaz\nqux";
   assert_equals(textarea.defaultValue, "foo\r\nbar\rbaz\nqux", "the defaultValue should reflect the textContent");
   assert_equals(textarea.value, "foo\nbar\nbaz\nqux", "The value property should normalize CRLF and CR to LF");
 
@@ -53,9 +131,20 @@ test(() => {
 test(() => {
 
   const textarea = document.createElement("textarea");
+
+  textarea.appendChild(document.createTextNode("foo\r"));
+  textarea.appendChild(document.createTextNode("\nbar\rbaz\nqux"));
+  assert_equals(textarea.defaultValue, "foo\r\nbar\rbaz\nqux", "the defaultValue should reflect the textContent");
+  assert_equals(textarea.value, "foo\nbar\nbaz\nqux", "The value property should normalize CRLF and CR to LF");
+
+}, "value normalizes CRLF even spread over multiple text nodes");
+
+test(() => {
+
+  const textarea = document.createElement("textarea");
+
   textarea.textContent = "foo";
   textarea.value = "baz";
-
   assert_equals(textarea.defaultValue, "foo", "setting the value property should not affect the defaultValue");
   assert_equals(textarea.textContent, "foo", "setting the value property should not affect the textContent");
   assert_equals(textarea.value, "baz",
@@ -68,4 +157,25 @@ test(() => {
   assert_equals(textarea.value, "", "setting the value property to null should result in an empty string");
 
 }, "tests for the value setter");
+
+test(() => {
+
+  const textarea = document.createElement("textarea");
+
+  textarea.defaultValue = "foo\0";
+  assert_equals(textarea.defaultValue, "foo\0", "defaultValue after setting defaultValue");
+  assert_equals(textarea.textContent, "foo\0", "textContent after setting defaultValue");
+  assert_equals(textarea.value, "foo\0", "value after setting defaultValue");
+
+  textarea.textContent = "bar\0";
+  assert_equals(textarea.defaultValue, "bar\0", "defaultValue after setting textContent");
+  assert_equals(textarea.textContent, "bar\0", "textContent after setting textContent");
+  assert_equals(textarea.value, "bar\0", "value after setting textContent");
+
+  textarea.value = "baz\0";
+  assert_equals(textarea.defaultValue, "bar\0", "defaultValue after setting value");
+  assert_equals(textarea.textContent, "bar\0", "textContent after setting value");
+  assert_equals(textarea.value, "baz\0", "value after setting value");
+
+}, "tests for U+0000 NULL");
 </script>
index 7a9ba2c..8174880 100644 (file)
@@ -18,6 +18,7 @@ List of files:
 /LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/the-textarea-element/textarea-newline-bidi-expected.html
 /LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/the-textarea-element/textarea-newline-bidi.html
 /LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/the-textarea-element/textarea-type.html
+/LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/the-textarea-element/value-defaultValue-textContent-xhtml.xhtml
 /LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/the-textarea-element/value-defaultValue-textContent.html
 /LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/the-textarea-element/wrap-reflect-1a-expected.html
 /LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/the-textarea-element/wrap-reflect-1a.html
index d9134d1..519eb6c 100644 (file)
@@ -1,3 +1,35 @@
+2017-08-04  Chris Dumez  <cdumez@apple.com>
+
+        Match newly-clarified spec on textarea defaultValue/value/child text content
+        https://bugs.webkit.org/show_bug.cgi?id=173878
+
+        Reviewed by Darin Adler.
+
+        Update HTMLTextArea.defaultValue to match align with other browsers and match the
+        latest HTML specification:
+        - https://html.spec.whatwg.org/#dom-textarea-defaultvalue
+
+        The defaultValue getter should return the child text content:
+        - https://dom.spec.whatwg.org/#concept-child-text-content
+        Our code was traversing all Text descendants, not just the children.
+
+        The defaultValue setter should act as the setter of the Element's textContent
+        IDL attribute. Previously, we had a custom logic that was only removing the
+        text children.
+
+        Test: imported/w3c/web-platform-tests/html/semantics/forms/the-textarea-element/value-defaultValue-textContent.html
+
+        * dom/ScriptElement.cpp:
+        (WebCore::ScriptElement::scriptContent const):
+        * dom/TextNodeTraversal.cpp:
+        (WebCore::TextNodeTraversal::childTextContent):
+        * dom/TextNodeTraversal.h:
+        * html/HTMLTextAreaElement.cpp:
+        (WebCore::HTMLTextAreaElement::defaultValue const):
+        (WebCore::HTMLTextAreaElement::setDefaultValue):
+        * html/HTMLTitleElement.cpp:
+        (WebCore::HTMLTitleElement::text const):
+
 2017-08-04  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         RenderImageResourceStyleImage::image() should return the nullImage() if the image is not available
index aa5f6bc..a833e28 100644 (file)
@@ -461,10 +461,7 @@ bool ScriptElement::isScriptForEventSupported() const
 
 String ScriptElement::scriptContent() const
 {
-    StringBuilder result;
-    for (auto* text = TextNodeTraversal::firstChild(m_element); text; text = TextNodeTraversal::nextSibling(*text))
-        result.append(text->data());
-    return result.toString();
+    return TextNodeTraversal::childTextContent(m_element);
 }
 
 void ScriptElement::ref()
index 112a284..b78ca79 100644 (file)
@@ -54,5 +54,13 @@ String contentsAsString(const Node& root)
     return String();
 }
 
+String childTextContent(const ContainerNode& root)
+{
+    StringBuilder result;
+    for (Text* text = TextNodeTraversal::firstChild(root); text; text = TextNodeTraversal::nextSibling(*text))
+        result.append(text->data());
+    return result.toString();
+}
+
 }
 }
index b520e6b..a159369 100644 (file)
@@ -56,6 +56,7 @@ Text* nextSibling(const Node&);
 String contentsAsString(const Node&);
 String contentsAsString(const ContainerNode&);
 void appendContents(const ContainerNode&, StringBuilder& result);
+String childTextContent(const ContainerNode&);
 
 }
 
index 7306749..e3a0219 100644 (file)
@@ -404,30 +404,12 @@ void HTMLTextAreaElement::setValueCommon(const String& newValue)
 
 String HTMLTextAreaElement::defaultValue() const
 {
-    return TextNodeTraversal::contentsAsString(*this);
+    return TextNodeTraversal::childTextContent(*this);
 }
 
 void HTMLTextAreaElement::setDefaultValue(const String& defaultValue)
 {
-    Ref<HTMLTextAreaElement> protectedThis(*this);
-
-    // To preserve comments, remove only the text nodes, then add a single text node.
-    Vector<Ref<Text>> textNodes;
-    for (Text* textNode = TextNodeTraversal::firstChild(*this); textNode; textNode = TextNodeTraversal::nextSibling(*textNode))
-        textNodes.append(*textNode);
-
-    for (auto& textNode : textNodes)
-        removeChild(textNode.get());
-
-    // Normalize line endings.
-    String value = defaultValue;
-    value.replace("\r\n", "\n");
-    value.replace('\r', '\n');
-
-    insertBefore(document().createTextNode(value), firstChild());
-
-    if (!m_isDirty)
-        setNonDirtyValue(value);
+    setTextContent(defaultValue);
 }
 
 String HTMLTextAreaElement::validationMessage() const
index 81ba8de..7d3ea55 100644 (file)
@@ -74,10 +74,7 @@ void HTMLTitleElement::childrenChanged(const ChildChange& change)
 
 String HTMLTitleElement::text() const
 {
-    StringBuilder result;
-    for (Text* text = TextNodeTraversal::firstChild(*this); text; text = TextNodeTraversal::nextSibling(*text))
-        result.append(text->data());
-    return result.toString();
+    return TextNodeTraversal::childTextContent(*this);
 }
 
 StringWithDirection HTMLTitleElement::computedTextWithDirection()