412793 - update.properties could use comments

Status: RESOLVED DUPLICATE of bug 357145

(Toolkit :: Application Update, defect)

Description

[Axel Hecht [:Pike]]

update.properties is a pretty cryptic file to localize, it's full of string and parameter substitutions without any comment whatsoever.

This is somewhat split off of bug 391598, but that one just added a few more undocumented strings.

Comment 1

[Axel Hecht [:Pike]]

PS: see for tips on http://developer.mozilla.org/en/docs/Localization_notes, too.

Comment 2

[Dão Gottwald [:dao]]

Attachment: add notes

Comment 3

[Marek Stępień [:marcoos, inactive]]

Comment on attachment 297938 [details] [diff] [review]
add notes

You're removing some of the strings, not just adding l10n notes:

+# LOCALIZATION NOTE (updateMoreInfoContentDownloading): First %s: product name (brandShortName), second %s: version identifier (e.g. 3.0.1)
 updateMoreInfoContentDownloading=Getting more details about %S %S…
-statusSucceededFormat=Installed on: %S
-statusFailed=Install Failed
...
-rateFormat= at #1
-progressFormat=#1#2

Comment 4

[Axel Hecht [:Pike]]

bug 405044 removed the use of statusSucceededFormat and statusFailed, AFAICT (and mxr) without removing the entities. The other two formats show up as removed lines in the diff, but are added, too, so those are just artifacts of diff.

Comment 5

[Axel Hecht [:Pike]]

Is our update naming story really as inconsistent as lxr makes me think? I.e., nsIUpdate.name is defined to be what updateName is, or what <update> said in the xml, but we're not using that value most of the time, and head back to format via updateName again? Or is that intentional?

updateFullName on the other hand is used in http://mxr.mozilla.org/mozilla/source/toolkit/mozapps/update/content/history.js#66, and does reference update.name, i.e., could, if we set it, show something different to "Firefox (3.0.1)".

Comment 6

[Dão Gottwald [:dao]]

(In reply to comment #5)
> Is our update naming story really as inconsistent as lxr makes me think? I.e.,
> nsIUpdate.name is defined to be what updateName is, or what <update> said in
> the xml, but we're not using that value most of the time, and head back to
> format via updateName again? Or is that intentional?

I think we are actually using update.name most of the time, but maybe not all over where we could. The direct use of updateName instead of update.name in update.js does look wrong to me.

> updateFullName on the other hand is used in
> http://mxr.mozilla.org/mozilla/source/toolkit/mozapps/update/content/history.js#66,
> and does reference update.name, i.e., could, if we set it, show something
> different to "Firefox (3.0.1)".

updateFullName should look like this: "Firefox 3.0.1 (20080401)", unless the update server sets a name: "April Security Update for Firefox 3 (20080401)".

Comment 7

[Axel Hecht [:Pike]]

Comment on attachment 297938 [details] [diff] [review]
add notes

r-, most importantly, the comments should all talk about %S instead of %s.

I'm wondering, would you mind adding explicit numbering to the entries with multiple %S? It's easier to read the comment "%1$S is brandShortName", in particular when you're updating a localization that changed the order. Doing so will be purely internal to this file, it will blow up the patch size, though.

Could we come up with a better story for updateName? Something like is updateName or the name sent from AUS. Or so. I'm not happy with that myself, I just feel awkward hiding the variety of input that AUS can be configured to insert here.

Mind filing the bug on the update.js updateName use?

Comment 8

[Robert Strong (they/them - no direct email)]

There are a decent amount of comments now and the remaining work can be finished up in bug 357145