1628528 - Styling of "insecure password" warning doesn't look right

Status: VERIFIED FIXED

(Firefox :: Security, defect, P2)

Description

[Ed Lee :Mardak]

Attachment: screenshot from 76

At least compared to bug 1319919 attachment 8818946 [details], it doesn't look quite right, although the current implementation also has a "Learn More" (and all the text gets darker (black?) when the entry is focused). Also, the whitespace around the icon doesn't look quite right either.

Comment 1

[Ed Lee :Mardak]

I'm guessing this was regressed by bug 1511878 with a similar looking regression bug 1620048 attachment 9131104 [details].

The theme does use GrayText for insecure as did the downloads text:
https://searchfox.org/mozilla-central/rev/4e228dc5f594340d35da7453829ad9f3c3cb8b58/browser/themes/shared/autocomplete.inc.css#136

Comment 2

[Johann Hofmann [:johannh]]

Matt, is your team looking into this? :)

Comment 3

[:severin Rudie]

Yep, picking it up now.

Comment 4

[:severin Rudie]

Is there a source of truth mock for this? I found this image on support.mozilla.org, which looks different from the image in bug 1319919.

Comment 5

[Matthew N. [:MattN]]

That screenshot looks like it's when the row is selected. We want to go back to how it was before bug 1511878 probably regressed it but for that you'd have to test on 10.13 or earlier I guess…. You can use ./mach mozregression --launch 2019-01-14 the build from the day before and see how it looked but I guess you don't have a machine with that version. It seems like https://phabricator.services.mozilla.com/D66080 used --in-content-deemphasized-text (which we can't use because then it would change in dark mode but the rest of the autocomplete popup doesn't change) but that maps to var(--grey-60) in non-dark mode.

Btw. you can test with the password fields on http://bugs.mattn.ca/pwmgr/login_and_change_form.html

Also, if you want to inspect inside the autocomplete popup you should use the disable auto-hide option in the Browser Toolbox: https://send.firefox.com/download/aeac9798c509e81f/#K-jjHH_96HL4mP37tvwHlQ

Comment 6

[:severin Rudie]

Attachment: Bug 1628528 - Fix text color of the 'insecure connection' login field warning. r=MattN

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cf7b701ef2ea
Fix text color of the 'insecure connection' login field warning. r=MattN

Comment 8

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/cf7b701ef2ea

Comment 9

[Timea Cernea [:tbabos][inactive]]

Attachment: Fix with Firefox main Themes

Hey Severin,

I checked the insecure warnings on all 3 Firefox main themes, the results can be seen in the attachment.
Default and Dark theme have the same style while the Light theme is different (note: there are no hover effects applied to any of them, in fact none of them have highlight on hover or click available)
For the scenario where we have only the insecure warning and "Viewed Saved Logins" dropdown, it looks ok only for the Default and Dark theme.
For the scenario where we have saved logins listed, generate password, footer and insecure warning, IMHO the Light theme style is better to divide the insecure warning from the rest of the content given in looks quite messy as it is.

Could this be improved/adjusted or should it go like this? I can submit a separate bug for further adjustments if needed and close this one as verified, let me know how it would be better.

EDIT: the above are happening on Windows 10.
On MacOS 10.13:

• Default and Light looks the same
• Dark theme on MacOS = dark theme on Win10

So there are differences between OSes too... which makes it a bit more confusing :)

Comment 10

[:severin Rudie]

Hey Timea,

Good catch on this! I agree that this could be improved, but I think it's still in a much better spot than release, so I'm inclined to keep the changes anyway. I think opening a separate bug for this and tagging UX would be the best route, they can decide what kind of improvements they would like here and we can fix them all at once.

Comment 11

[Timea Cernea [:tbabos][inactive]]

I also agree with that, thank you. Will follow up with an enhancement for the themes and possibly line it up for all OSes.
Closing this as verified-fixed on Windows 10 x64, MacOS 10.13 and Ubuntu 16.04 on the latest Beta 77.0b2.