Add support for defining a theme color for both light & dark modes (prefers color scheme)

jensimmons commented 3 years ago

Currently, in a web manifest file, you can define a theme color. https://www.w3.org/TR/appmanifest/#theme_color-member For example:

{
    "lang": "en",
    "short_name": "Resilience",
    "name": "Resilient Web Design by Jeremy Keith",
    "description": "A web book in seven chapters on the past, present, and future of web design. By Jeremy Keith.",
    "background_color": "#5f7995",
    "theme_color": "#5f7995"
}

There's not currently any way to express that a certain theme color should be used for light mode, while another is used for dark mode.

In HTML, there's currently a proposal to add support for color schemes by adding a media attribute to the meta tag: https://github.com/whatwg/html/issues/6495

<meta name="theme-color" media="(prefers-color-scheme: light)" content="red">
<meta name="theme-color" media="(prefers-color-scheme: dark)"  content="darkred">

It would be great to be able to do a similar thing in the web manifest file.

malchata commented 3 years ago

One approach that sounds good to me would be to overload "theme_color" to accept either a string (the current behavior), or a nested object like so:

"theme_color": {
  "dark": "#000000",
  "light": "#ffffff"
}

Not sure if this approach conflicts with existing functionality in any way, but it's the first approach that came to mind.

dcrousso commented 3 years ago

I don't think we can touch "theme_color" as that will break older browsers.

We may want to create a "theme_colors" (plural) property that is an array of objects (so that it's extensible if needed in the future) that would take precedence over "theme_color"

"theme_color": "red",
"theme_colors": [
    { "color": "red" },
    { "color": "darkred", "media": "(prefers-color-scheme: dark)" }
]

I don't have a strong preference, but I think using the last item in the list that matches (i.e. last pass) is generally easier to understand/follow (it also more closely matches the way it would be written in CSS).

If this is an approach we like, perhaps we could/should do it for "background_color" (i.e. "background_colors") too?

malchata commented 3 years ago

I'm in favor of this approach. It leaves existing functionality where it is, and extends the web manifest in a way that allows for existing or future media values should they become applicable in this scenario.

mirisuzanne commented 3 years ago

That approach makes sense to me as well – although I might think of it as a default/shorthand, and longhand list of alternates, rather than new and legacy values. That would allow a simpler combination in the short term:

"theme_color": "red",
"theme_colors": [
    { "color": "darkred", "media": "(prefers-color-scheme: dark)" }
]

Authors could also choose to provide only the default:

"theme_color": "red",

Or (down the road) move to only using the longhand:

"theme_colors": [
    { "color": "red" },
    { "color": "darkred", "media": "(prefers-color-scheme: dark)" }
]

tomayac commented 3 years ago

Also see https://github.com/w3c/image-resource/issues/26 where the same question is discussed in the context of ImageResource as used by Web Application Manifest.

mgiuca commented 3 years ago

+1, see my comment on w3c/image-resource#26 where I said "sure it would be great for ImageResources, but we'd also want it for the theme color". @aarongustafson FYI.

We (Google) are thinking about implementing something like this in the near future (~N quarters) so having agreement on the exact format of this field in the manifest would be great.

@mirisuzanne 's proposal sounds good to me. The problem it's trying to solve (not breaking backwards compat with the existing theme_color spec) has echoes of display-override and the solution is similar (have a separate field that overrides the existing field's values with more nuanced information).

aarongustafson commented 3 years ago

Thanks for filing this @jensimmons, It’s been on my backlog for a few weeks.

We discussed this over in #955 as well. I think the piece that’s missing from the above suggestions is that multiple members may need to change based on the color scheme. My original thought was that we introduce a color_schemes member which supports multiple color schemes as defined by CSS. That way it stay in lock-step with that spec as it continues to evolve. So, for example:

"color_schemes": {
  "dark": {
    "theme_color": "#fff",
    "background_color": "#000"
  },
  "light": {
    "theme_color": "#000",
    "background_color": "#fff"
  }
}

Of course there’s also prefers_contrast and forced_colors, which we may want to consider in any solution. Perhaps, combining this idea with @mirisuzanne’s:

"color_overrides": {
  "(prefers-color-scheme: dark)": {
    "theme_color": "#fff",
    "background_color": "#000"
  },
  "(prefers-color-scheme: light)": {
    "theme_color": "#000",
    "background_color": "#fff"
  }
}

or

"color_overrides": [
  {
    "media": "(prefers-color-scheme: dark)",
    "theme_color": "#fff",
    "background_color": "#000"
  },
  {
    "media": "(prefers-color-scheme: light)",
    "theme_color": "#000",
    "background_color": "#fff"
  }
]

In terms of processing, I’m not sure which would be faster to compute. We’d probably also want the first match to win if there are multiple matches.

Thoughts?

mirisuzanne commented 3 years ago

I like your first combined syntax, with media-conditions as keys, with nested objects. If we were matching the way CSS works, I would expect last-take-precedence when there are multiple matches - but maybe first-takes-precedence fits the existing scheme better? I don't know.

aarongustafson commented 3 years ago

I like your first combined syntax, with media-conditions as keys, with nested objects.

That’s my preference as well. Not knowing the innards of browser engines, it also feels like it would be faster to process because you wouldn’t have to parse the object to determine whether you need to throw it away.

If we were matching the way CSS works, I would expect last-take-precedence when there are multiple matches - but maybe first-takes-precedence fits the existing scheme better? I don't know.

This is the confusing piece, I agree. So we have 2 conflicting paths for this stuff: In CSS proximity wins whereas in things like audio, video, and picture the first match wins (even with media in play). Again, not knowing the browser innards well and going purely by the processing algorithms we define in the spec, I believe the first match winning is less computationally expensive overall, which is why I tend to lean in that direction.

I’d love someone with knowledge of the perf cost to weigh in here and set me straight if I’m incorrect.

conde2 commented 3 years ago

"color_overrides": [
  {
    "media": "(prefers-color-scheme: dark)",
    "theme_color": "#fff",
    "background_color": "#000"
  },
  {
    "media": "(prefers-color-scheme: light)",
    "theme_color": "#000",
    "background_color": "#fff"
  }
]

1+ for this one, would like to see this in webmanifest

jensimmons commented 3 years ago

Re: adding theme_colors to live alongside theme_color, and perhaps extending that idea to add background_colors to live alongside background_color...

This is the kind of thing the CSSWG always avoids. It's very confusing for developers to have remember whether or not there's an 's' at the end — and that the version with the 's' works differently than the one without. This is especially true for folks who don't speak English fluently or at all, which is an incredible number of developers.

An entirely different name is much better, like color_schemes or color_overrides. This is something that people can learn & understand completely separate from the original mechanism for theme color... and eventually let theme_color fade away as something the industry ignores when teaching or doing web development in the future.

aarongustafson commented 3 years ago

100% agree with @jensimmons on the naming stuff above.

Could @mgiuca @marcoscaceres @tomayac @rayankans and/or anyone else who works on internals weigh in on any perf differences between the options I outlined above.

marcoscaceres commented 3 years ago

I can't see any perf differences, but I'd be inclined to go with:

"color_overrides": [
  {
    "media": "(prefers-color-scheme: dark)",
    "theme_color": "#fff",
    "background_color": "#000"
  },
  {
    "media": "(prefers-color-scheme: light)",
    "theme_color": "#000",
    "background_color": "#fff"
  }
]

Just from an object creation POV. I'm personally not a huge fan of property names having a dual role.

However, we will need to work out what to do when multiple media queries/features match. Last wins? first wins? Most specific wins?

marcoscaceres commented 3 years ago

Hmmm..., we'd probably want to just generalize the overrides... and now I've changed my opinion about the property names, as it's clear what it's doing...

"media_overrides": {
  "(prefers-color-scheme: dark)": {
    "theme_color": "#fff",
    "background_color": "#000"
  },
  "(prefers-color-scheme: light)": {
    "theme_color": "#000",
    "background_color": "#fff"
  }
}

tomayac commented 3 years ago

I like the elegancy of the above proposal and move my thumbs up to this:

"media_overrides": {
  "(prefers-color-scheme: dark)": {
    "theme_color": "#fff",
    "background_color": "#000"
  },
  "(prefers-color-scheme: light)": {
    "theme_color": "#000",
    "background_color": "#fff"
  }
}

The approach feels very familiar to how you'd do this in CSS:

@media (prefers-color-scheme: dark) {
  /* … */
}

As an analogy, for processing <link media>, the spec simply says the resource must be applied if the media query matches. In case of multiple matches, I'd vouch for using the last matching one.

marcoscaceres commented 3 years ago

Half joking... we could apply them in order:

{
"background_color": "white",
"media_overrides": {
  "all": {
    "background_color": "green",
    "name": "lolz"
  },
  "(prefers-color-scheme: light)": {
    "background_color": "blue",
    "theme_color": "#000"
  }
}
}

Would:

always override "background_color": "green",
Add "name", because why not...
add "theme_color": "#000" iff (prefers-color-scheme: light)

🧐

tomayac commented 3 years ago

With you on the…

add "theme_color": "#000" iff (prefers-color-scheme: light)

…part.

Not sure what the difference of the "media": "all" part would be for "background-color" compared to just the regular "background-color". I mean, yes, you could do that I guess…

For the "name" in there, I think we should limit the allowed children of an "@media block" (we need a name for this) and limit it somehow to just visible things.

marcoscaceres commented 3 years ago

For the "name" in there, I think we should limit the allowed children of an "@media block" (we need a name for this) and limit it somehow to just visible things.

Yes, we could define a list props that it applies to, and only override those (ignore the rest).

marcoscaceres commented 3 years ago

Ok, so concrete proposal is now "media_overrides", whose member names are a valid <media-query-list> production of Media Queries. For each declared property, the members are limited to:

theme_color
background_color

Rough processing:

For each "query" of get own property names "media_overrides":

If query is not a valid MQ, continue.
Evaluate query. If media matches, use the property value (by re-processing it to make sure it's valid).

That should provide the maximum flexibility.

Thoughts?

tomayac commented 3 years ago

For "icons", which we deal with in https://github.com/w3c/image-resource/issues/26, it seems like @aarongustafson's proposal is the latest and greatest.

It would be great to align the two proposals. From what I can tell, ImageResource icons are used in the "icons" member, but also nested in the "shortcuts" member.

Combined, this would look like in the example below. There's some duplication with the "shortcuts", but I think it's tolerable.

{
    "media_overrides": {

        "(prefers-color-scheme: dark)": {
            "theme_color": "#fff",
            "background_color": "#000",

            "icons": [{
                "src": "/icons/icon-dark.png",
                "sizes": "192x192"
            }],

            "shortcuts": [{
                "name": "Play",
                "short_name": "Play",
                "description": "Play the list of podcasts",
                "url": "/play?utm_source=homescreen",
                "icons": [{
                    "src": "/icons/play-dark.png",
                    "sizes": "192x192"
                }]
            }]
        },

        "(prefers-color-scheme: light)": {
            "theme_color": "#000",
            "background_color": "#fff",

            "icons": [{
                "src": "/icons/icon-light.png",
                "sizes": "192x192"
            }],

            "shortcuts": [{
                "name": "Play",
                "short_name": "Play",
                "description": "Play the list of podcasts",
                "url": "/play?utm_source=homescreen",
                "icons": [{
                    "src": "/icons/play-light.png",
                    "sizes": "192x192"
                }]
            }]
        }
    }
}

aarongustafson commented 3 years ago

This is great folks! I agree that I like the syntax better where the key is the MQ. Should I start a PR on this?

For "icons", which we deal with in w3c/image-resource#26, it seems like @aarongustafson's proposal is the latest and greatest.

Yes, I have it tracked in a bug against ImageResource as well. Ideally, it’d be great to have the ImageResource support color prefs so as not to create the kind of complicated duplication you mentioned, @tomayac. But if folks need to override separately, it’d be there as a fallback.

Just to cover all the bases, I’d suggest we try to land the ImageResource one first so the Manifest spec can reference it.

dcrousso commented 3 years ago

While I don't have a personal preference over how this is structured, I would give some caution over using <media-query-list> as property keys instead of as the value of a "media" property inside the object as the former basically prevents any combination with some other form of "only apply this when X" system that could get added in the future. I would be somewhat surprised if something like that was added, but I also can't deny it as a possibility and would hate to introduce yet another "*_overrides" and have to figure out a precedence/combination logic if that does happen.

tomayac commented 3 years ago

Trying to speak for developers, I think consistency is key here. I don’t think it’s a good experience for developers having to remember that for ImageResource it’s a "media" property they need to set and for "media_override" the media query is the actual key. So I’d rather go for using a "media" property throughout. Sorry for the back-and-forth here.

Another point that I realize now: If folks create their manifests programmatically, it’s also more convenient to be able to foo.media rather than having to use foo['(prefers-foo: bar)'].

jensimmons commented 3 years ago

Yeah, hot take — I’m not sure what media_override means. Overriding? What am I overriding? Isn't all my code technically an override of previously-parsed code?

Instead, media matches what I’d write in HTML and in CSS. The more we can make the code similar in all three places, the better.

<meta name="theme-color" media="(prefers-color-scheme: light)" content="red">
<meta name="theme-color" media="(prefers-color-scheme: dark)"  content="darkred">

@media (prefers-color-scheme: dark) {
  img {
    filter: brightness(.8) contrast(1.2);
  }
}

"media": {
  "(prefers-color-scheme: dark)": {
    "theme_color": "#fff",
    "background_color": "#000"
  },
  "(prefers-color-scheme: light)": {
    "theme_color": "#000",
    "background_color": "#fff"
  }
}

dcrousso commented 3 years ago

from my earlier comment

basically prevents any combination with some other form of "only apply this when X" system that could get added in the future

this actually exists right now in @supports. Using a <media-query-list> as the key means that @supports wouldn't be allowed. Especially since we're dealing with CSS colors I think it'd perhaps be useful for a developer to be able to do something along the lines of

{
    "conditions": ["@media (prefers-color-scheme: dark)", "@supports (color: color(display-p3 1 1 1 1))"],
    "theme_color": "color(display-p3 1 1 1 1)",
    ...
}

(i.e. check that the exact color they want to use is supported before attempting to use it)

marcoscaceres commented 3 years ago

@jensimmons... great points about matchings css/html! and I know I've misspelled "override" a lot 😅, so the shorter "media" is nice for that reason too!

@dcrousso, I think that's pushing it a bit beyond the use case. Worst case, you can do the supports checks in JS, then instruct either the service worker or server to respond with the particular color syntax. I'd prefer we keep things simple unless there is strong case for checking @supports.

@aarongustafson, let's get a all the straw-person in place and engine buy-in (or no objections!) before we proceed with spec'ing it.

If Mozilla supports adding this, I'd be happy to implement it in Gecko. @annevk or @tantek, are either of you the right people to ask about this? Or is this something we ned to get a standards position on? Appreciate your guidance.

@jensimmons, @hober, similar question. We know Apple doesn't comment on future products or implementations, but would you object to this being added to the spec?

Any further refinements appreciated. I think we are still unclear if we should process all the matches, or if first or last wins.

annevk commented 3 years ago

Something like this seems reasonable to me and in line with the arguments put forward in https://github.com/mozilla/standards-positions/issues/500 against multiple manifests (some restating much older arguments dating back to the original design of manifests). It seems small enough to me to not warrant a position.

(Thanks in advance for the patch!)

marcoscaceres commented 3 years ago

@tomayac wrote:

There's some duplication with the "shortcuts", but I think it's tolerable

I'm not liking the duplication of shortcuts, TBH. I wonder if we can do better there... it looks like image resources should sprout at media member?

{"shortcuts": [
    {
        "name": "Play",
        "short_name": "Play",
        "description": "Play the list of podcasts",
        "url": "/play?utm_source=homescreen",
        "icons": [
            {
                "media": "(prefers-color-scheme: light)",
                "src": "/icons/play-light.png",
                "sizes": "192x192"
            },
            {
                "media": "(prefers-color-scheme: dark)",
                "src": "/icons/play-dark.png",
                "sizes": "192x192"
            }
        ]
    }
]}

alancutter commented 3 years ago

I don't think this key-value API surface is suitable:

"media_overrides": {
  "(prefers-color-scheme: dark)": {
    "theme_color": "#fff",
    "background_color": "#000"
  },
  "(prefers-color-scheme: light)": {
    "theme_color": "#000",
    "background_color": "#fff"
  }
}

JSON object properties aren't ordered (in the spec). It wouldn't be clear which one "wins" if there are overlaps.

marcoscaceres commented 3 years ago

JSON object properties aren't ordered (in the spec). It wouldn't be clear which one "wins" if there are overlaps.

They are ordered in the JS spec and in our spec. We explicitly use an ordered map.

To be clear, all browsers treat JSON properties as ordered irrespective of what the JSON spec says.

alancutter commented 3 years ago

Thanks! I was working on decade old out of date knowledge. Having JSON object properties ordered in spec opens up a few doors for APIs, neat!

tomayac commented 3 years ago

I'm not liking the duplication of shortcuts, TBH. I wonder if we can do better there...

@marcoscaceres Neither do I. But if we agree to add "media" to ImageResource dictionaries, which we both seem to be aligned on, then we're indeed good.

jensimmons commented 3 years ago

Have we fully thought through what happens with forced-color, or the other work being done on Media Queries 5? https://drafts.csswg.org/mediaqueries-5/#forced-colors

@Tabatkins @fantasai @frivoal

marcoscaceres commented 3 years ago

While the CSS crew is here, somewhat unrelated, but related, could we move "display mode media feature" to MQ5? https://www.w3.org/TR/appmanifest/#the-display-mode-media-feature

mgiuca commented 3 years ago

Weighing in here on the syntax:

Quick summary:

If order is important, we should use an array, not an object (relying on the order of the keys). But I don't think the order should be important.
I strongly strongly dislike the syntax "(prefers-color-scheme: dark)" (either as a key or as a value). We should use simple strings like "light" and "dark".

Aaron's first proposal of these three SGTM.

On the order of dictionary keys

@marcoscaceres wrote:

They are ordered in the JS spec and in our spec. We explicitly use an ordered map.

To be clear, all browsers treat JSON properties as ordered irrespective of what the JSON spec says.

To go into the technicals: yes, we use an ordered map to represent the output of processing the manifest, but we still start by parsing JSON into an object, which we define to be "object" as defined in the JSON spec, RFC 8259. That spec explicitly defines "object" as "an unordered collection of zero or more name/value pairs" (emphasis mine). So per our own specification, we aren't allowed to depend on the order of keys (despite what the implementations make possible).

But I don't think the spec really matters that much (after all we could change what the spec says if we really wanted to). I think what's more important is how confusing it is for developers. I would argue, very. When I see the syntax:

{
   "x": "y",
   "a": "b"
}

in JSON, JavaScript, Python, Ruby, Lua, etc, I have a basic assumption that the order of the keys won't matter, because they're going to be inserted into either a hash table or a sorted tree. It is very unintuitive to tell developers that the order of these keys matters; it goes against decades of experience in virtually all modern programming languages.

If you want to define a data structure where the order matters, use an array.

Having said that, I don't have a problem with Aaron's first syntax:

"color_schemes": {
  "dark": {
    "theme_color": "#fff",
    "background_color": "#000"
  },
  "light": {
    "theme_color": "#000",
    "background_color": "#fff"
  }
}

because this isn't a case of "try one and fall back to the other", right? It would be "if there's a match, use it, if not, use the theme_color and background_color from the top-level object. Am I understanding correctly?

On the strings used in the key

It's not clear to me whether the key "(prefers-color-scheme: light)" would be matched as a literal value, or whether it would be parsed as CSS or something else.

If we're planning to literally put into the spec to match against the key (or value) string "(prefers-color-scheme: light)" and "(prefers-color-scheme: dark)", then why would we use such a complex string? Why not just light and dark?
If we're planning to parse the contents of this string, please please don't. It looks like this is trying to look like CSS? Are you actually suggesting that we run a CSS parser over this string? Or that we define a new mini-language that we will parse inside this string? Either way, please don't parse these strings. Represent the data structure we need in JSON.

Again, Aaron's first suggestion seems fine to me.

frivoal commented 3 years ago

While the CSS crew is here, somewhat unrelated, but related, could we move "display mode media feature" to MQ5? https://www.w3.org/TR/appmanifest/#the-display-mode-media-feature

Opened an issue in the csswg to discuss that: https://github.com/w3c/csswg-drafts/issues/6343 (I'm supportive)

marcoscaceres commented 3 years ago

@mgiuca wrote:

I strongly strongly dislike the syntax "(prefers-color-scheme: dark)"

The syntax is the media queries syntax (<media-query-list>). The limitation of "light" and "dark" as keys is that it's restricted to, well, just color-scheme "light" and "dark". With using a media query, we get the full ability to check of any <media-query-list> matches.

And interesting discussion to have is if having full-blown <media-query-list> support is worthwhile or overkill.

From an extensibility perspective, it's pretty nice.

However, the challenge we face is then who actually evaluates the <media-query-list> to do the member selection? If it's the OS, then "light" and "dark" is definitely better: we can't expect an OS to do anything worthwhile with <media-query-list>, and converting <media-query-list> to something that the OS understands would be quite challenging and error prone.

If it's the user agent doing the selection, then <media-query-list> would be most appropriate.

It is very unintuitive to tell developers that the order of these keys matters; it goes against decades of experience in virtually all modern programming languages.

Except JS and on the Web Platform, where the order is retained. What happens in those other languages doesn't seems particularly relevant, as we are dealing with the web platform where order of JSON members is retained and significant.

frivoal commented 3 years ago

Have we fully thought through what happens with forced-color, or the other work being done on Media Queries 5? https://drafts.csswg.org/mediaqueries-5/#forced-colors

Seems perfectly reasonable to me, and I'd say that the ability to tap into queries other than prefers-color-scheme is a nice extension point. While defined by the csswg, mediaqueries aren't meant to be limited to css only, and are defined to be usable in other contexts too. This seems like a completely reasonable fit. That said, while all the stuff from MQ5 should be fine, the simple width and height media features (and some others from MQ3 and MQ4) might need a little clarification to make sure we agree on what is the object whose size (etc) they're measuring. Presumably, that'd be the size of the web app's web view itself, but possibly it could (in some contexts only?) have something to do with the size at which the icon is being displayed, or that sort of things. Probably not, but we should be explicit.

As for forced colors, it comes from a OS / UA setting, so presumably, the OS/UA already knows how to apply it to any amount of web-app chrome that is outside of the web view itself, and for the content of the webview (or the impact on media queries), the spec existing spec should apply just fine as is.

tomayac commented 3 years ago

+1 for extensibility. A good candidate could be prefers-contrast: more, so, developers could, for example, ship a set of icons without gradients and clearer outlines in such cases.

hober commented 3 years ago

I think the reasoning in https://github.com/whatwg/html/issues/6495#issuecomment-800592391 applies re: using media query syntax v. some new micro syntax. This should be consistent with <meta media>.

mgiuca commented 3 years ago

@hober if we're going to be using that HTML proposal (adding a media attribute with CSS MQ in it) as a precedent, then it would make sense to use Aaron's third proposal:

"color_overrides": [
  {
    "media": "(prefers-color-scheme: dark)",
    "theme_color": "#fff",
    "background_color": "#000"
  },
  {
    "media": "(prefers-color-scheme: light)",
    "theme_color": "#000",
    "background_color": "#fff"
  }
]

That kinda-sorta mirrors the HTML meta attribute.

But I still don't like the complexity (I get that it's more flexible, but if developers don't need that flexibility, we're just adding massive complexity to all the tools that need to process the manifest, for no gain).

I think this passage from Marcos is the key issue:

However, the challenge we face is then who actually evaluates the <media-query-list> to do the member selection? If it's the OS, then "light" and "dark" is definitely better: we can't expect an OS to do anything worthwhile with <media-query-list>, and converting <media-query-list> to something that the OS understands would be quite challenging and error prone.

If it's the user agent doing the selection, then <media-query-list> would be most appropriate.

I think it should definitely be open to the former. I want the freedom to be able to delegate everything possible from the manifest to the OS or to other software that doesn't have a full browser inside it. Examples:

An app store should be able to parse the manifest and use what it finds to create a page for the app.
The user agent should be able to convert the metadata into some format the OS understands, for example, on Linux, convert it into a FreeDesktop .desktop file to let the OS interpret it as it would a native app.
A packaging tool should be able to convert it into a native OS package format, such as an Android package, which has its own manifest format.
A scanning tool like Lighthouse should be able to interpret the manifest format without having to have an entire CSS parser built into it.

Obviously we can't convert everything in the Manifest format 100% losslessly, but I want to minimize data loss when converting to another format. I haven't looked closely at these other formats for this particular issue, but I would imagine, say, Android would let you set a "light theme color" and "dark theme color" for an app. If we have a MQ for this, it will be very difficult for a converter tool to convert the manifest format into any other format.

Some folks on my team pointed out that we already depend on CSS parsing for processing the manifest: for color values. We could see this as a precedent, but a word of caution: this has caused a lot of issues for us, basically same as the above. Internally (in Chromium), it's made it impossible for us to move the manifest parser out of Blink due to the dependency on the CSS parser. And it also means we have a lossy conversion when we convert those colour values to other formats like Android packages (I would guess that some more obscure ways to specify a colour probably break when processed by a "simple" Manifest parser that's not hooked up to a CSS engine). I'd like to do less of this, not more.

As for Aaron's second suggestion (having the MQ be in the key), this just seems totally inappropriate to me. Dictionary keys should be fixed identifier-like strings, not parseable syntax.

Except JS and on the Web Platform, where the order is retained. What happens in those other languages doesn't seems particularly relevant, as we are dealing with the web platform where order of JSON members is retained and significant.

Wow, TIL that JS retains the order of keys in the dictionary. I still maintain that this is very unintuitive for developers. As I understand, this was introduced in ES2015 because some apps were relying on it. The reason I brought up all those other languages is that in general, in computer science, a dictionary is considered to be an unordered list of key/value pairs. The JSON spec that the Manifest spec references literally says this. So according to the letter of the spec, an implementation of the Manifest spec would be well within its rights to randomize the order of the keys.

I suppose this is tied to the MQ issue. If we don't use MQs, then the order shouldn't matter (you can only match "light" or "dark").

aarongustafson commented 3 years ago

@mgiuca @marcoscaceres @hober @frivoal @tomayac, et al. I truly appreciate all the back-and-forth here. I can definitely understand Matt’s reluctance toward wanting to ship a CSS parser with a Manifest parser, but at the same time I want to make sure we are being at least a little forward-looking in consideration of any solution in this area. To my mind, we need to be able to support two things:

dark/light modes (current problem)
other user-preferences (future problems)

If we solve for one without solving for the other, I think we’re going to regret the decision.

As most of y’all have said, I don’t relish the thought of creating some new micro-syntax, but we also need a way to directly connect what authors put in their Manifest to what they—or their team members—are doing in their CSS. To that end, it might be possible to do that using a color_overrides object if we allowed it to be keyed with any valid MQ "prefers-" suffix key combined with its value, all of which would be derived from the MQ spec. So, for example

"color_overrides": {
  "color-scheme-dark": {
    "theme_color": "#fff",
    "background_color": "#000"
  },
  "color-scheme-light": {
    "theme_color": "#000",
    "background_color": "#fff"
  },
  "contrast-more": {
    "theme_color": "#000",
    "background_color": "#fff"
  }
}

Browsers (or OSes) could ignore the ones they don’t understand (as per usual) and we have a clearly-defined path forward for enabling other user preferences down the line. This would allow us to avoid the CSS parser dependency without wholly jettisoning the connection between the two features.

Other key options like media_overrides or preference_overrides might make more sense than color_overrides if we want to allow folks to redefine icons and such too.

dmurph commented 3 years ago

If we go with "light" and "dark", I don't think that necessarily means we couldn't do the other options later, the only complication would be to spec how "light" and "dark" apply in those new configurations.

e.g. if we decide to do "color-scheme-dark" "contrast-more" approach, we can just say that "dark" is overridden if "color-scheme-dark" is also specified, same with media query (which I guess can be detected by ( and ), but IDK much about css parsing).

I'm a big fan of keeping things simple and complicating later only if necessary.

mgiuca commented 3 years ago

Thanks @aarongustafson .

Looking at your example here, it looks a lot simpler than the CSS MQ which I am a fan of.

However, I still think this should be a list, not a dictionary. See my above comments about whether dictionary order should matter. If order matters, it would be clearer for color_overrides be a list instead:

"color_overrides": [
  {
    "color-scheme": "dark",
    "theme_color": "#fff",
    "background_color": "#000"
  },
  {
    "color-scheme": "light",
    "theme_color": "#000",
    "background_color": "#fff"
  },
  {
    "color-scheme": "contrast-more",
    "theme_color": "#000",
    "background_color": "#fff"
  }
]

That way it's clear that this is an ordered fallback chain, just like other newly proposed features display_override and capture_links.

Other key options like media_overrides or preference_overrides might make more sense than color_overrides if we want to allow folks to redefine icons and such too.

Perhaps media_overrides is the best of these three. I think "color" is too specific and "preference" is too broad for the sorts of things we're trying to capture here.

marcoscaceres commented 3 years ago

@mgiuca wrote:

Some folks on my team pointed out that we already depend on CSS parsing for processing the manifest: for color values.

Yes. Gecko also handles this by normalizing color values to RGBA and then to a format Android supports:

https://github.com/mozilla/gecko-dev/blob/fd2fe22dc1226cc475ace26d4dd84f155049763e/dom/manifest/ValueExtractor.jsm#L74-L83

This is easy to do for colors, because the conversion is done once, but it's specifically targeting Android. And I don't know what other formats, say, would be needed for Windows or MacOS should Firefox one day support those.

However, I still think this should be a list, not a dictionary.

I agree, as that would give us parity with how we use ImageResources (e.g., icons).

The only thing I would change is the name "color_overrides"... I'd still just go with "media" for the reasons @jensimmons gave, and to not lock us into only affecting "color" stuff, should we need something else in the future.

I think we are are getting close to agreement 🎉🙏🤞

marcoscaceres commented 3 years ago

I'd still like to consider aligning closely with:

https://drafts.csswg.org/mediaqueries-5/#mf-user-preferences

What about this:

{
    "user_preferences": [
        {
            "prefers_color_scheme": "light",
            "theme_color": "white",
            "background_color": "black"
        },
        {
            "prefers_color_scheme": "dark",
            "theme_color": "black",
            "background_color": "white"
        },
        {
            "forced_colors": "active"
        }
    ]
}

Pros:

Restricted specifically to "User Preference Media Features" in MQ5 and above
not a MQ (but can be covered into one easily!)
Shares the MQ5 names and values
works with ImageResource too.
Easy to map to OS.
Doesn't need to deal with web page specific queries.

Cons:

Well, I can't think of any 😂

WDYT?

marcoscaceres commented 3 years ago

Updated the above... wow, I'm totally biased... but I quite like it! 😍

aarongustafson commented 3 years ago

@mgiuca Totally cool with the list approach.

@marcoscaceres I was starting to ruminate in the "user preferences" direction after posting my last message as well. Looking at it again, I do find the mixture of hyphens and underscores a bit confusing, however.

I also do like the symmetry of @jensimmons’ suggestion of media as a key, but don't find the overriding behavior to be as immediately apparent from the key name as it is with a key like translations which implies change. user_preferences (note the underscore) seems a little more clear in this regard.

Gonna sleep on it, but I also think we're close.

tomayac commented 3 years ago

+1 to @marcoscaceres' proposal, with s/-/_/.

marcoscaceres commented 3 years ago

Good catch @tomayac (and @aarongustafson, just noticed you spotted it too 🦅👀)! Updated above...

(we might need some developer warnings in the spec around that! That's gonna bite people 🧛‍♀️)

w3c / manifest

Add support for defining a theme color for both light & dark modes (prefers color scheme) #975