Closed Bug 1313130 Opened 4 years ago Closed 4 years ago

E10s: Hovering Over <Select> and pressing Backspace Navigates to Previous Page

Categories

(Toolkit :: Form Manager, defect)

52 Branch
All
Windows
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla53
Tracking Status
firefox49 --- wontfix
firefox50 --- wontfix
firefox51 --- wontfix
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: jwilliams, Assigned: enndeakin)

References

(Blocks 1 open bug, )

Details

(Keywords: regression)

Attachments

(1 file)

STR:
Pre-Req
Have a previous page
1. Navigate to https://people.mozilla.org/~mnoorenberghe/w3c_notifications.htm
2. Click the Direction datalist
3. Hover over a selection
4. Press Backspace

Expected:
The datalist popup is closed

Actual:
Browser Navigates to previous page

This is e10s specific. When e10s = False, the datalist popup stays open.

All Windows Platforms are affected
Ubuntu is not affected
No longer blocks: 1296638
Summary: E10s: Hovering Over Non-login field with datalist and pressing Backspace Navigates to Previous Page → E10s: Hovering Over <Select> and pressing Backspace Navigates to Previous Page
mozregression --bad 9-18-2015 --good 9-17-2016

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e7d613b3bcfe1e865378bfac37de64560d1234ec&tochange=11dc79e232110ba6de5179e46dfbda77b52a88c3

I suspect bug 1191897 is what caused this regression.

Neil I see you worked on bug 1191897. Is this the intended functionality?
Flags: needinfo?(enndeakin)
I don't think it was intended or not intended. Should be easy enough to add a special case for the backspace key here.
Flags: needinfo?(enndeakin)
Neil, David said to assign this to you :)
Assignee: nobody → enndeakin
Blocks: e10s-select
This patch removes ignorekeys="handled" and switches to ignorekeys="shortcuts".

The old behaviour was to not call preventDefault on keys that the menu handled itself. However, I don't think that is quite what we wanted to fix 1191897, as it causes different behaviour when you press a key that matches a menu label versus a key that doesn't.

Instead, I switch to a different behaviour which doesn't call preventDefault only when the accelerator key is down.
Attachment #8814156 - Flags: review?(ksteuber)
Attachment #8814156 - Flags: review?(ksteuber) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/70791e56e69ae0ce66c430a6e453eff5aeeef09e
Bug 1313130, change menu shortcut handling so that Windows does not call preventDefault only when the accelerator key is down rather than when a key isn't handled, r=ksteuber
https://hg.mozilla.org/mozilla-central/rev/70791e56e69a
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
I can verify this is fixed on the current nightly.
Status: RESOLVED → VERIFIED
Please request Aurora/Beta approval on this when you get a chance.
Flags: needinfo?(enndeakin)
Comment on attachment 8814156 [details] [diff] [review]
Modify menu shortcut handling a bit

Approval Request Comment
[Feature/Bug causing the regression]:1191897
[User impact if declined]:better handling of keyboard shortcuts when a select popup is open
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]:no
[Is the change risky?]:no
[Why is the change risky/not risky?]: improved version of 1191897
[String changes made/needed]:no
Flags: needinfo?(enndeakin)
Attachment #8814156 - Flags: approval-mozilla-aurora?
Comment on attachment 8814156 [details] [diff] [review]
Modify menu shortcut handling a bit

menu shortcut handling fix, aurora52+
Attachment #8814156 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Setting qe-verify- based on Comment 10.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.