1732090 - [Monochromatic Themes] The theme is not applied to the Sidebar selection colors

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Oana Botisan, Desktop QA]

Affected versions

• Firefox 94.0a1

Affected platforms

• macOS 10.15
• Windows 10 x64
• Ubuntu 20.04 x64

Preconditions

• browser.theme.temporary.monochromatic.enabled = true
• Select one the Monochromatic themes.

Steps to reproduce

1. Go to Application menu -> More tools -> Customize toolbar... and drag the "Show sidebars" button on the Toolbar.
2. Click on it and observe the Sidebar..

Expected result

• The Sidebar respects the theme colors.

Actual result

• The Sidebar doesn't respect the theme colors.

Regression range

• Not a regression.

Additional notes

• Please look at the specs: https://www.figma.com/file/xaRff6432QsirRftX8NZgb/MR2-Themes?node-id=86%3A17747

Comment 1

[Pascal Chevrel:pascalc]

93 is not impacted

Comment 2

[Harry Twyford]

Attachment: Screen Shot 2021-09-22 at 1.22.40 PM.png

Which theme was applied? Sidebars are themed on my machine. Here's Abstract Bold, for example.

Comment 3

[Oana Botisan, Desktop QA]

I used themes like Lush Soft/Balanced and Abstract Balanced/Soft. And considering that nothing else changed about the Sidebar, other then the background, I didn't noticed that it was themed.
Maybe the hover/click effect on the close button "x" should be themed at least, because with the majority of the themes the Sidebar seems unchanged.

Comment 4

[Tracy Walker [:tracy]]

As seen in macOS with build ID 20210923095548
Making an assumption that the sidebar styling should look like active tab style (text and background colors).

The following sidebar themes styling look correct:
Abstract Bold, Abstract Soft
Cheers Soft, Cheers Balanced, Cheers Bold
Elemental Soft, Elemental Bold
Foto Soft, Foto Bold
Graffiti Soft, Graffiti Bold
Lush Soft, Lush Bold

Style of sidebar in these themes does not look correct, in each case, there is a slight tint to background in active tab. the sidebar, is instead, pure white:
Abstract Balanced
Elemental Balanced
Foto Balanced
Graffiti Balanced
Lush Balanced

Comment 5

[Cristian Baica [:cbaica], Release Desktop QA]

There's also something else that classifies the sidebar as not being properly themed and that is the sidebar selection/highlight which is done by navigating inside it with the keyboard. This is more noticeable on the more colorful themes e.g. Foto (Balanced and Soft), Grafitti (Balanced and Soft), but also occurs on Bold themes, but it's less noticeable.
In all themes, across all OS's the selection is grey, not themed (see latest attachment).

Comment 6

[Cristian Baica [:cbaica], Release Desktop QA]

Attachment: sidebar item selection is grey, not themed

Comment 7

[Harry Twyford]

(In reply to Tracy Walker [:tracy] from comment #4)

As seen in macOS with build ID 20210923095548
Making an assumption that the sidebar styling should look like active tab style (text and background colors).

Style of sidebar in these themes does not look correct, in each case, there is a slight tint to background in active tab. the sidebar, is instead, pure white:
Abstract Balanced
Elemental Balanced
Foto Balanced
Graffiti Balanced
Lush Balanced

This is per-spec. The sidebar should be white in those themes. See https://www.figma.com/file/xaRff6432QsirRftX8NZgb/MR2-Themes?node-id=234%3A71993.

I'll change this bug to be about comments 5 and 6, which is an issue.

Comment 8

[Harry Twyford]

Oh, we already have bug 1732291. I'll address it there.

Comment 9

[Harry Twyford]

Oana pointed out to me the STR are a little different in this bug, so I'm reopening it.

Comment 10

[Harry Twyford]

Attachment: Bug 1732090 - Use sidebar_text as a fallback in highlighted sidebar rows. r?dao!

We only set the lwt-sidebar-highlight attribute when a theme defines the sidebar_highlight-text property. We check lwt-sidebar-highlight when setting the sidebar highlight background color. That means we don't style sidebar highlight colors unless we also style the text. We should use sidebar_text as a fallback when sidebar_highlight_text isn't defined. We do something similar in toolbar fields: we use toolbar_field_text in highlighted rows when toolbar_field_highlight_text isn't defined.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c7d53c8b6d8f
Use sidebar_text as a fallback in highlighted sidebar rows. r=desktop-theme-reviewers,dao

Comment 12

[Iulian Moraru]

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

Comment 13

[Oana Botisan, Desktop QA]

We verified the fix using Firefox 94.0b2 on Windows 10 x64, macOS 10.15 and Ubuntu 18.04 x64. The issue is not reproducing anymore.