1390509 - Fix access keys for location, camera and microphone permissions

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect)

Description

[Ton [:Tonnes]]

See https://bugzilla.mozilla.org/show_bug.cgi?id=1381401#c15.

Bug 1381401 added permissions strings and keys, though the keys apply to the Settings… button, not the strings themselves.

They should probably be changed to S, e, t and n - their entities are probably fine. In this order, the n already seems to be fine too.

Comment 1

[Johann Hofmann [:johannh]]

We had quite a long discussion about this in #fx-team, but we didn't come to a better solution than what's proposed in comment 0. We should probably do that.

Comment 3

[:prathiksha]

Attachment: Bug 1390509 - Fix access keys for location, camera and microphone permissions.

Review commit: https://reviewboard.mozilla.org/r/170280/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/170280/

Comment 4

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

Comment on attachment 8898920 [details]
Bug 1390509 - Fix access keys for location, camera and microphone permissions.

https://reviewboard.mozilla.org/r/170280/#review175482

::: browser/locales/en-US/chrome/browser/preferences/content.dtd:27
(Diff revision 1)
> -<!ENTITY  cameraSettingsButton.accesskey                        "c">
> +<!ENTITY  cameraSettingsButton.accesskey                        "e">
>  
>  <!ENTITY  microphonePermissions.label                           "Microphone">
> -<!ENTITY  microphoneSettingsButton.accesskey                    "m">
> +<!ENTITY  microphoneSettingsButton.accesskey                    "t">

We should just make all of these "S". Trying to get a unique access key here isn't really possible given all the places that these buttons can appear (preferences search) and there are quite a lot of buttons on the page already.

Comment 5

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

flod, do we need to rev the access keys here? Would we need to rev the entity name for the Settings label? It seems they all use the same entity ("permissionSettingsButton2.label") for the label (maybe that's another problem).

Comment 6

[Francesco Lodolo [:flod]]

1. If you need to change access keys in English, there's no need to REV the string ID, just change them. Each locale has its own access keys.
2. Label and access key should follow the structure ID.label and ID.accesskey, if you change one ID, you should change the other too.

Having said that, using a string called microphoneSettingsButton.accesskey as an access key for a label permissionSettingsButton2.label doesn't make any sense. There should be one access key associated to that label, and no other accesskeys.

Comment 7

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

Thanks, that is my understanding as well. I just wanted to make sure.

Comment 8

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

Comment on attachment 8898920 [details]
Bug 1390509 - Fix access keys for location, camera and microphone permissions.

https://reviewboard.mozilla.org/r/170280/#review175486

::: browser/locales/en-US/chrome/browser/preferences/content.dtd:17
(Diff revision 1)
>  <!ENTITY  notificationsDoNotDisturbDetails.value "No notification will be shown until you restart &brandShortName;">
>  
>  <!ENTITY  popupExceptions.label                                 "Exceptions…">
>  <!ENTITY  popupExceptions.accesskey                             "E">
>  
>  <!ENTITY  permissionSettingsButton2.label                       "Settings…">

Please remove this string, and add a new one for locationSettingsButton.label, cameraSettingsButton.label, and microphoneSettingsButton.label.

They should all be equal to "Settings…", and should appear next to their associated accesskeys in this file.

Comment 9

[:prathiksha]

Comment on attachment 8898920 [details]
Bug 1390509 - Fix access keys for location, camera and microphone permissions.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/170280/diff/1-2/

Comment 10

[:prathiksha]

I made the changes but 'S' isn't working as an accesskey. 't' works. How about we use 't' instead ?

Comment 11

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

Comment on attachment 8898920 [details]
Bug 1390509 - Fix access keys for location, camera and microphone permissions.

https://reviewboard.mozilla.org/r/170280/#review175898

Looks good, thanks! Please change this to 't' since 'S' isn't working.

Comment 12

[:prathiksha]

Comment on attachment 8898920 [details]
Bug 1390509 - Fix access keys for location, camera and microphone permissions.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/170280/diff/2-3/

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a486d2393755
Fix access keys for location, camera and microphone permissions. r=jaws

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/a486d2393755