1954490 - more difficult to distinguish the current tab

Status: RESOLVED FIXED

(Toolkit :: Themes, defect, P1)

Description

[Sören Hentzschel]

Attachment: screenshot

Recently, a change to the shadow made it more difficult to recognize the active tab. Please note that there was already bug 1704347 before this change where people complained about the low contrast of the active tab, and now the active tab has even less contrast due to the reduced shadow.

Tested on macOS 15.3.1 with the default Firefox theme.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

:julianwels, since you are the author of the regressor, bug 1889754, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 2

[Dianna Smith [:diannaS]]

:dao do you think you can set a priority/severity? thank you!

Comment 3

[Dão Gottwald [:dao]]

[Tracking Requested - why for this release]:

Julian and Jules, how do we feel about this? Should we increase this shadow level's strength, or use a different level for tabs?

Setting S2 for now for possible accessibility impact as well as high visiblity in primary UI.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

The bug is marked as tracked for firefox138 (nightly). We have limited time to fix this, the soft freeze is in 3 days. However, the bug still isn't assigned.

:cbellini, could you please find an assignee for this tracked bug? Given that it is a regression and we know the cause, we could also simply backout the regressor. If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Comment 5

[Jules Simplicio [:jules]]

I believe Julian is going to address this, but they're out sick. If they're not back tomorrow I am happy to pick this up.

Comment 6

[Dianna Smith [:diannaS]]

As a reminder soft code freeze starts today!

Comment 7

[Julian Gaibler]

Hi! I'm looking into this right now.

The tab strip didn't have a passing contrast ratio before, though I think it's fair to say it got slightly worse due to the new shadow. Addressing the contrast issue properly is a larger project, so I'm trying to figure out how to adjust some values to bring it back to roughly where it was before.

Thanks for the reminder about the code freeze! I'm not sure if I can figure something out before merge day. We might need to uplift then.

Comment 8

[Julian Gaibler]

Attachment: Bug 1954490 - Improve contrast between active tab and tabstrip.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/030075958225
Improve contrast between active tab and tabstrip. r=desktop-theme-reviewers,reusable-components-reviewers,emilio,mstriemer

Comment 10

[agoloman]

Backed out for causing bc failures @browser_startup_flicker.js and @browser_windowclose.js.

• Backout link
• Push with failures
• Failure Log @browser_startup_flicker.js
• Failure Log @browser_windowclose.js.

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1889754

Comment 12

[Julian Gaibler]

looking into it

Comment 13

[Dianna Smith [:diannaS]]

:julian just making sure this is still on your radar for 138?

Comment 14

[Julian Gaibler]

Attachment: diff.png

Yeah, it is

Just as an updated where I am:

I'm trying to figure out why the test is supposed to fail. Looks like there is a 1px wide area around the tab that changes RGB values by 1. That is consistent with the other intermittent failures (bug 1872332, bug 1889278). Same area, same issue. So I don't think this is a regression and just the intermittently failing test.

Started another try run a little while ago to see if it fails consistently. If it doesn't, then I guess I was just unlucky; if it is I'd guess that changing the tabstrip color may have exacerbated the issue but not caused it.

Emilio, do you have any ideas here? ^^;

Comment 15

[Emilio Cobos Álvarez (:emilio)]

So the wrong area is in the tab shadow? I think we have an existing workaround for exactly this issue, see this. So probably you need to apply that workaround to those tests. But check with someone more familiar with those than me :)

Comment 16

[Dianna Smith [:diannaS]]

As a reminder we are tracking this for 138 and the last beta builds at the end of this week.

Comment 17

[Julian Gaibler]

After about 20 try runs, I finally have one that did not fail 🫠

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c87ec8a36c2a
Improve contrast between active tab and tabstrip. r=desktop-theme-reviewers,reusable-components-reviewers,emilio,mstriemer

Comment 19

[Julian Gaibler]

If this causes test failures please ping me before backing out! There are a couple of flaky tests I'd rather turn off than fight them right now (in the hopes we can still uplift this)

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9e8157a63eb4
Fix permafailure in browser_windowopen.js. CLOSED TREE

Comment 21

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/c87ec8a36c2a
https://hg.mozilla.org/mozilla-central/rev/9e8157a63eb4

Comment 22

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:julianwels, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox138 to wontfix.

For more information, please visit BugBot documentation.

Comment 23

[Julian Gaibler]

Attachment: beta-rebase-1954490.patch

Approval Request Comment
[Feature/Bug causing the regression]: 1889754
[User impact if declined]: Active tab harder to distinguish from other tabs in default theme.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: (This patch is rebased to beta and contains both patches)
[Is the change risky?]: No
[Why is the change risky/not risky?]: Only visual change (and test behavior)
[String changes made/needed]: No

Comment 24

[Dianna Smith [:diannaS]]

Comment on attachment 9479894 [details] [diff] [review]
beta-rebase-1954490.patch

Approved for 138.0rc1

Comment 25

[Pulsebot]

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