Sync UI: When printing or saving a sync key, the dialog prompts you to change it

RESOLVED FIXED in mozilla6

Status

()

defect
RESOLVED FIXED
8 years ago
7 months ago

People

(Reporter: faaborg, Assigned: emtwo)

Tracking

unspecified
mozilla6
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][verified in services])

Attachments

(2 attachments, 2 obsolete attachments)

In sync preferences, when you go to "Manage account > my sync key" the resulting dialog box says "Change your Sync Key."  It's possible that the user wanted to change it, although it is equally possibly that they simply wanted to view it to back up a copy.  Prompting them to change their key might actually cause them to change it needless.

Additionally, we no longer allow user modified sync keys, to ensure the best level of encryption.

Suggested changes:
-Change dialog title to "My Sync Key"
-Change "Generate" link to "Generate a new key"
Whiteboard: [good first bug]
(Assignee)

Updated

8 years ago
Assignee: nobody → msamuel
(Assignee)

Comment 1

8 years ago
Posted patch patch for bug 652483 (obsolete) — Splinter Review
Attachment #529013 - Flags: review?(rnewman)
Comment on attachment 529013 [details] [diff] [review]
patch for bug 652483

This isn't a complete fix, if my understanding is correct. (It might not be -- I've had a couple of cocktails so far this evening!)

This approach requires localization; you're changing the string for "Change your Sync Key" in en-US only. We have something like 88 locales, all told.

You might be better off changing the XUL to refer to mySyncKey.label, which is the English string "My Sync Key" (granted, it's "my" not "your"), and eliminating the change.synckey entity altogether. (That might require another bug to follow up with localizers for cleanup, though. I'm not too familiar with the process there.)

That said, you need to change syncKeyGenerate.label, so you're going to hit the localization issue anyway.

My suggestion is to find out who can give you a firm answer about whether this has localization impact. If it doesn't, you get r+ from me (though I don't like "change.synckey.title" not referring to "change"!). If it does, then we need to make sure we coordinate localization work, and decide whether anything else needs to happen.

I'll leave a channel-hanger to see if someone can weigh in on this.
Attachment #529013 - Flags: review?(rnewman)
(In reply to comment #3)
> This isn't a complete fix, if my understanding is correct. (It might not be --
> I've had a couple of cocktails so far this evening!)

Functionally it's complete, at least as far as I understand Alex in comment 0. However, technically the patch is not sufficient. Strings may never be changed while keeping the same string ID. If a string change is required, a new string ID has to be chosen (if no good new name suggests itself, one can simply add a "2").

This means you want to replace 'change.synckey.title' with 'mySyncKey.title' and 'syncKeyGenerate.label' with 'generateNewSyncKey.label' or something similar.

> This approach requires localization; you're changing the string for "Change
> your Sync Key" in en-US only. We have something like 88 locales, all told.

This sounds like you want to make Marina do the localization for all of our 80 something locales ;). m-c (and therefore s-c) are en-US only, nobody has to ever worry about translations when making changes there. We're not string frozen, so string changes are perfectly fine.
(Assignee)

Comment 5

8 years ago
(In reply to comment #4) 
> This means you want to replace 'change.synckey.title' with 'mySyncKey.title'
> and 'syncKeyGenerate.label' with 'generateNewSyncKey.label' or something
> similar.

Is it ok to make these replacements within the same file?  The file syncGenericChange.properties seems to contain mostly strings starting with 'change.*' the string 'mySyncKey.title' doesn't seem to fit in well with the rest then.  The 'generateNewSyncKey.label' seems ok within the same file to me though.
(In reply to comment #5)
> Is it ok to make these replacements within the same file?

Yup.

> The file
> syncGenericChange.properties seems to contain mostly strings starting with
> 'change.*' the string 'mySyncKey.title' doesn't seem to fit in well with the
> rest then.  The 'generateNewSyncKey.label' seems ok within the same file to me
> though.

The string IDs are made up and have no special meaning. But you do have a good point about maintaining consistency regarding the change.* prefix. You could make it change.mySyncKey.title or change.synckey2.title or something else. Up to you, really :)
(Assignee)

Comment 7

8 years ago
Posted patch patch for bug 652483 (obsolete) — Splinter Review
- Kept original string constants & added new ones in the .properties & .dtd files
- Changed the constants being referenced in the .js & .xul files

is this better?
Attachment #529013 - Attachment is obsolete: true
Attachment #529140 - Flags: review?(philipp)
Comment on attachment 529140 [details] [diff] [review]
patch for bug 652483

Yup that's better, but you can get rid of the old strings, unless they're still being used somewhere. I doubt it, but please double check, though.
Attachment #529140 - Flags: review?(philipp) → review+
(Assignee)

Comment 9

8 years ago
removed the unused strings
Attachment #529140 - Attachment is obsolete: true
Attachment #529147 - Flags: review?(philipp)
Attachment #529147 - Flags: review?(philipp) → review+
http://hg.mozilla.org/services/services-central/rev/f3eba0269bb2
Keywords: checkin-needed
Whiteboard: [good first bug] → [good first bug][fixed in services]
Verified with s-c builds of 20110502
Whiteboard: [good first bug][fixed in services] → [good first bug][verified in services]
http://hg.mozilla.org/mozilla-central/rev/f3eba0269bb2
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
Blocks: sync2sm
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.