Closed
Bug 1155784
Opened 9 years ago
Closed 9 years ago
<optgroup> items are considered when selecting index of <select> in e10s mode
Categories
(Core :: Layout: Form Controls, defect)
Tracking
()
RESOLVED
FIXED
mozilla40
People
(Reporter: mconley, Assigned: gw280)
References
(Blocks 1 open bug, )
Details
Attachments
(1 file)
|
2.66 KB,
patch
|
mconley
:
review+
|
Details | Diff | Splinter Review |
STR: 0) Open an e10s browser window 1) Go to data:text/html,<!DOCTYPE html><html><body><select><optgroup label="Swedish Cars"><option value="volvo">Volvo</option><option value="saab">Saab</option></optgroup><optgroup label="German Cars"><option value="mercedes">Mercedes</option><option value="audi">Audi</option></optgroup></select></body></html> 2) Click on the <select> element and choose Audi 3) Click on <select> again ER: "Audi" should be selected. AR: "German Cars", a disabled <optgroup> element, is selected instead.
|
Reporter | |
Updated•9 years ago
|
Assignee: nobody → gwright
|
Assignee | |
Updated•9 years ago
|
Blocks: e10s-select
|
|
Assignee | |
Comment 1•9 years ago
|
||
The issue here is that even when XUL menuitems are disabled (which is what we're using to represent an <optgroup>), they are still selectable via selectedIndex, and have an index. We take care of this for the most part by using the "value" attribute set on the menuitem to store the index as it would be on the child side, and using that for the change event that gets sent back (so the child gets the correct index). This patch moves the selection for the menuitem into the populate function so that we can select the item by comparing the requested selectedIndex against the index we use to store in the value attribute as we're building it, so that the popup displays the correct selected item.
Attachment #8594992 -
Flags: review?(mconley)
|
|
Reporter | |
Comment 2•9 years ago
|
||
Comment on attachment 8594992 [details] [diff] [review] 0001-Bug-1155784-Do-not-consider-optgroup-items-when-sett.patch Review of attachment 8594992 [details] [diff] [review]: ----------------------------------------------------------------- Good catch, George! ::: toolkit/modules/SelectParentHelper.jsm @@ +14,4 @@ > populate: function(menulist, items, selectedIndex) { > // Clear the current contents of the popup > menulist.menupopup.textContent = ""; > + populateChildren(menulist, menulist.menupopup, items, selectedIndex); Seems silly that we have to pass menulist *and* menulist.menupopup. Can you modify this to take just one?
Attachment #8594992 -
Flags: review?(mconley) → review+
|
|
Assignee | |
Comment 3•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/056853ed228d
https://hg.mozilla.org/mozilla-central/rev/056853ed228d
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
|
||
Updated•9 years ago
|
QA Whiteboard: [good first verify][verify in Nightly only]
You need to log in
before you can comment on or make changes to this bug.
Description
•