Code analysis found 1 defect in diff 1258318:

• 1 defect found by stylelint (Mozlint)

You can run this analysis locally with:

If you see a problem in this automated review, please report it here.

You can view these defects in the Diff Detail section of Phabricator diff 1258318.

Looks like this has stylelint complaints. Looks like elsewhere we use which might work here too?

In D296488#10305116, @Gijs wrote:

Looks like this has stylelint complaints. Looks like elsewhere we use which might work here too?

Thanks for your suggestion, @Gijs! I agree that meets the 3:1 contrast requirement, but I am concerned that because the 0.75 we use right now is already closer to the enabled state, using 0.8 might further remove the "disabled" affordance for users.
would prefer to keep it at 0.75 and add a comment to silence the design-token lint. Please let me know what do you think :)

Thanks for the update and pushing back here. I guess this points to the question of why we're using custom styles for these buttons rather than something that relies entirely on existing design system bits. It's probably desirable from a UX/product pov, but unfortunate because threading the needle between reusing and custom values here ends up making this less maintainable: if the colours in the design system change, we may either fail WCAG again, or lose the disabled affordance anyway. That shouldn't stop us making the change but we'll probably need to be mindful of the nova updates.

Going back to https://bugzilla.mozilla.org/show_bug.cgi?id=2034940#c0, I feel like I still don't fully understand the intent here.

the button's white text has low contrast (2.06:1) with its background, due to CSS opacity:0.5. Although disabled buttons are exempt from minimum contrast, this button is not, in its current implementation, disabled.

What does "this button is not, in its current implementation, disabled" mean? In what sense is this button not disabled? Why does it match , and why are you using the term "disabled" in the variable name?

In D296488#10323432, @Gijs wrote:

Thanks for the update and pushing back here. I guess this points to the question of why we're using custom styles for these buttons rather than something that relies entirely on existing design system bits. It's probably desirable from a UX/product pov, but unfortunate because threading the needle between reusing and custom values here ends up making this less maintainable: if the colours in the design system change, we may either fail WCAG again, or lose the disabled affordance anyway. That shouldn't stop us making the change but we'll probably need to be mindful of the nova updates.

@Gijs Totally agree, the custom gradient is what forces this hand-tuned balance. Good news: Nova already overrides to 0.7, and Nova switches this button to a solid color with the 0.7 opacity, so once we migrate this button to Nova styles we can drop the custom 0.75 entirely and just use the standard token. I've also added a note in the comment pointing to var(--button-opacity-disabled) as the replacement once Nova lands here. Thank you for the guidance!

In D296488#10323489, @dao wrote:

Going back to https://bugzilla.mozilla.org/show_bug.cgi?id=2034940#c0, I feel like I still don't fully understand the intent here.

the button's white text has low contrast (2.06:1) with its background, due to CSS opacity:0.5. Although disabled buttons are exempt from minimum contrast, this button is not, in its current implementation, disabled.

What does "this button is not, in its current implementation, disabled" mean? In what sense is this button not disabled? Why does it match , and why are you using the term "disabled" in the

Thank you so much for reaching out, @dao! The comment saying was initially made when reporting an issue about the let's go button being focusable with a keyboard even when disabled. That's why it was noted as not being fully disabled. We've now fixed the keyboard focus issue, and the next step is to update the button's opacity to meet the minimum contrast requirements. The accessibility team still believes it's important to ensure the contrast is sufficient, despite the focus issue being addressed. Here's a Slack discussion that explains the reasoning behind this decision.

In D296488#10370603, @czhou wrote:

Thank you so much for reaching out, @dao! The comment saying was initially made when reporting an issue about the let's go button being focusable with a keyboard even when disabled. That's why it was noted as not being fully disabled. We've now fixed the keyboard focus issue, and the next step is to update the button's opacity to meet the minimum contrast requirements. The accessibility team still believes it's important to ensure the contrast is sufficient, despite the focus issue being addressed. Here's a Slack discussion that explains the reasoning behind this decision.

I'm still confused by that conversation because ensuring minimum contrast necessarily means the button will be less discernable as being disabled. I assume that's why disabled elements are exempt from contrast requirements in the first place. In some cases we have in fact reduced the opacity of disabled elements in prefers-contrast mode to ensure they don't look enabled. I don't understand why we'd want to do the opposite for this button even after reading that thread.

In D296488#10372536, @dao wrote:

In D296488#10370603, @czhou wrote:

Thank you so much for reaching out, @dao! The comment saying was initially made when reporting an issue about the let's go button being focusable with a keyboard even when disabled. That's why it was noted as not being fully disabled. We've now fixed the keyboard focus issue, and the next step is to update the button's opacity to meet the minimum contrast requirements. The accessibility team still believes it's important to ensure the contrast is sufficient, despite the focus issue being addressed. Here's a Slack discussion that explains the reasoning behind this decision.

I'm still confused by that conversation because ensuring minimum contrast necessarily means the button will be less discernable as being disabled. I assume that's why disabled elements are exempt from contrast requirements in the first place. In some cases we have in fact reduced the opacity of disabled elements in prefers-contrast mode to ensure they don't look enabled. I don't understand why we'd want to do the opposite for this button even after reading that thread.

Thanks for pushing back here @dao! re-reading the Slack thread, the reasoning is more nuanced than "WCAG 1.4.3 says so". A few key points from the a11y team:

1. This isn't about meeting a WCAG conformance bar: they explicitly acknowledge it's *not* a WCAG violation in the strict sense (the disabled exemption would apply). It's about affordance for sighted users with color blindness, low vision, or even just bright glare on their monitor.

2. The implicit question is: if a disabled button shouldn't need to be readable, why is it visible at all? Since it remains on screen, it's still communicating something to the user. And the instructional text that tells them "pick a model first" is spatially separated from the button — by the time the user reaches the button, that instruction may have dropped out of short-term memory. The label itself needs to stay readable enough to remind them.

3. Crucially, they're *not* aiming for 4.5:1 (full WCAG AA) — that would defeat the disabled affordance, which is your concern. They're aiming for 3:1 — readable enough to discern, dim enough to still read as disabled. This is the Firefox-agreed standard for disabled text (per discussions with @jules from the Design Systems team).

So the spec we're targeting here is the Firefox "3:1 for disabled text" convention, which is a deliberate middle ground between WCAG AA (too readable) and the WCAG exemption (essentially invisible). 0.75 opacity hits that 3:1 target for the current gradient.

If it would be helpful, I'd be glad to connect you with the a11y team to share more about this rationale!

I defer to @dao here but I'm not sure I understand the benefit of all the custom prefixes?

In D296488#10433240, @Gijs wrote:

I defer to @dao here but I'm not sure I understand the benefit of all the custom prefixes?

This was motivated by https://phabricator.services.mozilla.com/D296488#inline-1611780. Essentially this looked confusingly like it was doing things in the "button" variable namespace.

Requesting changes for @Gijs' latest review comments.

I'm open to rubber-stamping this as we've already burnt too much time on this single button. The custom fiddling and reinventing the wheel seems unfortunate, and this is exactly what the design system aimed to prevent. Maybe I'm wish-casting, but I seem to remember reading somewhere that an upcoming design update brings the button back in line with the design system? Would be great if we could get there sooner rather than later.