1211975 - Inconsistent letter case and spacing in file abMainWindow.dtd

Status: RESOLVED FIXED

(Thunderbird :: Address Book, defect)

Description

[Giulio Portioli [:gportioli]]

Spin-off from bug 364133. File comm-central/mail/locales/en-US/chrome/messenger/addressbook/abMainWindow.dtd contains several .accesskey entities defined with a case that is not consistent with the upper/lowercase target letter in the corresponding .label entity (e.g. fileMenu.label = "File", fileMenu.accesskey = "f"). Best practice is for the case of the access key to match the case of the target letter in the label (i.e. "F" for "File").

The last two blocks of entities in the DTD ("Status Bar" and "Mac OS X Window Menu") also have inconsistent spacing relatively to the rest of the file.

Patch with tidied up DTD to follow shortly.

Comment 1

[Giulio Portioli [:gportioli]]

Attachment: Patch v1 - Letter case and spacing tidied up

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8673284 [details] [diff] [review]
Patch v1 - Letter case and spacing tidied up

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

Since accesskey's are (according to https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/Attribute/accesskey), case insensitive, I think this is a fine cosmetic change.

I think we've missed a few though.

::: mail/locales/en-US/chrome/messenger/addressbook/abMainWindow.dtd
@@ +31,2 @@
>  <!ENTITY printSetupCmd.label                            "Page Setup…">
>  <!ENTITY printSetupCmd.accesskey                        "u">

Missed this one.

@@ +31,4 @@
>  <!ENTITY printSetupCmd.label                            "Page Setup…">
>  <!ENTITY printSetupCmd.accesskey                        "u">
>  <!ENTITY printPreviewContactCmd.label                   "Print Preview Contact">
>  <!ENTITY printPreviewContactCmd.accesskey               "v">

Missed this one.

@@ +52,3 @@
>  <!ENTITY cutCmd.label                                   "Cut">
>  <!ENTITY cutCmd.key                                     "X">
>  <!ENTITY cutCmd.accesskey                               "t">

Missed this one.

@@ +73,2 @@
>  <!ENTITY swapFirstNameLastNameCmd.label                 "Swap First/Last Name">
>  <!ENTITY swapFirstNameLastNameCmd.accesskey             "w">

Missed this one.

@@ +75,5 @@
>  <!-- LOCALIZATION NOTE (hideSwapFnLnUI) : DONT_TRANSLATE -->
>  <!-- Swap FN/LN UI  Set to "false" to show swap fn/ln UI -->
>  <!ENTITY hideSwapFnLnUI "true">
>  <!ENTITY propertiesCmd2.label                           "Properties">
>  <!ENTITY propertiesCmd2.accesskey                       "i">

Missed this one.

@@ +125,5 @@
>  <!ENTITY exportCmd.accesskey                            "E">
>  <!ENTITY preferencesCmd2.label                          "Options">
>  <!ENTITY preferencesCmd2.accesskey                      "O">
>  <!ENTITY preferencesCmdUnix.label                       "Preferences">
>  <!ENTITY preferencesCmdUnix.accesskey                   "n">

Missed this one.

Comment 3

[Giulio Portioli [:gportioli]]

Do you mean 'missed' as in 'left in lowercase instead of changed to uppercase'? That was actually intended.

The full paragraph from the MDN doc reads: "Although the value is case insensitive, a letter with the case matching the accesskey attribute will be used if both cases exist in the label", so it seems preferable for the case of the accesskey to always match the case of the intended target letter in the label, even if that letter is unique in the label - for clarity, if nothing else.

The examples you listed are all instances of the above principle. I agree that it's all rather nit-picking, as the original reporter said (bug 364133, comment No. 2) but I thought I would fix it while I was at it.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8673284 [details] [diff] [review]
Patch v1 - Letter case and spacing tidied up

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

Ah, good eye. Sorry for the delay, thanks!

Comment 5

[:aceman]

https://hg.mozilla.org/comm-central/rev/32b027e2ec43