1128175 - [OS X] Some dialogs in Content and Privacy in-content preferences can't be dismissed with keyboard (ESC) when only text boxes and lists can receive focus (OS-level pref)

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect, P3)

Description

[Francesco Lodolo [:flod]]

Content->Fonts & Colors->Advanced… and Colors… can't be closed by pressing ESC on the keyboard. As far as I tested, all other dialogs can be closed correctly.

Comment 1

[Francesco Lodolo [:flod]]

Same issue with Privacy -> Clear history when Firefox closes -> Settings.

Comment 2

[:Gijs (he/him)]

Can you still reproduce this on current Nightly? Because I can't...

Comment 3

[Francesco Lodolo [:flod]]

Yes, it's still happening for all 3 dialogs. To be sure I checked with a clean profile, same behavior.

Firefox version: 38.0a1 (2015-02-12)

Take for example "Colors…": open the dialog, try to press ESC, nothing happens.

While testing I realized that, to make ESC work, I can either press "Enter" once, or change a control in the dialog (like deselect a checkbox). It's like the dialog has no focus when it's opened.

Comment 4

[:Gijs (he/him)]

(In reply to Francesco Lodolo [:flod] from comment #3)
> Yes, it's still happening for all 3 dialogs. To be sure I checked with a
> clean profile, same behavior.
> 
> Firefox version: 38.0a1 (2015-02-12)
> 
> Take for example "Colors…": open the dialog, try to press ESC, nothing
> happens.
> 
> While testing I realized that, to make ESC work, I can either press "Enter"
> once, or change a control in the dialog (like deselect a checkbox). It's
> like the dialog has no focus when it's opened.

This is bizarre. I just checked a clean profile on my mac. Here are the exact steps I follow:

1. open profile dialog with the same nightly, on my OS X retina mbp
2. create a new profile
3. command-, to open preferences
4. click "content"
5. click "advanced"
6. hit escape

and it Just Works. Ditto for the colors dialog. :-\

What's your OS level focusable elements pref set to?

Comment 5

[Francesco Lodolo [:flod]]

(In reply to :Gijs Kruitbosch from comment #4)
> What's your OS level focusable elements pref set to?

I wish I knew where to find that information (Google didn't help).
Note that it happens on 2 different Macs, same OS X version (10.10.2). As far as I remember, I never changed accessibility options in OS X.

The only other difference might be that I'm using a localized build (it) instead of the en-US one.

Comment 6

[:Gijs (he/him)]

(In reply to Francesco Lodolo [:flod] from comment #5)
> (In reply to :Gijs Kruitbosch from comment #4)
> > What's your OS level focusable elements pref set to?
> 
> I wish I knew where to find that information (Google didn't help).

http://www.guidingtech.com/34353/re-enable-mavericks-features/

scroll down to "2. Using the Keyboard to Select a Button On a Dialog Box".

> Note that it happens on 2 different Macs, same OS X version (10.10.2). As
> far as I remember, I never changed accessibility options in OS X.

Maybe this is it? I'm on 10.9.

> The only other difference might be that I'm using a localized build (it)
> instead of the en-US one.

I don't see why that would make a difference, but you could try an en-US build to make sure, I guess?

Comment 7

[Francesco Lodolo [:flod]]

(In reply to :Gijs Kruitbosch from comment #6)
> http://www.guidingtech.com/34353/re-enable-mavericks-features/
> scroll down to "2. Using the Keyboard to Select a Button On a Dialog Box".

Interesting, it works by setting "All controls". Any idea then why other dialogs work without that pref set (e.g. General -> Use a bookmark…, or even Content->Pop-up->Exceptions…)? It doesn't seem right.

> I don't see why that would make a difference, but you could try an en-US
> build to make sure, I guess?
Tried out of curiosity, same result (didn't think you were on 10.9, so that could have been one of the differences between my setup and yours).

Comment 8

[:Gijs (he/him)]

(In reply to Francesco Lodolo [:flod] from comment #7)
> (In reply to :Gijs Kruitbosch from comment #6)
> > http://www.guidingtech.com/34353/re-enable-mavericks-features/
> > scroll down to "2. Using the Keyboard to Select a Button On a Dialog Box".
> 
> Interesting, it works by setting "All controls". Any idea then why other
> dialogs work without that pref set (e.g. General -> Use a bookmark…, or even
> Content->Pop-up->Exceptions…)? It doesn't seem right.

It is indeed not right, but I bet that focus automatically moves into the dialog in my case (I had "All controls" set already) and it fails if focusing whatever type of element it's trying to focus isn't possible (because there's no list or input field, unlike the two counterexamples you just gave!), and then focus either goes to limbo land or somewhere else where we don't catch the [esc] keypress.

Not entirely sure how to fix this, but I'm happier now that I understand where & when & why this happens. This can stay on the shipping-blocker list for now, and we'll get back to it. :-)

Comment 9

[:Gijs (he/him)]

Attachment: add explicit <esc> handler,

Note: on fx-team tip, incl. the tab-trapping patch which I just landed

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8576585 [details] [diff] [review]
add explicit <esc> handler,

Review of attachment 8576585 [details] [diff] [review]:
-----------------------------------------------------------------

Nice, I'm happy how the addDialogEventListeners/removeDialogEventListeners has simplified the various places to add and remove the event listeners. This patch is pretty straightforward :)

Comment 11

[:Gijs (he/him)]

remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=473395996095

Comment 12

[:Gijs (he/him)]

remote:   https://hg.mozilla.org/integration/fx-team/rev/6a64409286ca

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6a64409286ca

Comment 14

[:Gijs (he/him)]

Comment on attachment 8576585 [details] [diff] [review]
add explicit <esc> handler,

Approval Request Comment
[Feature/regressing bug #]: n/a
[User impact if declined]: closing in-content pref dialogs with escape might not work on mac depending on system pref
[Describe test coverage new/current, TreeHerder]: no
[Risks and why]: very low, simple extra keyboard event handler
[String/UUID change made/needed]: no

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d04808ad7b1a