1196229 - Make link to manage the Firefox Account in Sync settings a link

Status: VERIFIED FIXED

(Firefox :: Sync, defect, P1)

Description

[Ryan Feeley [:rfeeley]]

Attachment: fxa-signed-in.gif

I believe originally, the link to Manage the Firefox Account was a link, but was changed to a button in the dark of night.

According to Stephen Horlander, we should reserve buttons for native UI and use links for website links as much as possible.

Current (frame 1) and proposed (frame 2) are attached in an animated GIF.

current: Manage
proposed: Manage Account

note order change, and text alignment.

Comment 1

[Edouard Oger [:eoger]]

Attachment: bug-1196229.patch

Comment 2

[Mark Hammond (PTO; back Oct-3) [:markh] [:mhammond]]

Comment on attachment 8651250 [details] [diff] [review]
bug-1196229.patch

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

::: browser/components/preferences/in-content/sync.xul
@@ -251,5 @@
>                <vbox flex="1">
>                  <label id="fxaEmailAddress1"/>
>                  <label id="fxaDisplayName" hidden="true"/>
> -                <description class="fxaAccountBoxButtons">
> -                  <button id="verifiedManage">&manage.label;</button>

Can you please also update the string in the "old prefs" sync.xul, then we can remove the manage.label string from the .dtd (it doesn't really matter that the new string might make the old prefs look strange as they are technically dead)

::: browser/locales/en-US/chrome/browser/preferences/sync.dtd
@@ +71,5 @@
>  
>  <!ENTITY notSignedIn.label           "You are not signed in.">
>  <!ENTITY signIn.label                "Sign in">
>  <!ENTITY profilePicture.tooltip      "Change profile picture">
>  <!ENTITY manage.label                "Manage">

(ie, kill this string)

Comment 3

[Edouard Oger [:eoger]]

Attachment: bug-1196229.patch

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/7db9835949b6

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7db9835949b6

Comment 6

[Stefan Plewako [:stef]]

And now we have two identical manageAccount.label strings in the sync.dtd file (for two different XUL elements)?

Comment 7

[Edouard Oger [:eoger]]

My bad, I'm sorry. I'll post a new patch right away.

Comment 8

[Edouard Oger [:eoger]]

Attachment: bug-1196229-followup.patch

Comment 9

[Stefan Plewako [:stef]]

(In reply to Edouard Oger [:eoger] from comment #8)
> Created attachment 8652443 [details] [diff] [review]

Usually it is not good idea to reuse strings for different elements (it is better to use two strings with identical content but different ids) but I didn't check how exactly it is used originally.

https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices

Comment 10

[Edouard Oger [:eoger]]

Attachment: bug-1196229-followup.patch

True, here's a revised copy.

Comment 11

[Stefan Plewako [:stef]]

Comment on attachment 8652461 [details] [diff] [review]
bug-1196229-followup.patch

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

Looks good form l10n perspective, f+

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8652461 [details] [diff] [review]
bug-1196229-followup.patch

This doesn't mean what you think it does.

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/4d3254435829

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4d3254435829

Comment 15

[Paul Silaghi, QA [:pauly]]

Attachment: sync.png

Making the window smaller causes some issues - see attached.
What do you think?

Comment 16

[Edouard Oger [:eoger]]

I opened bug 1200749, thanks Paul!

Comment 17

[Paul Silaghi, QA [:pauly]]

Marking this bug as verified on 43.0a1 (2015-09-01). Work continues in bug 1200749.