[Webkit-unassigned] [Bug 137553] abandoned select option is reselected when shift selecting new options
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Oct 15 14:04:27 PDT 2014
https://bugs.webkit.org/show_bug.cgi?id=137553
Ryosuke Niwa <rniwa at webkit.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #239868|review?, commit-queue? |review-, commit-queue-
Flag| |
--- Comment #13 from Ryosuke Niwa <rniwa at webkit.org> 2014-10-15 14:04:18 PST ---
(From update of attachment 239868)
View in context: https://bugs.webkit.org/attachment.cgi?id=239868&action=review
r- because I'd like to see another iteration.
> LayoutTests/fast/forms/listbox-selection-after-typeahead-expected.txt:9
> +PASS selectionPattern("sl1") is "0100000"
> +PASS selectionPattern("sl1") is "0000010"
> +PASS selectionPattern("sl1") is "0000110"
> +PASS successfullyParsed is true
These outputs don't tell us anything about what we're testing and why the result is correct.
> LayoutTests/fast/forms/listbox-selection-after-typeahead.html:24
> +parent.innerHTML = '<select id="sl1" multiple size=7>'
> + + '<option>White</option>'
> + + '<option>Silver</option>'
> + + '<option>Gray</option>'
> + + '<option>Black</option>'
> + + '<option>Red</option>'
> + + '<option>Maroon</option>'
> + + '<option>Yellow</option>'
> + + '</select>'
This doesn't need to be added by innerHTML. Also don't use abbreviated names such as sl1.
> LayoutTests/fast/forms/listbox-selection-after-typeahead.html:42
> + var event = document.createEvent("MouseEvent");
> + event.initMouseEvent("mousedown", true, true, document.defaultView, 1, sl.offsetLeft + borderPaddingLeft, sl.offsetTop + y, sl.offsetLeft + borderPaddingLeft, sl.offsetTop + y, false, false, false, false, 0, document);
> + sl.dispatchEvent(event);
Since we're already relying on eventSender for keyDown, why don't we just use eventSender for mouse click as well?
> LayoutTests/fast/forms/listbox-selection-after-typeahead.html:51
> +function selectionPattern(selectId)
I didn't understand what "selection pattern" meant until I read this code.
I would call it selectedStates instead.
>> LayoutTests/fast/forms/listbox-selection-after-typeahead.html:62
>> +mouseDownOnSelect("sl1", 1);
>> +shouldBe('selectionPattern("sl1")', '"0100000"');
>
> shouldBeEqualToString()
You should put mouseDownOnSelect("sl1", 1); into shouldBeEqualToString so that it appears in the expected result.
>> Source/WebCore/html/HTMLSelectElement.cpp:880
>>
>
> Deselect should be called first, otherwise the active selection wrongly records previously selected states
I think this code deserves a why comment saying that setActiveSelectionAnchorIndex expects selectedState to be up-to-date.
Alternatively, we should rename setActiveSelectionAnchorIndex to setActiveSelectionAnchorIndexCachingSelectedStates to make the code self explanatory.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list