Closed Bug 1187404 Opened 9 years ago Closed 9 years ago

[e10s] Tabbing out of <select> dropdown doesn't properly update the selected option

Categories

(Core :: Layout: Form Controls, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
e10s m8+ ---
firefox42 --- affected
firefox44 --- fixed

People

(Reporter: bzbarsky, Assigned: gw280)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

STEPS TO REPRODUCE: 1) Load https://bugzilla.mozilla.org/attachment.cgi?id=8638598&action=edit 2) Click the <select> next to "superreview". 3) Hit the down arrow on the keyboard. 4) Hit tab on the keyboard EXPECTED RESULTS: After step 3, the "?" option is selected. After step 4, the textfield to put the requestee in appears. ACTUAL RESULTS: Step 3 moves the selection, but then step 4 forgets it and closes the dropdown without changing the selected option. ADDITIONAL INFORMATION: Things work correctly if I close the dropdown with enter instead of tab. Also, you can replace step 2 with tabbing to the <select> and hitting Option+downarrow, at least on Mac, and get the same observed behavior.
Actually the correct behaviour on Mac and Linux at step 4 is to ignore the tab key entirely and just leave the menu open. Otherwise, we could handle this by adding a tab key handler to handle this as a changed value.
My correct behavior description was based on what we do in non-e10s builds. It's probably better to match those for a start, then file a separate bug to figure out whether we want the behavior chanage and if so on which OSes....
Assignee: nobody → gwright
Comment on attachment 8666754 [details] [diff] [review] 0001-Bug-1187404-Allow-tab-to-select-an-option-from-a-sel.patch >--- a/browser/base/content/browser.xul >+++ b/browser/base/content/browser.xul >@@ -161,6 +161,7 @@ > popuponly menulist to be its immediate parent. --> > <menulist popuponly="true" id="ContentSelectDropdown" hidden="true"> > <menupopup rolluponmousewheel="true" >+ activateontab="true" This patch doesn't add anything that activates anything on tab, so I think the attribute name is misleading. (I'm assuming SelectParentHelper.jsm does happens to do the right thing) See below. >- // close popups or deactivate menubar when Tab or F10 are pressed >- if (aTopVisibleMenuItem) { >- Rollup(0, false, nullptr, nullptr); >- } else if (mActiveMenuBar) { >- mActiveMenuBar->MenuClosed(); >+ if (!GetTopVisibleMenu()->Frame()->GetContent()->AttrValueIs(kNameSpaceID_None, You can just use aTopVisibleMenuItem, not GetTopVisibleMenu(). It can be null however. I'd suggest moving the attribute check inside the 'if (aTopVisibleMenuItem) {' block and leaving the menubar portion to happen within the else as it is currently. Then you can make the attribute name 'norollupontab' which correlates nicely with the existing rolluponmousewheel attribute.
Attachment #8666754 - Flags: review?(enndeakin) → review-
Comment on attachment 8666754 [details] [diff] [review] 0001-Bug-1187404-Allow-tab-to-select-an-option-from-a-sel.patch OK, I somehow missed the fallthrough case, so this looks ok as is, except for the comment I made about not calling GetTopVisibleMenu() again and null-checking aTopVisibleMenuItem.
Attachment #8666754 - Flags: review- → review+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: