LayoutTests:
authoradele <adele@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 17 Jun 2006 15:37:58 +0000 (15:37 +0000)
committeradele <adele@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 17 Jun 2006 15:37:58 +0000 (15:37 +0000)
        Reviewed by Darin.

        Test cases for
        <http://bugzilla.opendarwin.org/show_bug.cgi?id=6282>

        * fast/dom/select-selectedIndex-multiple.html: Added.
        * fast/dom/select-selectedIndex.html: Added.

WebCore:

        Reviewed by Darin.

        Fix for http://bugzilla.opendarwin.org/show_bug.cgi?id=6282:
        Adding new Option with new Option(text, value, defaultSelected, selected) fails to update selectedIndex

        Update selectedIndex when a new option is added using javascript.

        * bindings/js/kjs_html.cpp:
        (KJS::JSHTMLSelectCollection::put):
        * html/HTMLSelectElement.cpp:
        (WebCore::HTMLSelectElement::setSelectedIndex):
        (WebCore::HTMLSelectElement::setOption):
        (WebCore::HTMLSelectElement::setLength):
        * html/HTMLSelectElement.h:

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

LayoutTests/ChangeLog
LayoutTests/fast/dom/select-selectedIndex-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/select-selectedIndex-multiple-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/select-selectedIndex-multiple.html [new file with mode: 0644]
LayoutTests/fast/dom/select-selectedIndex.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/bindings/js/kjs_html.cpp
WebCore/html/HTMLSelectElement.cpp
WebCore/html/HTMLSelectElement.h

index e04c3cb839156e3bc77986bb119b4b08a66aa00d..9d0359568c6048274f995036f75163f228adedbf 100644 (file)
@@ -1,3 +1,13 @@
+2006-06-17  Rob Buis  <buis@kde.org>
+
+        Reviewed by Darin.
+
+        Test cases for
+        <http://bugzilla.opendarwin.org/show_bug.cgi?id=6282>
+
+        * fast/dom/select-selectedIndex-multiple.html: Added.
+        * fast/dom/select-selectedIndex.html: Added.
+
 2006-06-16  Adele Peterson  <adele@apple.com>
 
         Reviewed by Maciej.
