HTMLOptionsCollection's namedItem and name getter should return the first item
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 25 Apr 2013 18:32:06 +0000 (18:32 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 25 Apr 2013 18:32:06 +0000 (18:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=115150

Reviewed by Andreas Kling.

Source/WebCore:

Following the resolution in http://lists.whatwg.org/pipermail/whatwg-whatwg.org/2012-December/038355.html,
the spefication has been updated to only return the first item when there are multiple items of the same name
in HTMLOptionsCollection; this new behavior matches that of Firefox and Opera (Presto).

Implement this new behavior to remove the custom binding code and use the fast path in namedItem and name
getter of HTMLOptionsCollection. (Obtaining all items for a given name is expensive!).

Tests: fast/dom/HTMLSelectElement/named-options.html
       fast/dom/html-collections-named-getter.html

* bindings/js/JSHTMLOptionsCollectionCustom.cpp:
(WebCore): Removed the custom bindings for name getter and namedItem.
* html/HTMLOptionsCollection.idl:

LayoutTests:

Changed the expectations of the tests.

* fast/dom/HTMLSelectElement/named-options-expected.txt:
* fast/dom/HTMLSelectElement/script-tests/named-options.js:
* fast/dom/html-collections-named-getter-expected.txt:
* fast/dom/html-collections-named-getter.html:

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

LayoutTests/ChangeLog
LayoutTests/fast/dom/HTMLSelectElement/named-options-expected.txt
LayoutTests/fast/dom/HTMLSelectElement/script-tests/named-options.js
LayoutTests/fast/dom/html-collections-named-getter-expected.txt
LayoutTests/fast/dom/html-collections-named-getter.html
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSHTMLOptionsCollectionCustom.cpp
Source/WebCore/html/HTMLOptionsCollection.idl

index a82c0236f19b1f281bfe53258aedc988e1687c16..9490c9e15adbf574c76b89ef1e9c48e2aa85ae64 100644 (file)
@@ -1,3 +1,17 @@
+2013-04-25  Ryosuke Niwa  <rniwa@webkit.org>
+
+        HTMLOptionsCollection's namedItem and name getter should return the first item
+        https://bugs.webkit.org/show_bug.cgi?id=115150
+
+        Reviewed by Andreas Kling.
+
+        Changed the expectations of the tests.
+
+        * fast/dom/HTMLSelectElement/named-options-expected.txt:
+        * fast/dom/HTMLSelectElement/script-tests/named-options.js:
+        * fast/dom/html-collections-named-getter-expected.txt:
+        * fast/dom/html-collections-named-getter.html:
+
 2013-04-25  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: ConsoleMessage should include line and column number where possible
index a66750442131379a53d8e9b4fa0a2e8c15a45118..1695e530ca0fcabbddd2875c92dc126858c333ce 100644 (file)
@@ -9,15 +9,11 @@ PASS select1.namedItem('test').value is "Value"
 Confirm that the option named 'test' is accessible from the options collection
 PASS select1.options.namedItem('test').toString() is "[object HTMLOptionElement]"
 PASS select1.options.namedItem('test').value is "Value"
-Confirm that both options named 'test' are accessible from the options collection
-PASS select2.options.namedItem('test').length is 2
-PASS select2.options.namedItem('test').toString() is "[object NodeList]"
-PASS select2.options.namedItem('test')[0].value is "Value1"
-PASS select2.options.namedItem('test')[1].value is "Value2"
-PASS select2.options.test.length is 2
-PASS select2.options.test.toString() is "[object NodeList]"
-PASS select2.options.test[0].value is "Value1"
-PASS select2.options.test[1].value is "Value2"
+Confirm that the options collection returns the first option when there are multiple options named 'test'
+PASS select2.namedItem('test').toString() is "[object HTMLOptionElement]"
+PASS select2.namedItem('test').value is "Value1"
+PASS select2.options.test.toString() is "[object HTMLOptionElement]"
+PASS select2.options.test.value is "Value1"
 PASS successfullyParsed is true
 
 TEST COMPLETE
index b52a1208bbc29420ba5d30c5b0cff08a9a7140a2..8b5c289bb021beb3bf273ef55d8f8615423ce6ff 100644 (file)
@@ -16,16 +16,12 @@ debug("Confirm that the option named 'test' is accessible from the options colle
 shouldBeEqualToString("select1.options.namedItem('test').toString()", "[object HTMLOptionElement]");
 shouldBeEqualToString("select1.options.namedItem('test').value", "Value");
 
-debug("Confirm that both options named 'test' are accessible from the options collection");
-shouldBe("select2.options.namedItem('test').length", "2");
-shouldBeEqualToString("select2.options.namedItem('test').toString()", "[object NodeList]");
-shouldBeEqualToString("select2.options.namedItem('test')[0].value", "Value1");
-shouldBeEqualToString("select2.options.namedItem('test')[1].value", "Value2");
+debug("Confirm that the options collection returns the first option when there are multiple options named 'test'");
+shouldBeEqualToString("select2.namedItem('test').toString()", "[object HTMLOptionElement]");
+shouldBeEqualToString("select2.namedItem('test').value", "Value1");
 
-shouldBe("select2.options.test.length", "2");
-shouldBeEqualToString("select2.options.test.toString()", "[object NodeList]");
-shouldBeEqualToString("select2.options.test[0].value", "Value1");
-shouldBeEqualToString("select2.options.test[1].value", "Value2");
+shouldBeEqualToString("select2.options.test.toString()", "[object HTMLOptionElement]");
+shouldBeEqualToString("select2.options.test.value", "Value1");
 
 // Clean up after ourselves
 document.body.removeChild(select1);
index a189504b28d7c974e16e3fc8f479128c0ef1abb0..51a4ac4c6145a3027d2e7e46ae3f3cbc8c31634d 100644 (file)
@@ -37,10 +37,7 @@ PASS elements = [createElementWithId('option', 'foo'), createElementWithId('opti
      select.appendChild(elements[0]); select.options.length is 1
 PASS select.options['foo'] is elements[0]
 PASS select.appendChild(elements[1]); select.options.length is 2
-PASS select.options['foo'].toString() is '[object NodeList]'
-PASS select.options['foo'].length is 2
-PASS select.options['foo'][0] is elements[0]
-PASS select.options['foo'][1] is elements[1]
+PASS select.options['foo'] is elements[0]
 PASS select.removeChild(elements[0]); select.options['foo'] is elements[1]
 PASS select.innerHTML = ''; select.options.length is 0
 PASS removeTestElements(); form.elements.length is 0
index 2fa9e3a697f4367e9d961ae038f2d0bbe0e8b5de..befd18e0593ee2ecf643b95aac37a4f84de9f81f 100644 (file)
@@ -65,10 +65,7 @@ shouldBe("elements = [createElementWithId('option', 'foo'), createElementWithId(
     + "     select.appendChild(elements[0]); select.options.length", "1");
 shouldBe("select.options['foo']", "elements[0]");
 shouldBe("select.appendChild(elements[1]); select.options.length", "2");
-shouldBe("select.options['foo'].toString()", "'[object NodeList]'");
-shouldBe("select.options['foo'].length", "2");
-shouldBe("select.options['foo'][0]", "elements[0]");
-shouldBe("select.options['foo'][1]", "elements[1]");
+shouldBe("select.options['foo']", "elements[0]");
 shouldBe("select.removeChild(elements[0]); select.options['foo']", "elements[1]");
 shouldBe("select.innerHTML = ''; select.options.length", "0");
 shouldBe("removeTestElements(); form.elements.length", "0");
index 2b229f331f05731831dafe1c498c45f5b708d516..66148d25487375c5baaa8f1dadc23c5e9f7961e0 100644 (file)
@@ -1,3 +1,24 @@
+2013-04-25  Ryosuke Niwa  <rniwa@webkit.org>
+
+        HTMLOptionsCollection's namedItem and name getter should return the first item
+        https://bugs.webkit.org/show_bug.cgi?id=115150
+
+        Reviewed by Andreas Kling.
+
+        Following the resolution in http://lists.whatwg.org/pipermail/whatwg-whatwg.org/2012-December/038355.html,
+        the spefication has been updated to only return the first item when there are multiple items of the same name
+        in HTMLOptionsCollection; this new behavior matches that of Firefox and Opera (Presto).
+
+        Implement this new behavior to remove the custom binding code and use the fast path in namedItem and name
+        getter of HTMLOptionsCollection. (Obtaining all items for a given name is expensive!).
+
+        Tests: fast/dom/HTMLSelectElement/named-options.html
+               fast/dom/html-collections-named-getter.html
+
+        * bindings/js/JSHTMLOptionsCollectionCustom.cpp:
+        (WebCore): Removed the custom bindings for name getter and namedItem.
+        * html/HTMLOptionsCollection.idl:
+
 2013-04-25  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: ConsoleMessage should include line and column number where possible
index 48d23649a79ff6e9a769b20bb4ca32e9541bc4b1..47b338dbcb620ebec14375ac0e70416d52e111a9 100644 (file)
@@ -37,38 +37,6 @@ using namespace JSC;
 
 namespace WebCore {
 
-static JSValue getNamedItems(ExecState* exec, JSHTMLOptionsCollection* collection, PropertyName propertyName)
-{
-    Vector<RefPtr<Node> > namedItems;
-    const AtomicString& name = propertyNameToAtomicString(propertyName);
-    collection->impl()->namedItems(name, namedItems);
-
-    if (namedItems.isEmpty())
-        return jsUndefined();
-    if (namedItems.size() == 1)
-        return toJS(exec, collection->globalObject(), namedItems[0].get());
-
-    // FIXME: HTML5 specifies that this should be a LiveNodeList.
-    return toJS(exec, collection->globalObject(), StaticNodeList::adopt(namedItems).get());
-}
-
-bool JSHTMLOptionsCollection::canGetItemsForName(ExecState*, HTMLOptionsCollection* collection, PropertyName propertyName)
-{
-    return collection->hasNamedItem(propertyNameToAtomicString(propertyName));
-}
-
-JSValue JSHTMLOptionsCollection::nameGetter(ExecState* exec, JSValue slotBase, PropertyName propertyName)
-{
-    JSHTMLOptionsCollection* thisObj = jsCast<JSHTMLOptionsCollection*>(asObject(slotBase));
-    return getNamedItems(exec, thisObj, propertyName);
-}
-
-JSValue JSHTMLOptionsCollection::namedItem(ExecState* exec)
-{
-    JSValue value = getNamedItems(exec, this, Identifier(exec, exec->argument(0).toString(exec)->value(exec)));
-    return value.isUndefined() ? jsNull() : value;
-}
-
 void JSHTMLOptionsCollection::setLength(ExecState* exec, JSValue value)
 {
     HTMLOptionsCollection* imp = static_cast<HTMLOptionsCollection*>(impl());
index 577697722cb2c1d12aa2ebf6da4f608fbf64992f..d7d77d7ba03152daec7f4d53b2cf484530dbc7f8 100644 (file)
 [
     JSGenerateToNativeObject,
     CustomIndexedSetter,
-    NamedGetter,
     GenerateIsReachable=ImplOwnerNodeRoot,
 ] interface HTMLOptionsCollection : HTMLCollection {
     attribute long selectedIndex;
     [CustomSetter] attribute unsigned long length
         setter raises (DOMException);
 
-    [Custom] Node namedItem(in [Optional=DefaultIsUndefined] DOMString name);
+    Node namedItem(in [Optional=DefaultIsUndefined] DOMString name);
 
     [Custom] void add(in [Optional=DefaultIsUndefined] HTMLOptionElement option, 
                       in [Optional] unsigned long index)