Fix null handling of HTMLSelectElement.value attribute
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Jul 2016 16:37:40 +0000 (16:37 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Jul 2016 16:37:40 +0000 (16:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=159925

Reviewed by Benjamin Poulain.

Source/WebCore:

Fix null handling of HTMLSelectElement.value attribute:
- https://html.spec.whatwg.org/multipage/forms.html#htmlselectelement

We were treating null as the null String which would end up setting
selectedIndex to -1. However, we should treat null as the String "null"
which would set the selectedIndex to the index of the <option> element
whose value is "null".

Firefox and Chrome match the specification.

Test: fast/dom/HTMLSelectElement/value-null-handling.html

* html/HTMLSelectElement.cpp:
(WebCore::HTMLSelectElement::setValue):
* html/HTMLSelectElement.idl:

LayoutTests:

Add layout test coverage. I have verified that this test is passing in
both Firefox and Chrome.

* fast/dom/HTMLSelectElement/value-null-handling-expected.txt: Added.
* fast/dom/HTMLSelectElement/value-null-handling.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/dom/HTMLSelectElement/value-null-handling-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/HTMLSelectElement/value-null-handling.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLSelectElement.cpp
Source/WebCore/html/HTMLSelectElement.idl

index 23bbb04..e09822b 100644 (file)
@@ -1,3 +1,16 @@
+2016-07-20  Chris Dumez  <cdumez@apple.com>
+
+        Fix null handling of HTMLSelectElement.value attribute
+        https://bugs.webkit.org/show_bug.cgi?id=159925
+
+        Reviewed by Benjamin Poulain.
+
+        Add layout test coverage. I have verified that this test is passing in
+        both Firefox and Chrome.
+
+        * fast/dom/HTMLSelectElement/value-null-handling-expected.txt: Added.
+        * fast/dom/HTMLSelectElement/value-null-handling.html: Added.
+
 2016-07-20  Ryan Haddad  <ryanhaddad@apple.com>
 
         Consolidating duplicate TestExpectations for fast/images/animated-png.html.
diff --git a/LayoutTests/fast/dom/HTMLSelectElement/value-null-handling-expected.txt b/LayoutTests/fast/dom/HTMLSelectElement/value-null-handling-expected.txt
new file mode 100644 (file)
index 0000000..fa89ba2
--- /dev/null
@@ -0,0 +1,21 @@
+Tests null handling of HTMLSelectElement.value attribute
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+select.value = 'a'
+PASS select.value is "a"
+PASS select.selectedIndex is 0
+select.value = null
+PASS select.value is "null"
+PASS select.selectedIndex is 1
+select.value = 'a'
+PASS select.value is "a"
+PASS select.selectedIndex is 0
+select.value = null
+PASS select.value is ""
+PASS select.selectedIndex is -1
+PASS successfullyParsed is true
+
+TEST COMPLETE
+  
diff --git a/LayoutTests/fast/dom/HTMLSelectElement/value-null-handling.html b/LayoutTests/fast/dom/HTMLSelectElement/value-null-handling.html
new file mode 100644 (file)
index 0000000..8cef0d8
--- /dev/null
@@ -0,0 +1,37 @@
+<DOCTYPE html>
+<html>
+<body>
+<script src="../../../resources/js-test-pre.js"></script>
+<select id="testSelectWithNull">
+<option value="a"/>
+<option value="null"/>
+</select>
+<select id="testSelectWithoutNull">
+<option value="a"/>
+<option value="b"/>
+</select>
+<script>
+description("Tests null handling of HTMLSelectElement.value attribute");
+
+var select = document.getElementById("testSelectWithNull");
+evalAndLog("select.value = 'a'");
+shouldBeEqualToString("select.value", "a");
+shouldBe("select.selectedIndex", "0");
+
+evalAndLog("select.value = null");
+shouldBeEqualToString("select.value", "null");
+shouldBe("select.selectedIndex", "1");
+
+select = document.getElementById("testSelectWithoutNull");
+evalAndLog("select.value = 'a'");
+shouldBeEqualToString("select.value", "a");
+shouldBe("select.selectedIndex", "0");
+
+evalAndLog("select.value = null");
+shouldBeEqualToString("select.value", "");
+shouldBe("select.selectedIndex", "-1");
+
+</script>
+<script src="../../../resources/js-test-post.js"></script>
+</body>
+</html>
index 00e9942..705eaa2 100644 (file)
@@ -1,5 +1,28 @@
 2016-07-20  Chris Dumez  <cdumez@apple.com>
 
+        Fix null handling of HTMLSelectElement.value attribute
+        https://bugs.webkit.org/show_bug.cgi?id=159925
+
+        Reviewed by Benjamin Poulain.
+
+        Fix null handling of HTMLSelectElement.value attribute:
+        - https://html.spec.whatwg.org/multipage/forms.html#htmlselectelement
+
+        We were treating null as the null String which would end up setting
+        selectedIndex to -1. However, we should treat null as the String "null"
+        which would set the selectedIndex to the index of the <option> element
+        whose value is "null".
+
+        Firefox and Chrome match the specification.
+
+        Test: fast/dom/HTMLSelectElement/value-null-handling.html
+
+        * html/HTMLSelectElement.cpp:
+        (WebCore::HTMLSelectElement::setValue):
+        * html/HTMLSelectElement.idl:
+
+2016-07-20  Chris Dumez  <cdumez@apple.com>
+
         PostResolutionCallbackDisabler can resume pending requests while a ResourceLoadSuspender is alive
         https://bugs.webkit.org/show_bug.cgi?id=159962
         <rdar://problem/21439264>
index 28719dd..dd40123 100644 (file)
@@ -262,14 +262,8 @@ String HTMLSelectElement::value() const
     return emptyString();
 }
 
-void HTMLSelectElement::setValue(const String &value)
+void HTMLSelectElement::setValue(const Stringvalue)
 {
-    // We clear the previously selected option(s) when needed, to guarantee calling setSelectedIndex() only once.
-    if (value.isNull()) {
-        setSelectedIndex(-1);
-        return;
-    }
-
     // Find the option with value() matching the given parameter and make it the current selection.
     unsigned optionIndex = 0;
     for (auto* item : listItems()) {
index 1da6a82..044692f 100644 (file)
@@ -68,7 +68,7 @@
     readonly attribute HTMLCollection selectedOptions;
     attribute long selectedIndex;
 
-    [TreatNullAs=LegacyNullString] attribute DOMString value; // FIXME: This should not use [TreatNullAs=LegacyNullString].
+    attribute DOMString value;
 
     readonly attribute boolean willValidate;
     readonly attribute ValidityState validity;