Closed Bug 691854 Opened 8 years ago Closed 8 years ago

Double period in Firefox Sync server maintenance message

Categories

(Firefox :: Sync, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla11

People

(Reporter: dholbert, Assigned: nigelb)

Details

(Whiteboard: [good first bug][polish][fixed in services])

Attachments

(2 files, 1 obsolete file)

I'm currently getting this Sync error:
> Sync encountered an error while syncing: Firefox Sync server maintenance
> is underway, syncing will resume automatically.. Sync will automatically
> retry this action.

Filing this bug on the double-period after "automatically".

I'm guessing this is from the expanded string "%1$S" including a period *inside* of it, in the following string:
> 22 error.sync.description = Sync encountered an error while syncing: %1$S.  Sync will automatically retry this action.
http://mxr.mozilla.org/mozilla-central/source/services/sync/locales/en-US/sync.properties#22
Whiteboard: [good-first-bug][polish]
See screenshot in attachment 564612 [details] (from bug 691849).

Mozilla/5.0 (X11; Linux x86_64; rv:10.0a1) Gecko/20111003 Firefox/10.0a1
Whiteboard: [good-first-bug][polish] → [good first bug][polish]
Attached patch patch-v1 (obsolete) — Splinter Review
Removing the . from here, because there's already a '.' at the end of the sentence its inserted in http://mxr.mozilla.org/mozilla-central/source/services/sync/locales/en-US/sync.properties#22
Assignee: nobody → nigelbabu
Status: NEW → ASSIGNED
Attachment #579443 - Flags: review?(ally)
Attachment #579443 - Attachment is patch: true
Attachment #579443 - Flags: review?(ally) → review?(philipp)
Hey Axel, do we need to rev the string ID for a punctuation change (extraneous period at the end of the string in this case.)?
Comment on attachment 579443 [details] [diff] [review]
patch-v1

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

I'd love to say "no change needed", but http://mxr.mozilla.org/l10n-mozilla-aurora/search?string=reason.server_maintenance&find=services/sync proves the contrary. That bug is basically in all our localizations.

I suggest to rename it to something like error.sync.reason.server_maintenance_no_dot to make the change obvious.
Attachment #579443 - Flags: review?(philipp) → review-
(In reply to Axel Hecht [:Pike] from comment #4)
> I suggest to rename it to something like
> error.sync.reason.server_maintenance_no_dot to make the change obvious.

Hmm, except the name of the string is actually the value of the error constant in services/sync/constants.js... But yeah, ok. Thanks for checking!
Attached patch patch-v2Splinter Review
Changed the entity as well.
Attachment #579443 - Attachment is obsolete: true
Attachment #579944 - Flags: review?(philipp)
Comment on attachment 579944 [details] [diff] [review]
patch-v2

Looks good!
Attachment #579944 - Flags: review?(philipp) → review+
Attached patch patch-v3Splinter Review
Patch with r+
Whiteboard: [good first bug][polish] → [good first bug][polish][checkin-needed]
Keywords: checkin-needed
Whiteboard: [good first bug][polish][checkin-needed] → [good first bug][polish]
Pushed to s-c: https://hg.mozilla.org/services/services-central/rev/bd58575a3bbe
Keywords: checkin-needed
OS: Linux → All
Hardware: x86_64 → All
Whiteboard: [good first bug][polish] → [good first bug][polish][fixed in services]
Pushed to m-c: https://hg.mozilla.org/mozilla-central/rev/bd58575a3bbe
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
Comment on attachment 579944 [details] [diff] [review]
patch-v2

-error.sync.reason.server_maintenance = Firefox Sync server maintenance is underway, syncing will resume automatically.
+# LOCALIZATION NOTE (error.sync.reason.serverMaintenance): We removed the extraneous period from this string
+error.sync.reason.serverMaintenance  = Firefox Sync server maintenance is underway, syncing will resume automatically

I'm tempted to say the comment should be removed in one or two cycles. It has no use for those who haven't localised the previous string, and for those who have, it's a one-time message. No need to keep it around.

Since this string is already in Aurora, perhaps it would already be okay to drop the comment from comm-central?
Component: Firefox Sync: UI → Sync
Product: Cloud Services → Firefox
You need to log in before you can comment on or make changes to this bug.