596566 - Sync UI: Text for the physical Sync Key artifact

Status: VERIFIED FIXED

(Firefox :: Sync, defect)

Description

[Philipp von Weitershausen [:philikon]]

+++ This bug was initially created as a clone of Bug #596565 +++

As per bug 591533 comment 14.

Comment 1

[Mike Beltzner [:beltzner, not reading bugmail]]

Just copying that comment over here:

Strings:

 - &userName's Firefox Sync Key

 - This key is used to decode the data in your Firefox Sync account. You will
need to enter the key each time you configure Firefox Sync on a new computer or
device.

 - Keep it secret

 - Your Firefox Sync account is encrypted to protect your privacy. Without this
key, it would take years for anyone to decode your personal information. You
are the only person who holds this key. This means you're the only one who can
access your Firefox Sync data.

 - Keep it safe

 - _Do not lose this key._ We don't keep a copy of your key (that wouldn't be
keeping it secret!) so _we can't help you recover it_ if it's lost. You'll need
to use this key any time you connect a new computer or device to Firefox Sync.

 - Find out more about the Firefox Sync and your privacy at
http://services.mozilla.com

(Many, many thanks to Alex for his work on this to date in bug 591533; I am truly iterating
on the shoulders of giants while mixing metaphors like rhymes.)

Comment 2

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to bug 591533 comment #16)
> >  - &userName's Firefox Sync Key
> 
> I'm not sure that putting the Sync account name on the artifact is such a good
> idea, especially if it ends up being printed. It would mean two out of three
> credentials are on that piece of paper. For now I think we should stick with
> "Your Sync Key".

Was aiming for a bit of personalization here, as well as the potential that two people in the same household might end up with the key. I'm not as worried as you about having the username on the key from a security perspective, but also not really at the point where I think it's worth the argument. :)

> I don't think we should. With our entirely random Sync Key it's very easy to
> predict the entropy. With user generated stuff it's much less so. Let's keep
> the wording as general as possible.

Agreed.

Comment 3

[Ragavan S [:rags]]

