Closed Bug 1628528 Opened 5 years ago Closed 5 years ago

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

Categories

(Firefox :: Security, defect, P2)

Unspecified
macOS
defect

Tracking

()

VERIFIED FIXED
Firefox 77
Tracking Status
firefox-esr68 --- wontfix
firefox75 --- wontfix
firefox76 --- wontfix
firefox77 --- verified

People

(Reporter: Mardak, Assigned: severin)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(3 files)

Attached image screenshot from 76 (deleted) —

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.

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

OS: Unspecified → macOS
Regressed by: 1511878
Has Regression Range: --- → yes
Blocks: 1601442

Matt, is your team looking into this? :)

Flags: needinfo?(MattN+bmo)
Priority: -- → P2

Yep, picking it up now.

Assignee: nobody → severin.mozilla
Status: NEW → ASSIGNED
Flags: needinfo?(MattN+bmo)

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.

Flags: needinfo?(MattN+bmo)

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

Flags: needinfo?(MattN+bmo)
Attachment #9142219 - Attachment description: Bug 1628528 - fix 'insecure connection' login doorhanger. r=MattN' → Bug 1628528 - Fix text color of the 'insecure connection' login field warning. r=MattN
Pushed by mozilla@noorenberghe.ca: https://hg.mozilla.org/integration/autoland/rev/cf7b701ef2ea Fix text color of the 'insecure connection' login field warning. r=MattN
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 77
Flags: qe-verify+
Attached image Fix with Firefox main Themes (deleted) —

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 :)

Flags: needinfo?(severin.mozilla)

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.

Flags: needinfo?(severin.mozilla)

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.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: