1818829 - Support color configuration of FeatureCallout

Status: RESOLVED FIXED

(Firefox :: Messaging System, enhancement, P2)

Description

[Shane Hughes [:aminomancer]]

The Feature Callout prototypes were all designed for Firefox View, whose colors are based on the user's lightweight theme. But now we're using Feature Callout for PDF.js, whose colors are based on the user's system color scheme, not theme. And it's possible for a user to enable a dark theme while still keeping their content color scheme light, or vice versa.

Even when they do match, PDF.js also has slightly different colors in either light or dark mode than Firefox View. And most importantly, we want to develop Feature Callout as a general-purpose swiss army knife that can point to any feature in the browser, whether in a system page or in the chrome. But the chrome color scheme is fully configurable by theme extensions. All sorts of possibilities are supported there, but we only support light and dark modes, based on a media query. This will be very mismatched for some users, if we release chrome feature callouts without first adding support for theme configuration.

My thinking here is that different FeatureCallout consumers will pass different parameters to the constructor. Whatever code we write that eventually instantiates FeatureCallout for actually anchoring to chrome elements (not to be confused with the PDF.js surface, which runs in the chrome but pins callouts over the content area) would pass something like this:

theme: {
  backgroundColor: "var(--arrowpanel-background-color);",
  color: "var(--arrowpanel-color);",
  borderColor: "var(--arrowpanel-border-color);"
}

And other consumers could pass some shorthand corresponding to hard-coded values, if they don't know what colors they're supposed to use. For Firefox View and the chrome, it's easy enough to tell it which CSS custom properties to inherit from the page. But for PDF.js, it's difficult to pass CSS custom properties up from the content to the chrome so the cfeature callout can be styled similarly to the content.

Instead, that's a case where we'd probably want to just assume ahead of time what the PDF.js colors will be, and define them in _feature-callout.scss, targeting a specific attribute. So to that end, FeatureCallout.sys.mjs could expose a theme property to CSS by adding an attribute. So then the CSS rules could target .onboardingContainer.featureCallout[theme="pdfjs"].

This seems nice to handle in advance so we can start introducing Feature Callouts targeting chrome UI elements in experiments, e.g. for the Tamagotchi concept.

Comment 1

[Shane Hughes [:aminomancer]]

I'm working on this now and I noticed a new issue while referencing the figma prototype: Feature Callout buttons don't have an active style. They only have the default and hover styles. But the figma prototype shows that they actually are supposed to have a subtly darker/brighter active state. I would normally file another bug for it, but I think it'd be more efficient to just fix both of these at the same time, since we'll need to add theme props for the active state anyway - no point adding them in pure CSS, then.

Comment 2

[Shane Hughes [:aminomancer]]

I noticed we also have some HCM issues. The HCM colors don't quite match Firefox View or PDF.js. More importantly, the button label is invisible in the button hover state in HCM, because the text color is the same as the background color. This problem could be solved with pure CSS, but making the HCM colors match the parent/anchor element styles on both Firefox View and PDF.js is not really feasible without theming support, since they are different.

Another bug I encountered but is solved by theming is a color scheme mismatch on PDF.js feature callouts. If you enable a dark browser theme but set your system color scheme to light (or ui.systemUsesDarkTheme=0), you end up with dark chrome UI but light web content. And PDF.js is styled according to the content color scheme, but the browser chrome's color scheme is overridden based on the theme. Normally that wouldn't be a problem, but because the Feature Callouts for PDF.js are actually in the chrome (because it wasn't practical to support FeatureCallout on any arbitrary origin where the PDF.js viewer might load), they end up being styled according to the media queries as they are evaluated in the browser chrome, not as they are evaluated in the PDF.js viewer content. So you get a white PDF.js theme but a dark feature callout, which is a problem since the callout is supposed to look like it's part of the PDF.js viewer. The same is true for the reverse scenario, e.g. enable a light browser theme but set your system color scheme to dark (or ui.systemUsesDarkTheme=1).

Firefox actually provides a special media query for frontend use that does basically exactly what we want: -moz-content-prefers-color-scheme. The problem with it is that it's only supported in the browser chrome, so it can't be a 1:1 replacement for prefers-color-scheme. We have to support feature callouts in multiple contexts — right now it's just Firefox View and PDF.js, but even Firefox View presents an intractable problem for theming with pure CSS, since it doesn't support -moz-content-prefers-color-scheme. So in Firefox View, we need to use prefers-color-scheme, and in PDF.js we need to use -moz-content-prefers-color-scheme.

But also, we have plans to eventually show feature callouts as anchored to browser chrome elements. We can already support that for the most part thanks to our work on PDF.js, but the difference with chrome elements is that the callouts should be themed according to the browser theme, not the content color scheme. So in the chrome we actually don't really want to use either media query, we want to use the lightweight theme properties that are set on the chrome window root element and consumed by most of the chrome window frontend styles.

I came up with a basic chrome "theme" (basically mapping feature callout elements to lightweight theme properties) that works pretty well. We don't currently have a use case for it, but we should be able to trial it in our first chrome feature callout experiment, once the logic is added for showing feature callouts in the chrome on message triggers, like other messaging surfaces (that's essentially bug 1821826).

Comment 3

[Shane Hughes [:aminomancer]]

Attachment: Bug 1818829 - Implement Feature Callout theme configuration. r=#omc-reviewers,jprickett

Add logic to apply theme colors to Feature Callout based on where it's
going to show. We can use in-content CSS properties for Firefox View and
other themed system pages, but not for PDF.js, nor for any callouts we
might show in the browser chrome in the future. For the browser chrome
in general, we can use the lightweight theme properties directly, in the
same way the chrome frontend does. But PDF.js is a special case, since
although it exists in the chrome, it's meant to appear like it's in the
PDF.js viewer. And the PDF.js viewer has its own theme totally
independent of everything else. So this dynamically applies themes from
different sources.

This also fixes the bug where the PDF.js color scheme could mismatch the
PDF.js viewer if the browser theme and system color scheme don't match,
e.g. where system color scheme is light but a dark theme is installed,
or vice versa. For PDF.js specifically, we can use the
-moz-content-prefers-color-scheme media query to follow the color scheme
as it exists in the PDF.js viewer page instead of the color scheme in
the chrome window where the Feature Callout actually exists.

It also adds or modifies some colors that were previously missing or
different from the prototype, fixes the illegibility of buttons in HCM
and forced colors mode, and makes some other minor color changes.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fecef27ba1fa
Implement Feature Callout theme configuration. r=jprickett,omc-reviewers,fxview-reviewers,sfoster

Comment 5

[Cosmin Sabou [:CosminS]]

Backed out for causing newtab failures on _feature-callout-theme.scss

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=WpDe-DbHRSSBFmoeMeF7zg.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=fecef27ba1fa5cf52b78713a276ec575ddfb0579

Failure log: https://treeherder.mozilla.org/logviewer?job_id=410425964&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/33a0722bc7561611b02b541cd5bcd75159d189e9

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b11a1938b397
Implement Feature Callout theme configuration. r=jprickett,omc-reviewers,fxview-reviewers,sfoster

Comment 7

[Narcis Beleuzu [:NarcisB]]

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