(In reply to comment #2)
> 
> Was aiming for a bit of personalization here, as well as the potential that two
> people in the same household might end up with the key. I'm not as worried as
> you about having the username on the key from a security perspective, but also
> not really at the point where I think it's worth the argument. :)

I like the personalization as well. In the new design, there is no userName. There's a friendly name that the user can set for their account, but is not used as a credential. Why can't we use that?

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

That works for me, yes. I went to my Sync options and didn't notice a friendly name, but if we have that option, let's indeed do use it.

Comment 5

[Philipp von Weitershausen [:philikon]]

(In reply to comment #3)
> (In reply to comment #2)
> > 
> > Was aiming for a bit of personalization here, as well as the potential that two
> > people in the same household might end up with the key. I'm not as worried as
> > you about having the username on the key from a security perspective, but also
> > not really at the point where I think it's worth the argument. :)
> 
> I like the personalization as well. In the new design, there is no userName.
> There's a friendly name that the user can set for their account, but is not
> used as a credential. Why can't we use that?

Because it doesn't exist. This "shortname" as proposed by faaborg doesn't exist right now. Introducing it would mean changes to client library and -- depending on where this shortname would live -- either changes to the server code or the storage version. All possible of course, but ambitious given the time scale we had for revamping the setup wizard.

Comment 6

[Philipp von Weitershausen [:philikon]]

Attachment: sample

Here's a sample of what Sync is generating, based on beltzner's strings. Beware: No styling whatsoever.

Comment 7

[Philipp von Weitershausen [:philikon]]

Attachment: sample email

Here's a sample email based on beltzner's strings.

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

As long as we can change the styling of that email, and the strings are split up as I indicated, that looks fine to me.

Comment 9

[Philipp von Weitershausen [:philikon]]

Attachment: artifact v1

Comment 10

[Philipp von Weitershausen [:philikon]]

Attachment: email v1

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

Ah, crap, one edit:

"Find out more about the Firefox Sync..." -- remove "the"

Comment 12

[Philipp von Weitershausen [:philikon]]

(In reply to comment #8)
> As long as we can change the styling of that email,

The email is plaintext, so I'm not sure what you mean by styling. More newlines? :p

> and the strings are split up as I indicated, that looks fine to me.

They're not in the v1 patch, but I can do that. I guess it would give us more flexiibility in placing newlines...?

Comment 13

[Philipp von Weitershausen [:philikon]]

(In reply to comment #11)
> Ah, crap, one edit:
> 
> "Find out more about the Firefox Sync..." -- remove "the"

Caught that one already ;). Fixed in the patches.

Comment 14

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #12)
> The email is plaintext, so I'm not sure what you mean by styling. More
> newlines? :p

If that's all I get, yes. I'm going to be agitating pretty strongly for HTML based email, like we're living in 1999 or something.

> They're not in the v1 patch, but I can do that. I guess it would give us more
> flexiibility in placing newlines...?

Sure. Or HTML tags. Have you heard of HTML? It's gonna be *huge*.

Comment 15

[Philipp von Weitershausen [:philikon]]

Attachment: html email fail

Comment 16

[Philipp von Weitershausen [:philikon]]

(In reply to comment #14)
> (In reply to comment #12)
> > The email is plaintext, so I'm not sure what you mean by styling. More
> > newlines? :p
> 
> If that's all I get, yes. I'm going to be agitating pretty strongly for HTML
> based email, like we're living in 1999 or something.

Yeah, I'm afraid that's problematic. Even if your email program is set to compose in HTML by default, passing markup on correctly through the mailto handler doesn't seem to be possible (see attachment 475669 [details], tested this with GMail as well). Maybe there's another way that I don't know of?

> > They're not in the v1 patch, but I can do that. I guess it would give us more
> > flexiibility in placing newlines...?
> 
> Sure. Or HTML tags. Have you heard of HTML? It's gonna be *huge*.

Yo dawg, I herd you liek HTML so I put some tags in ur email!!11!

Comment 17

[Philipp von Weitershausen [:philikon]]

Attachment: email v2

Address beltzner's comments: split strings as originally indicated

Comment 18

[Philipp von Weitershausen [:philikon]]

This is basically ready to land except for the crash I'm getting on OSX (haven't tested any other platforms yet) when I try to print or save the artifact. The method works fine in the add-on on Firefox 3.6.9 and so long as the artifact doesn't have any <a> tags in it.

I could swear I had it working earlier and it broke after updating my checkout, but I couldn't find a working revision for now. TabCandy apparently ran into the same problem, or at least the same assertion, already: bug 575675. Makes sense since they also use iframe for their stuff.

Comment 19

[Philipp von Weitershausen [:philikon]]

I pushed this to try last night: http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-4c1e45b333e4/

The opt OSX build doesn't crash, the debug build does much like the one I built locally. Couldn't test any Windows builds because they either weren't built (try failure) or didn't work.

Comment 20

[Philipp von Weitershausen [:philikon]]

Unless we're willing to ship the artifact without <a href=""> links (which would be okay I guess for beta7 and the string freeze, not the final), we're now blocking on bug 575675.

P.S.: New round of try builds, now with Windows too:
http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-1460b6d25479/

Comment 21

[Philipp von Weitershausen [:philikon]]

Attachment: workaround

Workaround for bug 575675: Avoid <a href=""> in the HTML page, instead hack in the links programmatically on save.

It hurts me having to do it like this, but if we need to land the strings before bug 575675 gets fixed, this is a way.

Comment 22

[Philipp von Weitershausen [:philikon]]

Comment on attachment 476063 [details] [diff] [review]
workaround

Turns out there's a much simpler workaround: As long as we have an a:visited rule in the page, the assertion and the crash can be avoided.

Comment 23

[Philipp von Weitershausen [:philikon]]

Attachment: artifact v2

Avoid assertion and crash by having an explicit a:visited rule in the page.

This should be good to land now!

Comment 24

[Philipp von Weitershausen [:philikon]]

Attachment: artifact v2.1

Small change on v2: Make the references to bug 575675 a preprocessor comment so it doesn't show up in the saved document.

Comment 25

[Philipp von Weitershausen [:philikon]]

http://hg.mozilla.org/mozilla-central/rev/a03d65efc824
http://hg.mozilla.org/mozilla-central/rev/773a098c9b05