Closed Bug 1703635 Opened 6 months ago Closed 5 months ago

Protection panel is missing a number of Proton styles

Categories

(Firefox :: Protections UI, defect, P1)

Firefox 89
defect

Tracking

()

VERIFIED FIXED
89 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox87 --- unaffected
firefox88 --- unaffected
firefox89 --- verified

People

(Reporter: ciprian_georgiu, Assigned: mhowell)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [proton-door-hangers])

Attachments

(5 files)

Affected versions

  • latest Nightly 89.a01

Affected platforms

  • Win 10 x64
  • macOS 10.15
  • Ubuntu 18.04 x64

Steps to reproduce

  1. Launch Firefox.
  2. Go to reddit.com and click on the shield icon from the URL bar.
  3. Inspect the protection panel.

Expected result

  • The radio button width is bit bigger.
  • The "Site not working?" link is underlined.
  • "S" from "Protection Settings" is written with lower case. Same goes for "Protection Dashboard", "Tracking Content" etc. - please observe the figma specs.
  • The borders that delimitates each section are not stretched to the entire panel's width. Per specs, it should be a couple of pixels left at each sides. For example, the first border below the Protection for <website> is correctly displayed.

Actual result

  • Radio icon seems a bit smaller in width compared to the specs.
  • The "Site not working?" link is not underlined.
  • "S" from "Protection Settings" is not written with lower case. Same goes for "Protection Dashboard", "Tracking Content" etc. - please observe the figma specs.
  • The borders that delimitates each section are stretched to the entire panel's width. Expect is the first border from the header, which is correctly displayed.

Suggested Severity

  • S2, since the permission panel is not completely styled for proton.

Regression range

  • Not a regression.

Additional notes

  • please observe the attached sceenshots.
Attached image etp from specs.png

Attached a screenshot from figma specs.

Has Regression Range: --- → no
Has STR: --- → yes
Whiteboard: [proton-door-hangers]
Priority: -- → P2
Whiteboard: [proton-door-hangers] → [proton-door-hangers] [priority:2a]
Priority: P2 → P1
Whiteboard: [proton-door-hangers] [priority:2a] → [proton-door-hangers]

Bumping this to P1 per discussion this morning in the MR1 triage call

Blocks: 1700957
See Also: → 1703652
Assignee: nobody → mhowell
Status: NEW → ASSIGNED

@flod, one of the things this bug is asking for is to sentence-case the strings in the protection panel, but what's the status of the overall sentence casing effort? I don't want to land this and have it be the only thing with sentence casing for any length of time.

Flags: needinfo?(francesco.lodolo)

Redirecting to mconley, as far as I can tell is a low priority, and we probably don't want mixed cases at this point.

Flags: needinfo?(francesco.lodolo) → needinfo?(mconley)

We've already started to mix cases. The bookmarks panel header and buttons, for example, was intentionally converted to sentence case in bug 1703003.

I agree that sentence casing is a lower priority than some of our other bugs, and that if and when we wanted to do sentence casing, we'd ideally want to do it all at once to avoid inconsistencies.

So let's leave this for now, and try to tackle it when we do sentence casing more broadly.

Flags: needinfo?(mconley)

This patch makes a few changes to bring the protections panel closer in line
with the Proton spec:

  1. Add a few pixels of width to the tracking protection toggle switch.
  2. Add underlining to the "site not working" link.
  3. Replace the borders between sections of the panel with toolbarseparator
    elements, so that they become narrower than the panel width (and also
    match the panel header's separator).
Flags: needinfo?(prathikshaprasadsuman)
Flags: needinfo?(prathikshaprasadsuman)
Duplicate of this bug: 1703416
See Also: → 1703579, 1703586
Flags: needinfo?(nhnt11)
Pushed by mhowell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dc437102622b
Fix several protections panel styling issues. r=mconley
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 89 Branch
Attached image Screenshot_12.png

We found some differences between the specs and the fix in the Dowload details panel:

  • The border between the title and the information is still too long when the information panel is extended.
  • The border between the sections on the detail subpanel is still too long.
  • Every word from the sections is written with an uppercase.

Are these differences expected?

Flags: needinfo?(mhowell)
Attached image Screenshot_13.png

This is the image from the subpanel.

(In reply to Oana Botisan, Desktop Release QA from comment #12)

Thanks for taking a look at this.

  • The border between the title and the information is still too long when the information panel is extended.

This looks like it's affecting every panel header, not just this panel. That should really be a separate bug, if it isn't filed already.

  • The border between the sections on the detail subpanel is still too long.

Ah, it seems like I just overlooked those, I should have included those in my patch. It appears that all of the tracking subpanels would be affected. I'll file a followup bug for these and link it here.

  • Every word from the sections is written with an uppercase.

This one is expected; in the comments above we discussed changing that here and decided to wait until more of those changes are rolled out.

Flags: needinfo?(mhowell)
See Also: → 1705540
Flags: qe-verify+

Verified the fix using Firefox 89.0b2 on Windows 10 x64, Ubuntu 16.04 and macOS 10.15. The bug is not reproducing with the exceptions of the three issues that were logged separately.

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