diff --git a/LayoutTests/fast/dom/select-selectedIndex-expected.txt b/LayoutTests/fast/dom/select-selectedIndex-expected.txt
new file mode 100644 (file)
index 0000000..cbb4f62
--- /dev/null
@@ -0,0 +1,30 @@
+
+PASS mySelect.options.length is 2
+PASS mySelect.selectedIndex is 1
+PASS mySelect.options.length is 5
+PASS mySelect.selectedIndex is 1
+PASS mySelect.options.length is 2
+PASS mySelect.selectedIndex is 1
+PASS mySelect.options.length is 1
+PASS mySelect.selectedIndex is -1
+PASS mySelect.options.length is 2
+PASS mySelect.selectedIndex is 1
+PASS mySelect.options.length is 0
+PASS mySelect.selectedIndex is -1
+PASS mySelect.options.length is 0
+PASS mySelect.selectedIndex is -1
+PASS mySelect.options.length is 0
+PASS mySelect.selectedIndex is -1
+PASS mySelect.options.length is 2
+PASS mySelect.selectedIndex is 1
+PASS mySelect.options.length is 11
+PASS mySelect.selectedIndex is 10
+PASS mySelect.options.length is 11
+PASS mySelect.selectedIndex is 10
+PASS mySelect.options.length is 10
+PASS mySelect.selectedIndex is -1
+PASS mySelect.options.length is 10
+PASS mySelect.selectedIndex is -1
+PASS mySelect.options.length is 10
+PASS mySelect.selectedIndex is -1
+
diff --git a/LayoutTests/fast/dom/select-selectedIndex-multiple-expected.txt b/LayoutTests/fast/dom/select-selectedIndex-multiple-expected.txt
new file mode 100644 (file)
index 0000000..01eb767
--- /dev/null
@@ -0,0 +1,30 @@
+
+PASS mySelect.options.length is 2
+PASS mySelect.selectedIndex is 0
+PASS mySelect.options.length is 5
+PASS mySelect.selectedIndex is 0
+PASS mySelect.options.length is 2
+PASS mySelect.selectedIndex is 0
+PASS mySelect.options.length is 1
+PASS mySelect.selectedIndex is 0
+PASS mySelect.options.length is 2
+PASS mySelect.selectedIndex is 0
+PASS mySelect.options.length is 0
+PASS mySelect.selectedIndex is -1
+PASS mySelect.options.length is 0
+PASS mySelect.selectedIndex is -1
+PASS mySelect.options.length is 0
+PASS mySelect.selectedIndex is -1
+PASS mySelect.options.length is 2
+PASS mySelect.selectedIndex is 0
+PASS mySelect.options.length is 11
+PASS mySelect.selectedIndex is 0
+PASS mySelect.options.length is 11
+PASS mySelect.selectedIndex is 0
+PASS mySelect.options.length is 10
+PASS mySelect.selectedIndex is 0
+PASS mySelect.options.length is 10
+PASS mySelect.selectedIndex is 0
+PASS mySelect.options.length is 10
+PASS mySelect.selectedIndex is 0
+
diff --git a/LayoutTests/fast/dom/select-selectedIndex-multiple.html b/LayoutTests/fast/dom/select-selectedIndex-multiple.html
new file mode 100644 (file)
index 0000000..abbf310
--- /dev/null
@@ -0,0 +1,83 @@
+<select multiple id="test" size="3">
+</select>
+<div id="console"></div>
+<script src="../js/resources/js-test-pre.js"></script>
+<script>
+function reset(mySelect) {
+  mySelect.length = 0;
+  mySelect.options[mySelect.length] = new Option("one", "value", true, true);
+  mySelect.options[mySelect.length] = new Option("two", "value", true, true);
+}
+
+var mySelect = document.getElementById("test");
+reset(mySelect);
+
+mySelect.options.length = -1;
+shouldBe("mySelect.options.length", "2");
+shouldBe("mySelect.selectedIndex", "0");
+
+// 1) setting length to a larger length
+mySelect.options.length = 5;
+shouldBe("mySelect.options.length", "5");
+shouldBe("mySelect.selectedIndex", "0");
+
+// 2) setting length to a smaller length
+mySelect.options.length = 2;
+shouldBe("mySelect.options.length", "2");
+shouldBe("mySelect.selectedIndex", "0");
+
+mySelect.options.length = 1;
+shouldBe("mySelect.options.length", "1");
+shouldBe("mySelect.selectedIndex", "0");
+reset(mySelect);
+
+// 3) setting length to the same length
+mySelect.options.length = 2;
+shouldBe("mySelect.options.length", "2");
+shouldBe("mySelect.selectedIndex", "0");
+
+// 4) setting length to non-integer values (null,
+// undefined, non-numeric string, floating point number)
+mySelect.options.length = null;
+shouldBe("mySelect.options.length", "0");
+shouldBe("mySelect.selectedIndex", "-1");
+reset(mySelect);
+
+mySelect.options.length = undefined;
+shouldBe("mySelect.options.length", "0");
+shouldBe("mySelect.selectedIndex", "-1");
+reset(mySelect);
+
+mySelect.options.length = "apple";
+shouldBe("mySelect.options.length", "0");
+shouldBe("mySelect.selectedIndex", "-1");
+reset(mySelect);
+
+mySelect.options.length = 2.1;
+shouldBe("mySelect.options.length", "2");
+shouldBe("mySelect.selectedIndex", "0");
+
+// 5) setting an element by index past the end of the current list
+mySelect.options[10] = new Option("ten", "value", true, true);
+shouldBe("mySelect.options.length", "11");
+shouldBe("mySelect.selectedIndex", "0");
+
+// 6) setting an existing element by index
+mySelect.options[10] = mySelect.options[10];
+shouldBe("mySelect.options.length", "11");
+shouldBe("mySelect.selectedIndex", "0");
+
+// 7) trying to set an element that's not an option
+// element (null, undefined, other kinds of objects, other kinds of elements)
+mySelect.options[10] = null;
+shouldBe("mySelect.options.length", "10");
+shouldBe("mySelect.selectedIndex", "0");
+mySelect.options[10] = undefined;
+shouldBe("mySelect.options.length", "10");
+shouldBe("mySelect.selectedIndex", "0");
+mySelect.options[10] = mySelect;
+shouldBe("mySelect.options.length", "10");
+shouldBe("mySelect.selectedIndex", "0");
+
+</script>
+
diff --git a/LayoutTests/fast/dom/select-selectedIndex.html b/LayoutTests/fast/dom/select-selectedIndex.html
new file mode 100644 (file)
index 0000000..43905eb
--- /dev/null
@@ -0,0 +1,82 @@
+<select id="test" size="3">
+</select>
+<div id="console"></div>
+<script src="../js/resources/js-test-pre.js"></script>
+<script>
+function reset(mySelect) {
+  mySelect.length = 0;
+  mySelect.options[mySelect.length] = new Option("one", "value", true, true);
+  mySelect.options[mySelect.length] = new Option("two", "value", true, true);
+}
+
+var mySelect = document.getElementById("test");
+reset(mySelect);
+
+mySelect.options.length = -1;
+shouldBe("mySelect.options.length", "2");
+shouldBe("mySelect.selectedIndex", "1");
+
+// 1) setting length to a larger length
+mySelect.options.length = 5;
+shouldBe("mySelect.options.length", "5");
+shouldBe("mySelect.selectedIndex", "1");
+
+// 2) setting length to a smaller length
+mySelect.options.length = 2;
+shouldBe("mySelect.options.length", "2");
+shouldBe("mySelect.selectedIndex", "1");
+
+mySelect.options.length = 1;
+shouldBe("mySelect.options.length", "1");
+shouldBe("mySelect.selectedIndex", "-1");
+reset(mySelect);
+
+// 3) setting length to the same length
+mySelect.options.length = 2;
+shouldBe("mySelect.options.length", "2");
+shouldBe("mySelect.selectedIndex", "1");
+
+// 4) setting length to non-integer values (null,
+// undefined, non-numeric string, floating point number)
+mySelect.options.length = null;
+shouldBe("mySelect.options.length", "0");
+shouldBe("mySelect.selectedIndex", "-1");
+reset(mySelect);
+
+mySelect.options.length = undefined;
+shouldBe("mySelect.options.length", "0");
+shouldBe("mySelect.selectedIndex", "-1");
+reset(mySelect);
+
+mySelect.options.length = "apple";
+shouldBe("mySelect.options.length", "0");
+shouldBe("mySelect.selectedIndex", "-1");
+reset(mySelect);
+
+mySelect.options.length = 2.1;
+shouldBe("mySelect.options.length", "2");
+shouldBe("mySelect.selectedIndex", "1");
+
+// 5) setting an element by index past the end of the current list
+mySelect.options[10] = new Option("ten", "value", true, true);
+shouldBe("mySelect.options.length", "11");
+shouldBe("mySelect.selectedIndex", "10");
+
+// 6) setting an existing element by index
+mySelect.options[10] = mySelect.options[10];
+shouldBe("mySelect.options.length", "11");
+shouldBe("mySelect.selectedIndex", "10");
+
+// 7) trying to set an element that's not an option
+// element (null, undefined, other kinds of objects, other kinds of elements)
+mySelect.options[10] = null;
+shouldBe("mySelect.options.length", "10");
+shouldBe("mySelect.selectedIndex", "-1");
+mySelect.options[10] = undefined;
+shouldBe("mySelect.options.length", "10");
+shouldBe("mySelect.selectedIndex", "-1");
+mySelect.options[10] = mySelect;
+shouldBe("mySelect.options.length", "10");
+shouldBe("mySelect.selectedIndex", "-1");
+
+</script>
index d1e98fe7ff0a91a397da35c12419f02d9df6955e..c7b5c9f1f6be8be74d8c5195a5345749d038dc2e 100644 (file)
@@ -1,3 +1,20 @@
+2006-06-17  Rob Buis  <buis@kde.org>
+
+        Reviewed by Darin.
+
+        Fix for http://bugzilla.opendarwin.org/show_bug.cgi?id=6282:
+        Adding new Option with new Option(text, value, defaultSelected, selected) fails to update selectedIndex
+
+        Update selectedIndex when a new option is added using javascript.
+
+        * bindings/js/kjs_html.cpp:
+        (KJS::JSHTMLSelectCollection::put):
+        * html/HTMLSelectElement.cpp:
+        (WebCore::HTMLSelectElement::setSelectedIndex):
+        (WebCore::HTMLSelectElement::setOption):
+        (WebCore::HTMLSelectElement::setLength):
+        * html/HTMLSelectElement.h:
+
 2006-06-17  Mitz Pettel  <opendarwin.org@mitzpettel.com>
 
         Reviewed by Darin.
