Closed Bug 1361952 Opened 7 years ago Closed 7 years ago

Update Firefox Account to match the spec

Categories

(Firefox :: Settings UI, defect, P1)

Product:
Firefox
Component:
Settings UI
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 57
Tracking Flags:
Tracking Status
firefox57 --- verified

People

(Reporter: chsiang, Assigned: rickychien)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Whiteboard: [photon-preference])

Attachments

(2 files, 1 obsolete file)

Bug 1361952 - Update Firefox Account to match the spec
59 bytes, text/x-review-board-request
jaws
: review+
Details
screenshot
62.50 KB, image/png
Details 
      No description provided.
Hi Sean,
Could you provide the final illustration to replace the current about:preferences#sync?
Engineering team can start working on this once you provide the illustration. 

Thanks.
Cindy
Flags: needinfo?(smartell)
Flags: qe-verify+
QA Contact: hani.yacoub
Attached image Screen Shot 2017-05-05 at 1.06.14 PM.png (obsolete) — 
This is the illustration we have today. We should update it to the latest.
This is the spec[1] for this bug. We need image resources to continue to work on it.

[1]: https://mozilla.invisionapp.com/share/X8BGCX9PD#/screens/239960709
Flags: needinfo?(smartell)
Summary: Update Firefox sync illustration to be consistent with the onboarding tour → Update preferences category to match the spec
Blocks: 1357306
No longer blocks: 1357308
Summary: Update preferences category to match the spec → Update Firefox sync illustration
Summary: Update Firefox sync illustration → Update Firefox Account to match the spec
Assignee: nobody → rchien
Status: NEW → ASSIGNED
Priority: P2 → P1
Attachment #8864740 - Attachment is obsolete: true
This patch relies on bug 1361952 & bug 1377174, please apply the both of patches first before testing. Thanks
Comment on attachment 8894732 [details]
Bug 1361952 - Update Firefox Account to match the spec

https://reviewboard.mozilla.org/r/165900/#review173482

r=me with the following changes

::: browser/themes/shared/incontentprefs/preferences.inc.css:660
(Diff revision 4)
>    list-style-image: url(chrome://browser/skin/warning.svg);
>    filter: drop-shadow(0 1px 0 hsla(206, 50%, 10%, .15));
>    margin: 4px 8px 0px 0px;
>  }
>  
> -#syncOptions {
> +#fxaSyncEngines checkbox {

This should use the child selector

::: browser/themes/shared/incontentprefs/preferences.inc.css:678
(Diff revision 4)
>    list-style-image: url("chrome://browser/skin/preferences/in-content-new/logo-ios.svg");
>  }
>  
>  .androidLink,
>  .iOSLink {
> +  margin: 0 4px 0 2px;

This is not correct for RTL.
Attachment #8894732 - Flags: review?(jaws) → review+
As comment 6 mentioned, this patch will be landed until bug 1377174 resolved fixed. thanks
Pushed by rchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0ae3b8deb294
Update Firefox Account to match the spec r=jaws
Thanks. Patch has re-landed.
Flags: needinfo?(rchien)
Pushed by rchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/16c102a1cc39
Update Firefox Account to match the spec r=jaws
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/78041948c80f
Backed out changeset 16c102a1cc39 for conflicting with m-c merge
Pushed by rchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c570f065e0ad
Update Firefox Account to match the spec r=jaws
Backed out because it removed the preprocessing rule from privacy.css and so failed mochitest browser_parsable_css.js:

https://hg.mozilla.org/integration/autoland/rev/c05948f9a175402c44afedee60a0869497f5a3c5

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=c570f065e0ade1bae026d5cf059305350496acfd&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=runnable&filter-resultStatus=usercancel
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=123603577&repo=autoland

[task 2017-08-16T14:44:30.267155Z] 14:44:30     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/static/browser_parsable_css.js | Got error message for chrome://browser/skin/preferences/in-content-new/privacy.css: Expected declaration but found ‘%’.  Skipped to next declaration. - 
[task 2017-08-16T14:44:30.269590Z] 14:44:30     INFO - Stack trace:
[task 2017-08-16T14:44:30.271976Z] 14:44:30     INFO - chrome://mochitests/content/browser/browser/base/content/test/static/browser_parsable_css.js:messageIsCSSError:187
[task 2017-08-16T14:44:30.274748Z] 14:44:30     INFO - chrome://mochitests/content/browser/browser/base/content/test/static/browser_parsable_css.js:checkAllTheCSS:351
etc.
Flags: needinfo?(rchien)
Pushed by rchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bf581c606046
Update Firefox Account to match the spec r=jaws
I'm curious why this changes bring about the privacy.css failure. My current patch just move the file order in jar.inc.mn at https://hg.mozilla.org/integration/autoland/rev/c570f065e0ad#l7.12. It is supposed to not trigger privacy.css parsing error like that.

privacy.css was introduced by bug 1381401 and also modify in bug 1371230. I'm going to re-reland this patch again and see what happen.
Ah, I forgot the `*` in front of skin/classic/browser/preferences/in-content-new/privacy.css so that it brings about the CSS parsing error.
Thank you philor. Issue has been addressed in comment 28, I'm going to re-land it!
Pushed by rchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9fb2a131d67f
Update Firefox Account to match the spec r=jaws
https://hg.mozilla.org/mozilla-central/rev/9fb2a131d67f
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
There is a regression with that patch: I am not able to see my avatar in about:preferences.
Hi Edouard Oger, I've filed a bug for this. Please see bug 1391517. Thanks
Attached image screenshot — 
There is no space between name and email address. Is this a regression from this patch?

(in my opinion it would look better with the email address in a new line)
According to the spec, the font size of the email address after adding a user name should be smaller than before adding a user name, but in this case the font size is the same.
Is this behavior intended?

Thanks.
Flags: needinfo?(rchien)
IIRC, it's intended and it has decided by visual designer from an offline conversation. All of those visual verification will be forward to visual designer once all our visual works are done (bug 1392532 & bug 1394652).
Flags: needinfo?(rchien)
Build ID: 20170903220032
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0

Verified as fixed on Firefox Nightly 57.0a1 on Windows 10 x 64 and on Mac OS X 10.12.

I'll verify this bug on Ubuntu 16.04 only after bug 1392532 will be fixed.
Build ID: 201709055220108
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0

Verified as fixed on Firefox Nightly 57.0a1 on Ubuntu 16.04 x64.
Status: RESOLVED → VERIFIED
status-firefox57: fixed → verified
Depends on: 1409710
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: