1305407 - Rename accesskeys in sync.dtd as .accesskey instead of .label.accesskey

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect)

Description

[Francesco Lodolo [:flod]]

https://hg.mozilla.org/mozilla-central/rev/09bb9527ecc0

As explained in https://bugzilla.mozilla.org/show_bug.cgi?id=1028029#c22 some localization tools try to match a label with its accesskey: if the label is STRING.label, the accesskey should be STRING.accesskey.

In this case all new accesskeys were created as STRING.label.accesskey instead.

Comment 1

[Francesco Lodolo [:flod]]

One more question: rejectUnlinkFxaAccount.forget.label.accesskey

It seems to be used for forget.label, why the change?

Comment 2

[Fischer [:Fischer]]

(In reply to Francesco Lodolo [:flod] from comment #0)
> https://hg.mozilla.org/mozilla-central/rev/09bb9527ecc0
> 
> As explained in https://bugzilla.mozilla.org/show_bug.cgi?id=1028029#c22
> some localization tools try to match a label with its accesskey: if the
> label is STRING.label, the accesskey should be STRING.accesskey.
> 
> In this case all new accesskeys were created as STRING.label.accesskey
> instead.

Thanks for reminding this. I will update the sync.dtd.

Comment 3

[Fischer [:Fischer]]

(In reply to Francesco Lodolo [:flod] from comment #1)
> One more question: rejectUnlinkFxaAccount.forget.label.accesskey
> 
> It seems to be used for forget.label, why the change?

Originally just to separate button#unverifiedUnlinkFxaAccount accesskey from button#rejectUnlinkFxaAccount in case that one day maybe they would have different label.
However as your comment, this would confuse localization tools so will update to "forget.accesskey" along with "forget.label".

Comment 4

[Fischer [:Fischer]]

Attachment: Bug 1305407 - Rename accesskeys in sync.dtd as .accesskey instead of .label.accesskey.

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

Comment 5

[Fischer [:Fischer]]

Comment on attachment 8796054 [details]
Bug 1305407 - Rename accesskeys in sync.dtd as .accesskey instead of .label.accesskey.

This updates .label.accesskey to .accesskey to avoid confusing localisation toools as :flod's comment, thanks.

Comment 6

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

Comment on attachment 8796054 [details]
Bug 1305407 - Rename accesskeys in sync.dtd as .accesskey instead of .label.accesskey.

https://reviewboard.mozilla.org/r/82044/#review82512

Normally I think if the accesskey entity name changes then we should change the label access key so that they both change at the same time and stay consistent with each other, but this time I think it's OK to not change the label entity names because this isn't really about changing the accesskeys with the exception of the rejectUnlinkFxaAccount one.

::: browser/locales/en-US/chrome/browser/preferences/sync.dtd:85
(Diff revision 1)
> -<!ENTITY forget.label.accesskey            "F">
> +<!ENTITY forget.accesskey            "F">
> -<!ENTITY rejectUnlinkFxaAccount.forget.label.accesskey            "e">

Are you sure that we can use 'F' for the rejectUnlinkFxaAccount label's accesskey now? This patch changes it from 'e' to 'F'.

Comment 7

[Fischer [:Fischer]]

(In reply to Jared Wein [:jaws] (overloaded with reviews / please needinfo? me) from comment #6)
> Comment on attachment 8796054 [details]
> Bug 1305407 - Rename accesskeys in sync.dtd as .accesskey instead of
> .label.accesskey
> 
> https://reviewboard.mozilla.org/r/82044/#review82512
> 
> Normally I think if the accesskey entity name changes then we should change
> the label access key so that they both change at the same time and stay
> consistent with each other, but this time I think it's OK to not change the
> label entity names because this isn't really about changing the accesskeys
> with the exception of the rejectUnlinkFxaAccount one.
> 
> ::: browser/locales/en-US/chrome/browser/preferences/sync.dtd:85
> (Diff revision 1)
> > -<!ENTITY forget.label.accesskey            "F">
> > +<!ENTITY forget.accesskey            "F">
> > -<!ENTITY rejectUnlinkFxaAccount.forget.label.accesskey            "e">
> 
> Are you sure that we can use 'F' for the rejectUnlinkFxaAccount label's
> accesskey now? This patch changes it from 'e' to 'F'.
Yes, tested and works. Only one of the button#unverifiedUnlinkFxaAccount and the button#rejectUnlinkFxaAccount would be visible under a given moment so "F" would only be triggered on one of them.

Comment 8

[Francesco Lodolo [:flod]]

Setting NI, since this bug never landed anywhere, and I assume something needs to be done in MozReview.

Comment 9

[Fischer [:Fischer]]

Comment on attachment 8796054 [details]
Bug 1305407 - Rename accesskeys in sync.dtd as .accesskey instead of .label.accesskey.

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

Comment 10

[Fischer [:Fischer]]

Rebase to the latest central.
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=08c2a0e8aa24&selectedJob=29006585

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4a88ccd4c485
Rename accesskeys in sync.dtd as .accesskey instead of .label.accesskey. r=jaws

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4a88ccd4c485