372794 - Unnecessary string errCode in ldapAutoCompErrs.properties

Status: RESOLVED FIXED

(MailNews Core :: LDAP Integration, defect)

Description

[Mark Banner (:standard8)]

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/mail/locales/en-US/chrome/messenger/addressbook/ldapAutoCompErrs.properties&rev=1.3&mark=65-78#65

The errCode string gets loaded in the autocomplete code and then formatted into alertFormat - always as the first item. errCode doesn't get used anywhere else, nor does alertFormat.

I suggest we just combine these two strings into one and save the bloat.

Comment 1

[Mark Banner (:standard8)]

Attachment: The fix

I'm not altogether sure its possible to display this output currently. However, we may need it later, and this will at least reduce the really unnecessary code.

Comment 2

[David :Bienvenu]

Comment on attachment 257552 [details] [diff] [review]
The fix

thx for the patch, Mark. If you change a string like alertFormat, you need to add a new property name like alertFormat1, so that the localizers know they need to change the string. This apparently will cause the different localization tinderbox builds to break, so the localizers know they need to change the string...

other than that, the patch looks ok.

Comment 3

[Mark Banner (:standard8)]

(In reply to comment #2)
> (From update of attachment 257552 [details] [diff] [review])
> thx for the patch, Mark. If you change a string like alertFormat, you need to
> add a new property name like alertFormat1, so that the localizers know they
> need to change the string. This apparently will cause the different
> localization tinderbox builds to break, so the localizers know they need to
> change the string...

Is that any string that we change the text on, or just ones with replacements in? If its any strings, then that'll severely limit us where we map number codes to strings.... Where has this been specified?

Comment 4

[[e-mail]]

Comment on attachment 257552 [details] [diff] [review]
The fix

I'll give r=me now because it will still apply once you've renamed the string to David's satisfaction :-)

Comment 5

[David :Bienvenu]

As I understand it, it's any string you significantly change the semantic meaning of.

I'm not sure where these rules/guidelines are written down. I learned about it like you - through review comments :-) I've cc'ed Axel - he might have more info/input.

Yes, that's unfortunate about mapping number codes to strings. Not sure what we can do about that - perhaps post to the l10n group?

Comment 6

[Axel Hecht [:Pike]]

It's written down at http://developer.mozilla.org/en/docs/Writing_localizable_code, which holds true for semantic key names. If error codes really satisfy that is a totally different question, on the other hand, their meaning shouldn't change.

I'd much rather change the key name to errorAlertFormat, which is a good name for what it is, than number it. Just consider the key names to be variable names. You changed it from a general alert format to an error alert format, which you then camel case and yay, fine name.

Apart from that, a 'yay' on cleaning this up, that looked like an odd nightmare to localize.

Comment 7

[Mark Banner (:standard8)]

Attachment: The fix v2

Thanks, at least I know why I'm changing things now :-)

Carrying forward Neil's r.

Comment 8

[David :Bienvenu]

Comment on attachment 257832 [details] [diff] [review]
The fix v2

thx, Mark. I'm sure our 40+ localizations thank you too :-)

Comment 9

[Mark Banner (:standard8)]

Patch checked in.