834905 - Add Alt+D shortcut to focus the location bar in Metro.

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: Browser, defect)

Description

[Matt Brubeck (:mbrubeck)]

Attachment: patch

Now that bug 790486 is fixed, we can add Alt-D as an alias for Control-L, to match desktop Firefox.

Comment 1

[Frank Yan (:fryn)]

Comment on attachment 706599 [details] [diff] [review]
patch

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

::: browser/metro/base/content/browser.xul
@@ +128,5 @@
>      <key id="key_reload2" key="r" modifiers="accel" command="cmd_reload"/>
>      <key id="key_forceReload" keycode="VK_F5" modifiers="shift" command="cmd_forceReload"/>
>      <key id="key_forceReload2" key="r" modifiers="accel,shift" command="cmd_forceReload"/>
>      <key id="key_focusURL" key="l" modifiers="accel" command="cmd_openLocation"/>
> +    <key id="key_focusURL2" key="d" modifiers="alt" command="cmd_openLocation"/>

In Firefox desktop, this key value varies by locale (perhaps since it's an accesskey?).
https://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-sets.inc#226
https://mxr.mozilla.org/l10n-central/search?string=urlbar.accesskey

In Firefox desktop, the key value for accel+l is also defined in a DTD, but it seems to be "l" for all locales as expected.
https://mxr.mozilla.org/l10n-central/search?string=openCmd.commandkey

What's our localization plan for Metro? I don't think it's very useful to put values that we want to be the same for all locales (e.g. "l" in accel+l) in DTDs, but if we want the "d" in alt+d to vary by locale (as accesskeys generally do), then we'll need that, though I wouldn't have it block a v1.

Comment 2

[Matt Brubeck (:mbrubeck)]

Attachment: patch

Good catch.  Since I'm mainly adding this for consistency with desktop Firefox, it seems important to go for consistency in all locales.

(But I won't bother adding an entity for control-L, since that one is not localized in practice.)

Comment 3

[Frank Yan (:fryn)]

Comment on attachment 707272 [details] [diff] [review]
patch

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

(In reply to Matt Brubeck (:mbrubeck) from comment #2)
> (But I won't bother adding an entity for control-L, since that one is not
> localized in practice.)

Cool, I agree. :)
Less unnecessary overhead in code and for localizers.

Comment 4

[Matt Brubeck (:mbrubeck)]

http://hg.mozilla.org/projects/elm/rev/94256301c753