Closed Bug 1615854 Opened 5 years ago Closed 5 years ago

Escape/cmd-./return keys do not work to dismiss "Clear Recent History" dialog

Categories

(Toolkit :: XUL Widgets, defect)

73 Branch
Unspecified
macOS
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla75
Tracking Status
firefox-esr68 --- unaffected
firefox73 + wontfix
firefox74 + verified
firefox75 + verified

People

(Reporter: yoasif, Assigned: bgrins)

References

(Regression)

Details

(Keywords: access, regression, Whiteboard: [access-p2])

Attachments

(1 file)

From https://www.reddit.com/r/firefox/comments/f4fy3q/clear_recent_history_keyboard_regression_on_macos/

Steps to reproduce:

  1. Do Command-shift-delete
  2. Press any one of escape, command-. or return

What happens:

Nothing.

Expected result:

escape, command-. should exit the dialog (Cancel).
return should "Clear Now"

11:40.21 INFO: Narrowed inbound regression window from [1fb1ca98, e183cbb4] (3 builds) to [88b25807, e183cbb4] (2 builds) (~1 steps left)
11:40.21 INFO: No more inbound revisions, bisection finished.
11:40.21 INFO: Last good revision: 88b25807638231dcf403d8f0b597fc770f1d24a4
11:40.21 INFO: First bad revision: e183cbb4983cfb3aecf97ab18fad916b91f89e7e
11:40.21 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=88b25807638231dcf403d8f0b597fc770f1d24a4&tochange=e183cbb4983cfb3aecf97ab18fad916b91f89e7e

[Tracking Requested - why for this release]: Regression in a user facing feature.

Has Regression Range: --- → yes
Has STR: --- → yes
Keywords: regression
Regressed by: 1585482
Summary: Escape/cmd-., return keys do not work to dismiss "Clear Recent History" dialog → Escape/cmd-./return keys do not work to dismiss "Clear Recent History" dialog
Component: Address Bar → XUL Widgets
Product: Firefox → Toolkit

Kirk, could you please look into this?

Flags: needinfo?(ksteuber)

I'm afraid that I'm no longer on the Browser Architecture team, and I don't currently have time to work on this.

I did take a few minutes to look into this, and I cannot reproduce the issue. escape and return both work for me. I can't test command-., because I'm on a Windows machine and I have no Command key. Perhaps this is an OS-specific issue?

Flags: needinfo?(ksteuber)

Brian, can you recommend somebody else to look into this? I'd do it myself but I'm not on Mac either.

Flags: needinfo?(bgrinstead)

Setting to fix-optional for 73, but we should definitely try to get this fixed for 74 as this presumably affects screen reader users disproportionately.

Keywords: access

Because this is mac-only and we don't actually have screen reader support on Mac, setting accessibility priority to 2.

Whiteboard: [access-p2]
Attachment #9128043 - Attachment description: Bug 1615854 - WIP - Listen to keypress events on the window instead of the dialog element to fix keyboard handling for dialogs in osx → Bug 1615854 - Listen to keypress events on the window instead of the dialog element to fix keyboard handling for dialogs in osx
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Attachment #9128043 - Attachment description: Bug 1615854 - Listen to keypress events on the window instead of the dialog element to fix keyboard handling for dialogs in osx → Bug 1615854 - Listen to keypress events on the document instead of the dialog element to fix keyboard handling for dialogs in osx
Pushed by bgrinstead@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/812c3e2899d7 Listen to keypress events on the document instead of the dialog element to fix keyboard handling for dialogs in osx r=dao
Flags: needinfo?(bgrinstead)
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla75

Brian, given the number of duplicates, do you think this is a good candidate for an uplift in RC? Thanks

Flags: needinfo?(bgrinstead)

(In reply to Pascal Chevrel:pascalc from comment #13)

Brian, given the number of duplicates, do you think this is a good candidate for an uplift in RC? Thanks

Sorry for the delay. It should be safe to uplift but I went ahead and flagged for qe-verify to get additional verification.

Flags: needinfo?(bgrinstead) → qe-verify+

Comment on attachment 9128043 [details]
Bug 1615854 - Listen to keypress events on the document instead of the dialog element to fix keyboard handling for dialogs in osx

Beta/Release Uplift Approval Request

  • User impact if declined: Keyboard shortcuts to close some dialogs on Mac don't work (for instance, "Clear Recent History")
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: See Comment 0. Could also use a test on Windows to make sure no behavior has changed there (though it was only broken on Mac previously).
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): It's a small frontend change only affecting UI inside of dialog widgets.

It's changing the keypress event listener target from the dialog element itself to the document. It's possible this would cause a problem if there's some other code that was expecting the dialog to be the target, since pre-Bug 1585482 it was also listening on the dialog element, but it seems relatively unlikely if not detected in manual testing.

  • String changes made/needed:
Attachment #9128043 - Flags: approval-mozilla-beta?

I can reproduce the bug and using mozregression can confirm that the bug got fixed by the patch in this bug.

Comment on attachment 9128043 [details]
Bug 1615854 - Listen to keypress events on the document instead of the dialog element to fix keyboard handling for dialogs in osx

Several reports for a UI bug on macOS, fix is simple and verified on Nightly, taking the patch for 74RC2, thanks.

Attachment #9128043 - Flags: approval-mozilla-beta? → approval-mozilla-release+

I have reproduced this in Nightly v75.0a1 from 2020-02-26, Beta v74.0b9 and Beta v74.0 (Release Candidate), and verified the fix in Nightly v75.0a1 from 2020-03-04 and Firefox Release Candidate v74.0 (taken from https://treeherder.mozilla.org/#/jobs?repo=mozilla-release&revision=07eaadc8461984cadfcd835f6ad27b5fc5596305&searchStr=mac%2Cos%2Cx%2Ccross%2Ccompiled%2Cshippable&selectedJob=291506243)

I deem this bug verified!

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: