Closed Bug 1319919 Opened 3 years ago Closed 3 years ago

Refine Insecure Password Warning style

Categories

(Firefox :: Security, defect, P1)

50 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 53
Tracking Status
firefox50 --- unaffected
firefox51 --- unaffected
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: rfeeley, Assigned: MattN)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 4 obsolete files)

Attached image insecure-small.png (obsolete) —
Using the same icon size, and the smaller text size/style from the search dropdown headings, we propose this solution which does not have a learn more (not needed in our opinion) and has a smaller font size.

Left/right margin of icon is 7.5 pixels (matches left margin of usernames below) and 11.5 pixel top/bottom margin.

Text has 7.5 pixel top, right and left margins.

I'm working on 2x resolution, so if 0.5 pixels don't work, round up to 8 and 12 pixels.
The bottom border is also missing from the current implementation.

Could you provided an update screenshot after bug 1129629?
Blocks: 1304224
No longer blocks: 1309044
Flags: needinfo?(rfeeley)
Specifying colours for better accessibility:

Background color: #eee
Text color: #666
Flags: needinfo?(rfeeley)
Does this block the 52 launch?
Yes, we can't ship the unpolished one with the "learn more" link in it.
The Learn More link was part of Aislinn and Phillip's design.  It is used to help confused users who want to 'learn more' and understand what is happening, why, and what they can and can't do about it.  Why don't we want it here?
This was explained in another bug in the tree. Web-style links do not fit this context (desktop software, inside a dropdown menu item). When users select the row, it will open the control center, which contains the SUMO link
Not sure why we're talking about Learn More here since that was already removed in bug 1318537.
(In reply to Ryan Feeley [:rfeeley] from comment #2)
> Specifying colours for better accessibility:
> 
> Background color: #eee
> Text color: #666

Shouldn't the colours match the footer of our other panels or will the Saved Logins footer look different than the warning header?
Flags: needinfo?(rfeeley)
Assignee: nobody → MattN+bmo
Status: NEW → ASSIGNED
Attached image insecure.png (obsolete) —
I tried a few variations. I think the only one that works is "Two" which is the same font size and colour as usernames, and with the same 1px #d0d0d0 divider. Make sense?
Flags: needinfo?(rfeeley) → needinfo?(MattN+bmo)
Attached image two.png (obsolete) —
So many moving parts on this, I realized the key can't be forgotten.
Attached image insecure-complete.png (obsolete) —
Based on team feedback, removed key, ellipsis, made title case
Attachment #8815697 - Attachment is obsolete: true
Priority: -- → P1
We can't land the "Open saved logins" label here unless we don't want to uplift this to Fx52. Should we broken that piece off to another bug?
I still think having a key icon next to a struck through lock icon is a mixed and confusing message for users.  Can we use a person icon instead?  For example, the one npr uses at the top of their site
(http://www.npr.org/#, click the search icon and see a little person show up next to the "Shop" text).

Lets add the additional iconography and the "Open Saved Logins" in a separate bug for Firefox 53.  We can use this bug to change the color, padding, size, etc.
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #12)
> We can't land the "Open saved logins" label here unless we don't want to
> uplift this to Fx52. Should we broken that piece off to another bug?

The footer is for bug 1189618 but Ryan is just showing the bigger picture together so the long term vision can be seen.
Flags: needinfo?(MattN+bmo)
My patch doesn't fix the right/left margins since (possibly broken) calculations in JS make that complicated :( This patch also doesn't affect the non-warning rows which Ryan wanted to adjust because that is kinda a separate issue and I wasn't sure if that should apply to form history too. I think we should land this incremental improvement which fixes the major issues and refine more later.
Comment on attachment 8818619 [details]
Bug 1319919 - Fix vertical spacing, font-size, and colors of the insecure field warning.

Look good to me.
Attachment #8818619 - Flags: review?(selee) → review+
Attached image open-saved-logins.png
I think this is best version so far, the one Matt and I reviewed at the all-hands.
Attachment #8813860 - Attachment is obsolete: true
Attachment #8815557 - Attachment is obsolete: true
Attachment #8815783 - Attachment is obsolete: true
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee48bfcb6ded
Fix vertical spacing, font-size, and colors of the insecure field warning. r=seanlee
Comment on attachment 8818619 [details]
Bug 1319919 - Fix vertical spacing, font-size, and colors of the insecure field warning.

Approval Request Comment
[Feature/Bug causing the regression]: Insecure field warning
[User impact if declined]: Incorrect spacing, colors, and selection in the login dropdown
[Is this code covered by automated tests?]: Yes, but not the styling
[Has the fix been verified in Nightly?]: Not yet
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: 
[Is the change risky?]: No
[Why is the change risky/not risky?]: simple CSS changes
[String changes made/needed]: None
Attachment #8818619 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/ee48bfcb6ded
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Comment on attachment 8818619 [details]
Bug 1319919 - Fix vertical spacing, font-size, and colors of the insecure field warning.

tweak styling for insecure password warning, take in aurora52
Attachment #8818619 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Hi Matthew,

I was testing this bug and I saw that there is an attachment "open-saved-logins.png" and I was wondering if that is the correct behaviour? In my case I don't see the "View saved logins" field. I assume this is the right behaviour, without that field, but I want to be sure. Thanks
Flags: needinfo?(MattN+bmo)
(In reply to ovidiu boca[:Ovidiu] from comment #24)
> Hi Matthew,
> 
> I was testing this bug and I saw that there is an attachment
> "open-saved-logins.png" and I was wondering if that is the correct
> behaviour? 

That screenshot is basically correct except for a few details like "Learn More".

> In my case I don't see the "View saved logins" field. I assume
> this is the right behaviour, without that field, but I want to be sure.
> Thanks

The footer will be implemented in bug 1319919 in future versions of Fx.
Flags: needinfo?(MattN+bmo)
Matt, should I mark this verified since there is bug 1333256 for the Learn more?
Flags: needinfo?(MattN+bmo)
Yep, though the footer impl. was moved to a different bug.
Status: RESOLVED → VERIFIED
Flags: needinfo?(MattN+bmo)
You need to log in before you can comment on or make changes to this bug.