index 8b859c486b2794d9ff6618b9f1e8df41aa3a0a2b..be13b7e582744e17cc73481f55a606f4c6ff26f9 100644 (file)
@@ -1750,79 +1750,44 @@ void JSHTMLSelectCollection::put(ExecState* exec, const Identifier &propertyName
 #ifdef KJS_VERBOSE
   kdDebug(6070) << "JSHTMLSelectCollection::put " << propertyName.deprecatedString() << endl;
 #endif
-  if ( propertyName == "selectedIndex" ) {
-    m_element->setSelectedIndex( value->toInt32( exec ) );
-    return;
-  }
-  // resize ?
+  if ( propertyName == "selectedIndex" )
+    m_element->setSelectedIndex(value->toInt32(exec));
   else if (propertyName == lengthPropertyName) {
-    int exception = 0;
-
-    unsigned newLen;
-    bool converted = value->getUInt32(newLen);
-
-    if (!converted)
-      return;
-
-    int diff = m_element->length() - newLen;
-
-    if (diff < 0) { // add dummy elements
-      do {
-        RefPtr<Element> option = m_element->ownerDocument()->createElement("option", exception);
-        if (exception)
-          break;         
-        m_element->add(static_cast<HTMLElement*>(option.get()), 0, exception);
-        if (exception)
-          break;
-      } while (++diff);
+    // resize ?
+    if (value->isNumber()) {
+      double newLen;
+      if (value->getNumber(newLen)) {
+        int exception = 0;
+        if (newLen >= 0) {
+          m_element->setLength(unsigned(floor(newLen)), exception);
+          setDOMException(exec, exception);
+        }
+      }
+    } else {
+      int exception = 0;
+      m_element->setLength(0, exception);
+      setDOMException(exec, exception);
     }
-    else // remove elements
-      while (diff-- > 0)
-        m_element->remove(newLen);
-
-    setDOMException(exec, exception);
-    return;
-  }
-  // an index ?
-  bool ok;
-  unsigned int u = propertyName.toUInt32(&ok);
-  if (!ok)
-    return;
-
-  if (value->isUndefinedOrNull()) {
-    // null and undefined delete. others, too ?
-    m_element->remove(u);
-    return;
-  }
+  } else {
+    // an index ?
+    bool ok;
+    unsigned i = propertyName.toUInt32(&ok);
+    if (ok) {
+      if (value->isUndefinedOrNull()) {
+        // null and undefined delete. others, too ?
+        m_element->remove(i);
+      } else {
+        WebCore::Node *option = toNode(value);
+        // is v an option element ?
+        if (!option || !option->hasTagName(optionTag))
+            return;
 
-  // is v an option element ?
-  WebCore::Node *option = toNode(value);
-  if (!option || !option->hasTagName(optionTag))
-    return;
-
-  int exception = 0;
-  int diff = int(u) - m_element->length();
-  HTMLElement* before = 0;
-  // out of array bounds ? first insert empty dummies
-  if (diff > 0) {
-    while (diff--) {
-      RefPtr<Element> dummyOption = m_element->ownerDocument()->createElement("option", exception);
-      if (!dummyOption)
-        break;      
-      m_element->add(static_cast<HTMLElement*>(dummyOption.get()), 0, exception);
-      if (exception) 
-          break;
+        int exception = 0;
+        m_element->setOption(i, static_cast<HTMLOptionElement*>(option), exception);
+        setDOMException(exec, exception);
+      }
     }
-    // replace an existing entry ?
-  } else if (diff < 0) {
-    before = static_cast<HTMLElement*>(m_element->options()->item(u+1));
-    m_element->remove(u);
   }
-  // finally add the new element
-  if (exception == 0)
-    m_element->add(static_cast<HTMLOptionElement*>(option), before, exception);
-
-  setDOMException(exec, exception);
 }
 
 ////////////////////// Image Object ////////////////////////
index 8aef28c9ef8bd66e81e5fb0c552654ba60a903cb..b1ace38785a456aaaba8546793af73ace948eaf2 100644 (file)
@@ -103,14 +103,16 @@ int HTMLSelectElement::selectedIndex() const
     return -1;
 }
 
-void HTMLSelectElement::setSelectedIndex( int  index )
+void HTMLSelectElement::setSelectedIndex( int index, bool deselect )
 {
     // deselect all other options and select only the new one
     Vector<HTMLElement*> items = listItems();
     int listIndex;
-    for (listIndex = 0; listIndex < int(items.size()); listIndex++) {
-        if (items[listIndex]->hasLocalName(optionTag))
-            static_cast<HTMLOptionElement*>(items[listIndex])->setSelected(false);
+    if (deselect) {
+        for (listIndex = 0; listIndex < int(items.size()); listIndex++) {
+            if (items[listIndex]->hasLocalName(optionTag))
+                static_cast<HTMLOptionElement*>(items[listIndex])->setSelected(false);
+        }
     }
     listIndex = optionToListIndex(index);
     if (listIndex >= 0)
@@ -468,4 +470,49 @@ Node* HTMLSelectElement::namedItem(const String &name, bool caseSensitive)
     return (options()->namedItem(name, caseSensitive));
 }
 
+void HTMLSelectElement::setOption(unsigned index, HTMLOptionElement* option, ExceptionCode& ec)
+{
+    ec = 0;
+    if (index > INT_MAX)
+        index = INT_MAX;
+    int diff = index  - length();
+    HTMLElement* before = 0;
+    // out of array bounds ? first insert empty dummies
+    if (diff > 0) {
+        setLength(index, ec);
+        // replace an existing entry ?
+    } else if (diff < 0) {
+        before = static_cast<HTMLElement*>(options()->item(index+1));
+        remove(index);
+    }
+    // finally add the new element
+    if (ec == 0) {
+        add(option, before, ec);
+        if (diff >= 0)
+            setSelectedIndex(index, !m_multiple);
+    }
+}
+
+void HTMLSelectElement::setLength(unsigned newLen, ExceptionCode& ec)
+{
+    ec = 0;
+    if (newLen > INT_MAX)
+        newLen = INT_MAX;
+    int diff = length() - newLen;
+
+    if (diff < 0) { // add dummy elements
+        do {
+            RefPtr<Element> option = ownerDocument()->createElement("option", ec);
+            if (!option)
+                break;
+            add(static_cast<HTMLElement*>(option.get()), 0, ec);
+            if (ec)
+                break;
+        } while (++diff);
+    }
+    else // remove elements
+        while (diff-- > 0)
+            remove(newLen);
+}
+
 } // namespace
index 549f5567c2d83d86d33e513bb0c819623046a117..f534650522c0da6f2920fff34c2a22b8c770a89c 100644 (file)
@@ -51,7 +51,7 @@ public:
     virtual void recalcStyle(StyleChange);
 
     int selectedIndex() const;
-    void setSelectedIndex(int index);
+    void setSelectedIndex(int index, bool = true);
 
     virtual bool isEnumeratable() const { return true; }
 
@@ -110,6 +110,9 @@ public:
 
     void setSize(int);
 
+    void setOption(unsigned index, HTMLOptionElement*, ExceptionCode&);
+    void setLength(unsigned, ExceptionCode&);
+
     virtual Node* namedItem(const String &name, bool caseSensitive = true);
 
 private: