1696500 - Proton panels should have the correct border colour in default/Light mode

Status: VERIFIED FIXED

(Firefox :: Theme, task, P1)

Description

[Mike Conley (:mconley) (:⚙️)]

Our panels have a --arrowpanel-border-color CSS variable that they use to set the border colour of the panels.

We should update the rules so that the Proton panels have the --arrowpanel-border-color match the value of --arrowpanel-background when in default / Light mode.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

According to mstange, having an invisible border is not possible on macOS. This is because the operating system is drawing the "shadow rim" around the panel, and we don't have any influence over it.

I seem to be able to do this for Windows though. Unsure about Linux.

Comment 2

[Markus Stange [:mstange]]

Let me clarify though: The macOS shadow rim is drawn outside any border we draw, in addition to it. So we should still remove any border from our side. Then the only remaining "border" is the shadow rim, which we don't have control over.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Hey Gijs, speaking to amylee, it sounds like we want to apply a hairline-color border on all panels on the platforms where we're applying borders to modals. Do you know which platforms those are?

Comment 4

[:Gijs (he/him)]

(In reply to Mike Conley (:mconley) (:⚙️) (Catching up on needinfos) from comment #3)

Hey Gijs, speaking to amylee, it sounds like we want to apply a hairline-color border on all panels on the platforms where we're applying borders to modals. Do you know which platforms those are?

I'm confused; the modals spec doesn't have any borders, nor does it have platform-dependent ones. It only has box shadows, and radiuses its content.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

From UX, it sounds like border styling on our panels isn't a blocker.

Comment 6

[Tyler Duzan]

Promoted to P1 per conversation in MR1 triage call this morning

Comment 7

[Bernard Igiri]

Attachment: Bug 1696500 - Proton panels should have the correct border colour in light mode. r=mconley

Set proton panel border colors to match spec for light/dark mode in the .css and manifest.json files.

Comment 8

[Dão Gottwald [::dao]]

It seems like this is already fixed:

https://searchfox.org/mozilla-central/rev/d280cc26237b62096b89317e4ed6dea8b2bdf822/browser/themes/shared/browser-custom-colors.inc.css#54

https://searchfox.org/mozilla-central/rev/d280cc26237b62096b89317e4ed6dea8b2bdf822/browser/themes/addons/light/manifest.json#26

Is there anything else that needs to happen here?

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Commented in Phabricator.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fbb0306fdaa5
Proton panels should have the correct border colour in light mode. r=mconley,desktop-theme-reviewers,dao

Comment 12

[Atila Butkovits]

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

Comment 13

[Marco Bonardo [:mak]]

Just a gentle reminder to request uplift, this is blocking a P2a (bug 1706154)

Comment 14

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

Comment on attachment 9217149 [details]
Bug 1696500 - Proton panels should have the correct border colour in light mode. r=mconley

Beta/Release Uplift Approval Request

• User impact if declined: Required for Proton/MR1
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: The light / system mode border for panels should be rgb(240,240,244). The change in this patch does not affect dark / dark-system mode.
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): CSS/web-extension theme only change
• String changes made/needed: none

Comment 15

[Pascal Chevrel:pascalc]

Comment on attachment 9217149 [details]
Bug 1696500 - Proton panels should have the correct border colour in light mode. r=mconley

Approved for 89 beta 6, thanks.

Comment 16

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-beta/rev/a8b821146e3a

Comment 18

[Pablo]

Hi jared

I checked this on windows10 64bit, Macos 10.14 and Ubuntu 20 64bit. Using firefox beta 89.0b8 and nightly 90.0a1
I had different results. Perhaps its because of the difference in each operating sytem? There was a big difference in ubuntu 20 tho.

What i did was to go to http://permission.site/ and then hit some of those like "microphone" and then check the border right next to the white area.

Here is what i got:

(Deepin picker - ubuntu 20)
C4C4C4 on system theme
F0F0F4 on light theme

( Windows 10 - using Rulerapp )
FFF0F0F4 on system theme
FFF0F0F4 on light theme

(MacOs 10.14 using xScope)
ececf1 on system theme
ececf1 on light theme

please let me know if i need to open another bug or if its fine.

thanks,

Comment 19

[Ciprian Georgiu [:ciprian_georgiu], Release Desktop QA]

Hi Jared, did you have time to look over the above comment?

Comment 20

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

I did not have time. I'm on PTO this week. Harry, could you take a look?

Comment 21

[Harry Twyford]

Attachment: macOS-border-after-change.png

(In reply to Pablo from comment #18)

There was a big difference in ubuntu 20 tho.

It's expected that the Ubuntu system theme will use different colors than Firefox Light. You got #f0f0f4 when using the Firefox Light theme, so this is working as expected.

(MacOs 10.14 using xScope)
ececf1 on system theme
ececf1 on light theme

Are you sure? I just measured the border color with Xscope on macOS and got #f0f0f4, as expected. If you're getting something else, could you please post a screenshot?

Comment 22

[Pablo]

Attachment: mac1014.png

Comment 23

[Pablo]

I checked again on a mac and the code started to show up correctly, but it was because its a Mac 10.15
when i switched to Mac 10.14.6 then i started to see the ececf1 code again, (attached screenshot)

Comment 24

[Pablo]

Fix was ferified in Beta and nightly since comment#18 using Ubuntu 20, Windows 10 and Mac 10.15
new bug was opened to investigate why on mac 10.14 it gives a different color:

https://bugzilla.mozilla.org/show_bug.cgi?id=1712957