Check trailing spaces in string firstLastFormat in TB AB

RESOLVED FIXED in Thunderbird 52.0

Status

defect
--
trivial
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: aceman, Assigned: aceman)

Tracking

Trunk
Thunderbird 52.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

1.91 KB, patch
mkmelin
: review+
iann_bugzilla
: review+
Details | Diff | Splinter Review
Assignee

Description

3 years ago
+++ This bug was initially created as a clone of Bug #1312262 comment 13 +++

:aceman 2016-10-30 18:08:08 CET
::: mail/locales/en-US/chrome/messenger/addressbook/addressBook.properties
@@ +11,3 @@
>  emptyListName=You must enter a list name.
>  lastFirstFormat=%S, %S
>  firstLastFormat=%S %S  

Heh, IF really these trailing spaces are required for proper operation, maybe it would use a localization note. Someone's eager editor may automatically strip trailing space.

In this bug find out if the trailing spaces in string "firstLastFormat" are really needed. Even if yes, that seems improper usage and should be solved differently.
Assignee

Updated

3 years ago
Summary: Make the dialog titles in AB consistent and dynamic → Check trailing spaces in string firstLastFormat in TB AB
Assignee

Comment 1

3 years ago
It seems fetching the strings from a properties file already automatically trims trailing spaces.

We even have a test at
https://dxr.mozilla.org/comm-central/rev/6b8c7cd17855f7de5eccb0ea9efcd5dbccc68573/mailnews/addrbook/test/unit/test_basic_nsIAbCard.js#54 that checks the resulting output:
do_check_eq(card.generateName(1), kLNValue + ", " + kFNValue);
do_check_eq(card.generateName(2), kFNValue + " " + kLNValue);

And it expects there to be no trailing spaces. So I will remove them.

More of a problem is that translators already found this bug:
https://dxr.mozilla.org/l10n-central/search?q=firstLastFormat&redirect=false

Some of them just trimmed the spaces, some preserved them some even made them obvious by forced \u0020 character (which may or may not have made the spaces actually preserved in the string).

How do we proceed here? Should I just change the string name? That would be unfortunate as it should match the schema of the other string name (lastFirstFormat).
Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(iann_bugzilla)
Assuming it doesn't really matter if translators put spaces or \u0020 there, I'd just fix the en-US string. If someone picks it up that's fine, but if not that doesn't change operations.
Flags: needinfo?(mkmelin+mozilla)
Assignee

Comment 3

3 years ago
Posted patch patchSplinter Review
Flags: needinfo?(iann_bugzilla)
Attachment #8809993 - Flags: review?(mkmelin+mozilla)
Attachment #8809993 - Flags: review?(iann_bugzilla)
Attachment #8809993 - Flags: review?(mkmelin+mozilla) → review+

Comment 4

3 years ago
Comment on attachment 8809993 [details] [diff] [review]
patch

r/a=me
Attachment #8809993 - Flags: review?(iann_bugzilla) → review+
Assignee

Comment 5

3 years ago
Thanks. I will post to to localization newsgroup, but we do not change the string IDs.
Keywords: checkin-needed
Assignee

Comment 6

3 years ago
https://hg.mozilla.org/comm-central/rev/26d7f9ff1aba6d7b0ac94e5c87b78dd66b245e99
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Keywords: checkin-needed
Product: Thunderbird → MailNews Core
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 52.0
You need to log in before you can comment on or make changes to this bug.