Rename *-key messages to *-command

RESOLVED FIXED

Status

L20n
General
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: stas, Assigned: gandalf)

Tracking

(Blocks: 1 bug)

Details

(Whiteboard: [gecko-l20n])

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

a year ago
For example:

    new-navigator-key =
        [xul/key]        N

should become:

    new-navigator-command =
        [xul/key]        N
(Reporter)

Updated

a year ago
Blocks: 1291693
(Assignee)

Updated

a year ago
Assignee: nobody → gandalf
(Assignee)

Comment 1

a year ago
Created attachment 8804056 [details] [diff] [review]
patch v1

Renaming -key to -command plus one OS()
Attachment #8804056 - Flags: review?(stas)
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8804056 - Attachment is obsolete: true
Attachment #8804056 - Flags: review?(stas)
(Reporter)

Comment 3

a year ago
mozreview-review
Comment on attachment 8804057 [details]
Bug 1309561 - Rename *-key messages to *-command;

https://reviewboard.mozilla.org/r/88204/#review87340

This should not land as it is right now, but the fixes are simple. Thanks.

::: browser/base/content/browser-menubar.inc
(Diff revision 1)
> -#else
> -#ifdef XP_MACOSX
> -                          data-l10n-id="quit-application-menuitem-mac"
> -#else
>                            data-l10n-id="quit-application-menuitem"
> -#endif

What is the reason for introducing this OS() in this patch?

::: browser/base/content/browser-menubar.inc:94
(Diff revision 1)
>                            data-l10n-id="quit-application-menuitem"
> -#endif
>  #ifdef XP_UNIX
>                            key="key_quitApplication"
>  #endif
>  #endif

There's one #endif too many here which breaks the build.  Try building locally or pushing to try.

::: browser/base/content/browser-sets.inc:238
(Diff revision 1)
> -    <key id="key_close" data-l10n-id="close-key" command="cmd_close" modifiers="accel"/>
> -    <key id="key_closeWindow" data-l10n-id="close-key" command="cmd_closeWindow" modifiers="accel,shift"/>
> +    <key id="key_close" data-l10n-id="close-command" command="cmd_close" modifiers="accel"/>
> +    <key id="key_closeWindow" data-l10n-id="close-command" command="cmd_closeWindow" modifiers="accel,shift"/>
>      <key id="key_toggleMute" key="&toggleMuteCmd.key;" command="cmd_toggleMute" modifiers="control"/>
> -    <key id="key_undo" data-l10n-id="undo-key" modifiers="accel"/>
> +    <key id="key_undo" data-l10n-id="undo-command" modifiers="accel"/>
>  #ifdef XP_UNIX
> -    <key id="key_redo" data-l10n-id="undo-key" modifiers="accel,shift"/>
> +    <!-- On unix, redo is ctrl+shift+z => undo key + shift -->

I would phrase that as "On Unix, Redo's command key is the Undo's command key + Shift."

::: browser/locales/en-US/browser/menubar.ftl:134
(Diff revision 1)
>      [xul/key]        A
>  
>  find-on-menuitem =
>      [xul/label]      Find in This Page…
>      [xul/accesskey]  F
> -find-on-key =
> +find-on-command1 =

This should be `find-on-command =`.

::: browser/locales/en-US/browser/menubar.ftl:139
(Diff revision 1)
> -find-on-key =
> +find-on-command1 =
>      [xul/key]        f
>  find-again-menuitem =
>      [xul/label]      Find Again
>      [xul/accesskey]  g
> -find-again-key1 =
> +find-again-command =

And this should be `find-again-command1 =`.
Attachment #8804057 - Flags: review?(stas) → review-
Comment hidden (mozreview-request)
(Assignee)

Comment 5

a year ago
Created attachment 8804339 [details] [diff] [review]
key2command.patch

manual patch in case mozreview one is too confusing.
(Reporter)

Comment 6

a year ago
Comment on attachment 8804339 [details] [diff] [review]
key2command.patch

I'm not able to r+ in mozreview, so r=me on this squashed patch.  Thanks!
Attachment #8804339 - Flags: review+
(Assignee)

Comment 8

a year ago
Landed: http://hg.mozilla.org/projects/larch/rev/cd69a5db20d5
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
(Reporter)

Comment 9

a year ago
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #8)
> Landed: http://hg.mozilla.org/projects/larch/rev/cd69a5db20d5

This is not the patch that I r+'ed.  It contains the OS() builtin and removes the #ifdef in browser/base/content/browser-menubar.inc.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 10

a year ago
https://hg.mozilla.org/projects/larch/rev/0c5eca45fc7f3eee4295e70f77110869a0196a87
Bug 1309561 - Follow-up to the previous patch. Land the review feedback fixes. r=stas
(Assignee)

Comment 11

a year ago
Aaand, follow-up landed: http://hg.mozilla.org/projects/larch/rev/0c5eca45fc7f
Status: REOPENED → RESOLVED
Last Resolved: a year agoa year ago
Resolution: --- → FIXED
(Reporter)

Comment 12

a year ago
mozreview-review
Comment on attachment 8804057 [details]
Bug 1309561 - Rename *-key messages to *-command;

https://reviewboard.mozilla.org/r/88204/#review90092
Attachment #8804057 - Flags: review?(stas)
You need to log in before you can comment on or make changes to this bug.