|Submitter||Diff||Changes||Open Issues||Last Updated|
|Error loading review requests:|
59 bytes, text/x-review-board-request
|Details | Review|
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.
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 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.
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).
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.
Thanks, that is my understanding as well. I just wanted to make sure.
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.
I made the changes but 'S' isn't working as an accesskey. 't' works. How about we use 't' instead ?
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.
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/a486d2393755 Fix access keys for location, camera and microphone permissions. r=jaws