- Revision
- 98505
- Author
- jon...@apple.com
- Date
- 2011-10-26 11:36:22 -0700 (Wed, 26 Oct 2011)
Log Message
selectedIndex gets set from -1 to 0 when modifying options
https://bugs.webkit.org/show_bug.cgi?id=70547
<rdar://problem/8388856>
Reviewed by Darin Adler.
Source/WebCore:
Changing the text causes a recalculation of the list items, which in the menu list case
forces the first element to be selected. We check the value of the selected option prior,
and restore it if it differs.
Test: fast/dom/HTMLSelectElement/selected-index-preserved-when-option-text-changes.html
* html/HTMLOptionElement.cpp:
(WebCore::HTMLOptionElement::setText):
* html/HTMLSelectElement.h: promote usesMenuList() from private to public for use by HTMLOptionElement
LayoutTests:
The added tests set the selected index to either -1 or 1 (a legitimate value), and then change
the text, value, and label of the option. In all cases, the selected index should remain unchanged.
* fast/dom/HTMLSelectElement/selected-index-preserved-when-option-text-changes-expected.txt: Added.
* fast/dom/HTMLSelectElement/selected-index-preserved-when-option-text-changes.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (98504 => 98505)
--- trunk/LayoutTests/ChangeLog 2011-10-26 18:29:55 UTC (rev 98504)
+++ trunk/LayoutTests/ChangeLog 2011-10-26 18:36:22 UTC (rev 98505)
@@ -1,3 +1,17 @@
+2011-10-26 Jon Lee <jon...@apple.com>
+
+ selectedIndex gets set from -1 to 0 when modifying options
+ https://bugs.webkit.org/show_bug.cgi?id=70547
+ <rdar://problem/8388856>
+
+ Reviewed by Darin Adler.
+
+ The added tests set the selected index to either -1 or 1 (a legitimate value), and then change
+ the text, value, and label of the option. In all cases, the selected index should remain unchanged.
+
+ * fast/dom/HTMLSelectElement/selected-index-preserved-when-option-text-changes-expected.txt: Added.
+ * fast/dom/HTMLSelectElement/selected-index-preserved-when-option-text-changes.html: Added.
+
2011-10-26 Zoltan Herczeg <zherc...@webkit.org>
Add new CSS escape sequence parsing tests
Added: trunk/LayoutTests/fast/dom/HTMLSelectElement/selected-index-preserved-when-option-text-changes-expected.txt (0 => 98505)
--- trunk/LayoutTests/fast/dom/HTMLSelectElement/selected-index-preserved-when-option-text-changes-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/HTMLSelectElement/selected-index-preserved-when-option-text-changes-expected.txt 2011-10-26 18:36:22 UTC (rev 98505)
@@ -0,0 +1,40 @@
+
+Fix for bug 70547-- changing the text of an option in a select element with no selected option (-1) should preserve that option
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Running tests on menu list
+Setting selected index to -1
+Changed text attribute of first item
+PASS mySelect.selectedIndex is -1
+Changed value attribute of first item
+PASS mySelect.selectedIndex is -1
+Changed label attribute of first item
+PASS mySelect.selectedIndex is -1
+Setting selected index to 1
+Changed text attribute of first item
+PASS mySelect.selectedIndex is 1
+Changed value attribute of first item
+PASS mySelect.selectedIndex is 1
+Changed label attribute of first item
+PASS mySelect.selectedIndex is 1
+Running tests on list box
+Setting selected index to -1
+Changed text attribute of first item
+PASS mySelect.selectedIndex is -1
+Changed value attribute of first item
+PASS mySelect.selectedIndex is -1
+Changed label attribute of first item
+PASS mySelect.selectedIndex is -1
+Setting selected index to 1
+Changed text attribute of first item
+PASS mySelect.selectedIndex is 1
+Changed value attribute of first item
+PASS mySelect.selectedIndex is 1
+Changed label attribute of first item
+PASS mySelect.selectedIndex is 1
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/dom/HTMLSelectElement/selected-index-preserved-when-option-text-changes.html (0 => 98505)
--- trunk/LayoutTests/fast/dom/HTMLSelectElement/selected-index-preserved-when-option-text-changes.html (rev 0)
+++ trunk/LayoutTests/fast/dom/HTMLSelectElement/selected-index-preserved-when-option-text-changes.html 2011-10-26 18:36:22 UTC (rev 98505)
@@ -0,0 +1,70 @@
+<!doctype html>
+<html>
+<body>
+
+<select id="theSelect">
+<option>Item 1</option>
+<option>Item 2</option>
+<option>Item 3</option>
+</select>
+<select id="theSelect2" multiple>
+<option>Item 1</option>
+<option>Item 2</option>
+<option>Item 3</option>
+<option>Item 4</option>
+<option>Item 5</option>
+</select>
+
+<div id="console"></div>
+<script src=""
+<script>
+description("Fix for bug 70547-- changing the text of an option in a select element with no selected option (-1) should preserve that option");
+
+function runTest() {
+ debug("Setting selected index to -1");
+ mySelect.selectedIndex = -1;
+ mySelect.options[0].text = "Changed text for item 1";
+ debug("Changed text attribute of first item");
+ shouldBe("mySelect.selectedIndex", "-1");
+
+ mySelect.selectedIndex = -1;
+ mySelect.options[0].value = "Changed value for item 1";
+ debug("Changed value attribute of first item");
+ shouldBe("mySelect.selectedIndex", "-1");
+
+ mySelect.selectedIndex = -1;
+ mySelect.options[0].label = "Changed label for item 1";
+ debug("Changed label attribute of first item");
+ shouldBe("mySelect.selectedIndex", "-1");
+
+ debug("Setting selected index to 1");
+ mySelect.selectedIndex = 1;
+ mySelect.options[0].text = "Changed text for item 1";
+ debug("Changed text attribute of first item");
+ shouldBe("mySelect.selectedIndex", "1");
+
+ mySelect.selectedIndex = 1;
+ mySelect.options[0].value = "Changed value for item 1";
+ debug("Changed value attribute of first item");
+ shouldBe("mySelect.selectedIndex", "1");
+
+ mySelect.selectedIndex = 1;
+ mySelect.options[0].label = "Changed label for item 1";
+ debug("Changed label attribute of first item");
+ shouldBe("mySelect.selectedIndex", "1");
+}
+
+var mySelect = document.getElementById("theSelect");
+debug("Running tests on menu list");
+runTest();
+
+mySelect = document.getElementById("theSelect2");
+debug("Running tests on list box");
+runTest();
+
+var successfullyParsed = true;
+</script>
+<script src=""
+
+</body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (98504 => 98505)
--- trunk/Source/WebCore/ChangeLog 2011-10-26 18:29:55 UTC (rev 98504)
+++ trunk/Source/WebCore/ChangeLog 2011-10-26 18:36:22 UTC (rev 98505)
@@ -1,3 +1,21 @@
+2011-10-26 Jon Lee <jon...@apple.com>
+
+ selectedIndex gets set from -1 to 0 when modifying options
+ https://bugs.webkit.org/show_bug.cgi?id=70547
+ <rdar://problem/8388856>
+
+ Reviewed by Darin Adler.
+
+ Changing the text causes a recalculation of the list items, which in the menu list case
+ forces the first element to be selected. We check the value of the selected option prior,
+ and restore it if it differs.
+
+ Test: fast/dom/HTMLSelectElement/selected-index-preserved-when-option-text-changes.html
+
+ * html/HTMLOptionElement.cpp:
+ (WebCore::HTMLOptionElement::setText):
+ * html/HTMLSelectElement.h: promote usesMenuList() from private to public for use by HTMLOptionElement
+
2011-10-25 Alexey Proskuryakov <a...@apple.com>
Embedded PDFs should be known to DocumentLoader
Modified: trunk/Source/WebCore/html/HTMLOptionElement.cpp (98504 => 98505)
--- trunk/Source/WebCore/html/HTMLOptionElement.cpp 2011-10-26 18:29:55 UTC (rev 98504)
+++ trunk/Source/WebCore/html/HTMLOptionElement.cpp 2011-10-26 18:36:22 UTC (rev 98505)
@@ -137,15 +137,24 @@
void HTMLOptionElement::setText(const String &text, ExceptionCode& ec)
{
+ // Changing the text causes a recalc of a select's items, which will reset the selected
+ // index to the first item if the select is single selection with a menu list. We attempt to
+ // preserve the selected item.
+ HTMLSelectElement* select = ownerSelectElement();
+ bool selectIsMenuList = select && select->usesMenuList();
+ int oldSelectedIndex = selectIsMenuList ? select->selectedIndex() : -1;
+
// Handle the common special case where there's exactly 1 child node, and it's a text node.
Node* child = firstChild();
- if (child && child->isTextNode() && !child->nextSibling()) {
+ if (child && child->isTextNode() && !child->nextSibling())
static_cast<Text *>(child)->setData(text, ec);
- return;
+ else {
+ removeChildren();
+ appendChild(Text::create(document(), text), ec);
}
-
- removeChildren();
- appendChild(Text::create(document(), text), ec);
+
+ if (selectIsMenuList && select->selectedIndex() != oldSelectedIndex)
+ select->setSelectedIndex(oldSelectedIndex);
}
void HTMLOptionElement::accessKeyAction(bool)
Modified: trunk/Source/WebCore/html/HTMLSelectElement.h (98504 => 98505)
--- trunk/Source/WebCore/html/HTMLSelectElement.h 2011-10-26 18:29:55 UTC (rev 98504)
+++ trunk/Source/WebCore/html/HTMLSelectElement.h 2011-10-26 18:36:22 UTC (rev 98505)
@@ -52,6 +52,8 @@
int size() const { return m_size; }
bool multiple() const { return m_multiple; }
+ bool usesMenuList() const;
+
void add(HTMLElement*, HTMLElement* beforeElement, ExceptionCode&);
void remove(int index);
void remove(HTMLOptionElement*);
@@ -148,7 +150,6 @@
bool platformHandleKeydownEvent(KeyboardEvent*);
void listBoxDefaultEventHandler(Event*);
void setOptionsChangedOnRenderer();
- bool usesMenuList() const;
enum SkipDirection {
SkipBackwards = -1,
