-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Define <meta media>. Fixes #6495. #6569
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable to me.
You also need to update the element index to add media
to meta
.
Good catch. Fixed in 43d605f. |
…Fix incorrect data-x attribute value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me modulo nits.
Because of the IDL attribute we need to require tests and implementer support, I think.
I fixed them in 0381fe9.
Yup! I've filed Implementation bugs & a Mozilla standards-positions issue, and @gsnedders is going to write some tests. |
Looking at the history of |
My understanding is that we generally nudge people toward web app manifests instead, but I will ask around. |
<li><p>If <var>element</var> has a <code data-x="attr-link-media">media</code> attribute | ||
and the value of <var>element</var>'s <code data-x="attr-meta-media">media</code> | ||
attribute does not <span data-x="matches the environment">match the environment</span>, then | ||
<span>continue</span>.</p></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bgrins pointed out to me that this step would preclude updating the theme color if the user changed their preferences, e.g., from light to dark mode. That doesn't seem great?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For link@rel=stylesheet in https://html.spec.whatwg.org/multipage/links.html#link-type-stylesheet it just relies on CSSOM to define this; for all other link types it seems like https://html.spec.whatwg.org/multipage/semantics.html#processing-the-media-attribute doesn't actually reprocess media either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's one of the reasons we have to be careful about using media
with <link>
(and why I pushed back on it for rel=manifest
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bgrins pointed out to me that this step would preclude updating the theme color if the user changed their preferences, e.g., from light to dark mode. That doesn't seem great?
I don't think that's true? If the user updates their preferences (to dark mode, say), this step will cause <meta ... media="(prefers-color-scheme: light)">
to be skipped. Which is what you want. On a subsequent iteration of the loop, when this step looks at <meta ... media="(prefers-color-scheme: dark)">
, it won't skip it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the time
|
|
…-run when the environment changes.
We are supportive of the feature, but have not assigned anyone to implement it. I am asking around to see where this might fall in the backlog. Discussion about that is on the Chrome bug. |
Any movement on this? I would really like to see this land in HTML this week. I hear no objections. We have consensus about the syntax — there's nothing left to debate. We have interest from two implementors. Spec work is underway to add this ability to web manifest files as well, so that we have parity. What's the blocker? |
The blocker is a second implementer that has the feature on their near term roadmap, per the WHATWG working mode's "should". See Anne's comment from 28 days ago. |
For multiple equally appropriate
Why is this different for |
This PR doesn't change that aspect of |
@hober You're right it doesn't change that aspect specifically. The addition of the |
There's an in-progress Chromium CL by @beaufortfrancois of the Chrome team that adds support for this. Does this count as a second implementer? |
Yeah, that seems really promising. Let's wait to see if they actually intend to prototype or ship (by sending the corresponding email to blink-dev), but if that happens and this doesn't just end up a personal experiment in a branch, we'll be in good shape. |
@beaufortfrancois has the email in draft. It’s not just an experiment. 😃 |
Yep @beaufortfrancois is excited to bring this feature to Chrome ;) Expect an intent to be sent next week. FYI @b1tr0t |
And here's our intent to prototype and ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/6I-I3lZWy5k |
This CL makes sure the media HTML attribute is supported for the meta HTML element when name is "theme-color" so that the status bar color is reflected when media features change (light/dark mode for instance). Intent to prototype: https://groups.google.com/a/chromium.org/g/blink-dev/c/6I-I3lZWy5k Spec PR: whatwg/html#6569 Demo: https://media-meta-theme-color.glitch.me/ Change-Id: Id284c9b67f6d61635680357f337211f1a8c14dc6 Bug: 1200528 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2945116 Commit-Queue: François Beaufort <beaufort.francois@gmail.com> Reviewed-by: Yoav Weiss <yoavweiss@chromium.org> Reviewed-by: Rune Lillesveen <futhark@chromium.org> Reviewed-by: David Roger <droger@chromium.org> Cr-Commit-Position: refs/heads/master@{#893838}
This CL enables by default the flag to honor the media HTML attribute for <meta name="theme-color">. Intent to ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/6I-I3lZWy5k Spec PR: whatwg/html#6569 Demo: https://media-meta-theme-color.glitch.me/ Bug: 1200528 Change-Id: Ic8fed423a4610a4f2dfbad31728715400aa0de24 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2959617 Reviewed-by: Yoav Weiss <yoavweiss@chromium.org> Reviewed-by: Rune Lillesveen <futhark@chromium.org> Commit-Queue: François Beaufort <beaufort.francois@gmail.com> Cr-Commit-Position: refs/heads/master@{#894186}
FYI It's in Chrome Canary as well now: https://twitter.com/quicksave2k/status/1407326048526770176 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your patience and edits!
source
Outdated
@@ -13689,7 +13689,7 @@ interface <dfn>HTMLLinkElement</dfn> : <span>HTMLElement</span> { | |||
data-x="attr-link-media">media</code> attribute is prescriptive. The user agent must apply the | |||
external resource when the <code data-x="attr-link-media">media</code> attribute's value | |||
<span>matches the environment</span> and the other relevant conditions apply, and must not apply | |||
it otherwise.</p><!-- note similar text in <style> element section --> | |||
it otherwise.</p><!-- note similar text in <style> and <meta> element sections --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment change should be reverted. I will fix and then merge.
…attribute, a=testonly Automatic update from web-platform-tests Add reflection tests for the meta@media attribute For whatwg/html#6569. -- wpt-commits: 8be2ce944ac18cae010c851f23b7f3f06826f913 wpt-pr: 28876
…attribute, a=testonly Automatic update from web-platform-tests Add reflection tests for the meta@media attribute For whatwg/html#6569. -- wpt-commits: 8be2ce944ac18cae010c851f23b7f3f06826f913 wpt-pr: 28876
This CL makes sure the media HTML attribute is supported for the meta HTML element when name is "theme-color" so that the status bar color is reflected when media features change (light/dark mode for instance). Intent to prototype: https://groups.google.com/a/chromium.org/g/blink-dev/c/6I-I3lZWy5k Spec PR: whatwg/html#6569 Demo: https://media-meta-theme-color.glitch.me/ Change-Id: Id284c9b67f6d61635680357f337211f1a8c14dc6 Bug: 1200528 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2945116 Commit-Queue: François Beaufort <beaufort.francois@gmail.com> Reviewed-by: Yoav Weiss <yoavweiss@chromium.org> Reviewed-by: Rune Lillesveen <futhark@chromium.org> Reviewed-by: David Roger <droger@chromium.org> Cr-Commit-Position: refs/heads/master@{#893838} NOKEYCHECK=True GitOrigin-RevId: 99a4b42f00c0839bb0b5b3d56cca2a06315dd1b0
This CL enables by default the flag to honor the media HTML attribute for <meta name="theme-color">. Intent to ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/6I-I3lZWy5k Spec PR: whatwg/html#6569 Demo: https://media-meta-theme-color.glitch.me/ Bug: 1200528 Change-Id: Ic8fed423a4610a4f2dfbad31728715400aa0de24 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2959617 Reviewed-by: Yoav Weiss <yoavweiss@chromium.org> Reviewed-by: Rune Lillesveen <futhark@chromium.org> Commit-Queue: François Beaufort <beaufort.francois@gmail.com> Cr-Commit-Position: refs/heads/master@{#894186} NOKEYCHECK=True GitOrigin-RevId: 4ad19e2adda9a33a5ff79cedcfbe5d7d39387491
(See WHATWG Working Mode: Changes for more details.)
/acknowledgements.html ( diff )
/indices.html ( diff )
/semantics.html ( diff )