1391517 - Fix missing Firefox Account profile avatar & shrink sync settings checkbox width

Status: VERIFIED FIXED

(Firefox :: Preferences, defect, P1)

Description

[Ricky Chien [:rickychien] (inactive)]

Missing account avatar after login is a regression from https://bugzilla.mozilla.org/show_bug.cgi?id=1361952#c33.

Comment 1

[Ricky Chien [:rickychien] (inactive)]

Attachment: Bug 1391517 - Fix missing fxa profile avatar & shrink checkbox width

Review commit: https://reviewboard.mozilla.org/r/169988/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/169988/

Comment 2

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8898599 [details]
Bug 1391517 - Fix missing fxa profile avatar & shrink checkbox width

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/169988/diff/1-2/

Comment 3

[Ricky Chien [:rickychien] (inactive)]

I also spotted an another issue that the checkbox width of "Sync Settings" in Firefox Account page doesn't follow the visual spec [1]. The checkbox width should be set to 224px (216px + 8px) according to spec.

Patch has been updated for fixing checkbox width as well. Thanks

[1] https://mozilla.invisionapp.com/share/X8BGCX9PD#/screens/244683212

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8898599 [details]
Bug 1391517 - Fix missing fxa profile avatar & shrink checkbox width

https://reviewboard.mozilla.org/r/169988/#review175164

::: browser/components/preferences/in-content-new/sync.js:334
(Diff revision 2)
>          } else {
>            fxaLoginStatus.removeAttribute("hasName");
>          }
>          if (data.avatar) {
>            let bgImage = "url(\"" + data.avatar + "\")";
> -          let profileImageElement = document.getElementById("fxaProfileImage");
> +          let profileImageElement = document.querySelector("#fxaLoginVerified > #fxaProfileImage");

We shouldn't have more than one element on a page that shares the same ID. This basically breaks fundamental assumptions of how IDs work.

Can you please change all #fxaProfileImage to be classes?

Same thing for #fxaLoginRejectedWarning too please.

Comment 5

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8898599 [details]
Bug 1391517 - Fix missing fxa profile avatar & shrink checkbox width

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/169988/diff/2-3/

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8898599 [details]
Bug 1391517 - Fix missing fxa profile avatar & shrink checkbox width

https://reviewboard.mozilla.org/r/169988/#review175170

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8c617506930e
Fix missing fxa profile avatar & shrink checkbox width r=jaws

Comment 8

[Wes Kocher (:KWierso)]

https://hg.mozilla.org/mozilla-central/rev/8c617506930e

Comment 9

[Hani Yacoub]

Build ID: 20170829100404
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, Mac OS X 10.12 and Ubuntu 16.04 x64.