csswg-drafts icon indicating copy to clipboard operation
csswg-drafts copied to clipboard

[mediaqueries] Merge error handling section into evaluating section

Open gsnedders opened this issue 2 years ago • 3 comments

At this point, it's not clear what the motivation to have https://andreubotella.com/csswg-auto-build/mediaqueries-4/#error-handling as a separate section to https://andreubotella.com/csswg-auto-build/mediaqueries-4/#evaluating is.

The current split leads to some ambiguities in places:

An unknown <media-type> must be treated as not matching.

What does "not matching" mean, given "a media query is a logical expression that is either true or false"? (This also runs into #7594 given we don't define how to evaluate <media-type>.)

An unknown <mf-name> or <mf-value>, or a feature value which does not matches the value syntax for that media feature, results in the value “unknown”.

Results in the value "unknown" for what? Does it make the <media-feature> unknown, or the closest parent <media-condition> unknown, or does it make the whole <media-query> unknown? Implementations currently differ here!

A <media-query> whose value is “unknown” must be replaced with not all.

But, as above, a media query is either true or false? What does it mean to "replace" a <media-query>?

It seems like we may well want to define the rewriting to not all in Serializing Media Queries instead?

gsnedders avatar Aug 11 '22 17:08 gsnedders

Results in the value "unknown" for what? Does it make the <media-feature> unknown, or the closest parent <media-condition> unknown, or does it make the whole <media-query> unknown? Implementations currently differ here!

To quote @mdubet in https://github.com/web-platform-tests/wpt/issues/35435:

Firefox Developer Edition (v104):

> window.matchMedia("(scan: foobar)")
> MediaQueryList { media: "not all", matches: false, onchange: null }

> window.matchMedia("(foobar: none) or (color)")
> MediaQueryList { media: "not all", matches: false, onchange: null }

In Chrome Canary:

> window.matchMedia("(scan: foobar)")
> MediaQueryList {media: '(scan: foobar)', matches: false, onchange: null}

> window.matchMedia("(foobar: none) or (color)")
> MediaQueryList {media: '(foobar: none) or (color)', matches: true, onchange: null}

gsnedders avatar Aug 11 '22 17:08 gsnedders

The CSS Working Group just discussed [mediaqueries] Merge error handling section into evaluating section, and agreed to the following:

  • RESOLVED: Properly define resolution of media type and top-level MQ
  • RESOLVED: Clarify the spec text to reflect intended unknown mechanics
  • RESOLVED: Replace "replace by not all" with "evaluates to false", open a separate issue on serialization
The full IRC log of that discussion <fantasai> Topic: [mediaqueries] Merge error handling section into evaluating section
<fantasai> github: https://github.com/w3c/csswg-drafts/issues/7595
<fantasai> gsnedders1: We currently don't have interop with some types of errors around invalid media features
<florian> q+
<fantasai> gsnedders1: because essentially we don't define clearly how any of this is meant to work
<astearns> ack lea
<fantasai> gsnedders1: there were proposals to put this into Interop 2023
<fantasai> gsnedders1: need to know what exactly we'll be testing
<fantasai> gsnedders1: a few sections
<fantasai> gsnedders1: First, we don't define how to evaluate media type or the top-level media query at all
<fantasai> gsnedders1: and we just say that an unknown media type is treated as not matching
<fantasai> gsnedders1: whereas unknown media feature [logic]
<fantasai> gsnedders1: Suggest to define how to evaluate MQ with handwavy definition of media types matching if appropriate
<fantasai> TabAtkins: Agree
<astearns> ack florian
<fantasai> florian: I agree, media type is not properly defined, but that's not hard. Can be more specific than mentioned
<fantasai> florian: since most deprecated by now
<fantasai> florian: so only two cases where we differ from screen
<fantasai> florian: for top-level MQ, what it ought to be is fairly obvious but we should write it down
<fantasai> gsnedders1: Proposed resolution is to define how to evaluate media query and media type
<fantasai> gsnedders1: in somewhat obvious way
<fantasai> astearns: any objections?
<fantasai> RESOLVED: Properly define resolution of media type and top-level MQ
<fantasai> gsnedders1: Next, unknown media name or ???
<fantasai> gsnedders1: results in value of uknown
<fantasai> gsnedders1: but we don't say what exactly is given unknown
<fantasai> gsnedders1: an MF name or MF value doesn't have a value
<fantasai> gsnedders1: Currently FF and Chrome differ
<fantasai> gsnedders1: FF makes the entire MQ uknown if any media feature name or value is unknown
<fantasai> gsnedders1: whereas Chrome only makes that specific one unknown
<fantasai> florian: Chrome is right, the whole point of this logic is doing this
<fantasai> TabAtkins: [agrees]
<fantasai> gsnedders1: I can justify both behaviors from current spec text
<fantasai> florian: We'll clarify, the intention is 100% clear to the spec editors, so we just need to fix the tet
<fantasai> RESOLVED: Clarify the spec text to reflect intended unknown mechanics
<fantasai> gsnedders1: Final bit is, MQ whose value is unknown must be replaced with "not all"
<fantasai> gsnedders1: but not clear what it means to replace an MQ
<fantasai> gsnedders1: maybe need to define MQ serialization?
<fantasai> TabAtkins: I think that's right, but...
<fantasai> TabAtkins: I think it's a serialization issue
<fantasai> gsnedders1: I think we need to say that unknown MQ evaluates to false, and serializes to "not all"
<fantasai> florian: makes sense to me
<fantasai> emilio: We don't have this thing to support ... with generalized enclosed
<fantasai> emilio: is that a compat requirement?
<fantasai> florian: part where unknown MQ evaluates to false, that is absolutely desired intent
<fantasai> florian: whether accidentally replaced with "not all" with no effect on serialization or whether was intended to talk about serialization, then I don't know
<fantasai> gsnedders1: I think it goes back to CSS2
<TabAtkins> MQ has a bunch of legacy weirdness that I didn't replace
<fantasai> gsnedders1: It could literally just be copy-pasted from earlier level with no thinking
<fantasai> emilio: WE don't implement this update to the spec, but if we did we'd probably want to be consistent with @supports
<fantasai> emilio: and @supports when you throw something random at it, you serialize that
<fantasai> emilio: rather than a false statement
<fantasai> florian: should we say, gets evaluated as false? or does that lose something important?
<fantasai> emilio: I think so, but not sure how that maps. I think spec said this had to be some value with 3-way state...
<fantasai> florian: when you propagate unknown ...
<fantasai> emilio: That was not the desired behavior, and we changed it. Unknown just evaluates as false
<fantasai> emilio: so probably we want to be consistent, and unknown evaluates as false?
<fantasai> TabAtkins: we very specifically want @supports unknown to be false, because if you can't parse it you definitely don't support it
<fantasai> TabAtkins: but for MQ, whether you are that type of media or not, you don't know if you can't parse the MQ
<fremy> +1 to what TabAtkins just said
<fantasai> florian: I think that the "gets replaced by not all" is an ancient and imprecise way of saying evaluate as false, and we should replace this and not talk about serialization
<fantasai> gsnedders1: with prior resolution, very few if anything will result in the whole MQ being unknown
<fantasai> TabAtkins: not necessarily true, e.g. combine with and and you'll get unknown propagate up
<fantasai> gsnedders1: yes, nevermind
<fantasai> astearns: Should we resolve, or need more time?
<fantasai> emilio: Fine as long as we're all on the same page
<fantasai> florian: Instead of "replace by not all" say "evalutates to false"
<fantasai> gsnedders1: define serialization of unknown MQ to be "not all", is that still the intention?
<fantasai> florian: Let's open a separate issue for serialization, since might have compat concerns
<fantasai> florian: Want to look into that offline
<fantasai> RESOLVED: Replace "replace by not all" with "evaluates to false", open a separate issue on serialization

css-meeting-bot avatar Oct 19 '22 16:10 css-meeting-bot

Okay, so at least we need to preserve a distinction between "false" and "invalid" - the latter serializes as "not all" for legacy reasons, but the former is serialized properly. We don't expose MQs in any form except serialized, so whether the "not all"-ification is a a serialization thing or actually replacing the MQ is up to us. (I'd favor it being serialization.)

I now recall that I specifically carried over the "not all" behavior for things that resolve to unknown because that's what previous UAs would do to such MQs. It's not perfect; they'd also serialize (unknown) or (min-width: 0) as not all, whereas now it'll be true and just serialize as-is. That said, I'm fine with breaking compat more fully here and making unknown things always serialize properly. However, I suspect that the "unknown media type" still needs to serialize to "not all", as that's likely the most common way we're exposing this behavior today.

tabatkins avatar Oct 19 '22 23:10 tabatkins

I now recall that I specifically carried over the "not all" behavior for things that resolve to unknown because that's what previous UAs would do to such MQs.

The tests currently seem to use serialising not all as a test for "media query could be parsed", after @andruud's changes back in May (which match his implementation in Chrome), i.e., only expecting not all for:

A media query that does not match the grammar in the previous section must be replaced by not all during parsing.

For example, the tests assert that @media screen, (max-width) {} must serialise as @media screen, (max-width) {}.

So it seems like Chrome doesn't implement that serialising behaviour now? And it sounded on the call like @emilio was reasonably questioning whether we want to rewrite it on serialisation rather than propagating the unknown media query to the output (which to me sounds like the sensible thing to do), and if Chrome is currently shipping then I guess we have the evidence it's web compatible.

However, I suspect that the "unknown media type" still needs to serialize to "not all", as that's likely the most common way we're exposing this behavior today.

We don't seem to have tests for that, at least.

gsnedders avatar Oct 20 '22 00:10 gsnedders

Oh, that's very promising then.

tabatkins avatar Oct 20 '22 15:10 tabatkins

Minor note: there is nothing about <mf-value>s in the "wrong order" in <mf-range>, eg. (2px < width < 1px). I will consider that this case must always parse as valid but must evaluate to false (as Chrome/FF do), which is the "default" behavior, but this does not conform to the matching convention mentioned in CSS Values for clamp(), and I am not sure it should conform to this convention.

EDIT: a 3-value range in a wrong order can only be false, therefore I change my mind and I think it should be a parse error, so that the author can fix it.

cdoublev avatar Nov 15 '22 20:11 cdoublev

Before this change, it was possible to detect non-supported media queries by checking for "not all", as developers, including myself, have documented in blog posts like this.

window.matchMedia('(prefers-color-scheme: dark)').media
// '(prefers-color-scheme: dark)'

// This check no longer works.
window.matchMedia('(hahaha-lol: not-a-thing)').media
// Before: 'not all'
// Now: '(hahaha-lol: not-a-thing)'

Is this a potential web compatibility issue, as feature-detecting code like the above now breaks?

(CC: @mathiasbynens in the context of this DevTools CL)

tomayac avatar Aug 16 '23 17:08 tomayac

Minor note: there is nothing about <mf-value>s in the "wrong order

Nothing particularly wrong with that. It'll never be true, but it's just a shorthand for writing the two comparisons separately, which you can always do. Making it a parse error would be unnecessarily complex, and would mean that things like env() evaluation have to happen at parse time, which seems unnecessary.

tabatkins avatar Aug 17 '23 00:08 tabatkins

(FWIW, updated my blog post.)

tomayac avatar Aug 17 '23 15:08 tomayac

Can you please confirm that your intent is to apply the not all-ification only in the first case below?

  1. 1: it does not match any branch of the grammar
  2. unknown: it matches <media-type> (<ident>) and evaluates to unknown
  3. (unkown): it matches <general-enclosed> (or <mf-name> -> <mf-boolean> -> <media-feature>?) and evaluates to unknown

I would definitely expect it to apply in (this very contrived example):

styleSheet.insertRule(`@media ${matchMedia(';').media} {}}`)

@tomayac, maybe this could be a solution?

Resolved: Add an at-rule function with syntax at-rule(@keyword) or at-rule(@keyword; descriptor: value)

https://github.com/w3c/csswg-drafts/issues/2463#issuecomment-1016720310

For example, @supports at-rule(@media; some-feature). Media queries are not technically descriptors though, so maybe @supports media-query(some-feature).

cdoublev avatar Oct 07 '23 09:10 cdoublev

#2463 (comment)

For example, @supports at-rule(@media; some-feature). Media queries are not technically descriptors though, so maybe @supports media-query(some-feature).

Thanks, I hadn't seen this before. I assume this would be queryable with CSS.supports('at-rule(@media; some-feature)')!? In a contrived example where (some-feature: value1 | value2), but where only value2 would be supported, could you somehow query this (with CSS or JS)? I guess more concretely I'm actually asking if CSS.supports('at-rule(@media; (prefers-color-scheme: dark)') would work, or only CSS.supports('at-rule(@media; prefers-color-scheme'). The way I read it, both should work?!

tomayac avatar Oct 09 '23 14:10 tomayac

Yes, I also assume it.

We can already do CSS.supports('selector(type) and selector(.class)'), so you could do CSS.supports('media-query(some-feature: value1) or media-query(some-feature: value2)') (declaration), CSS.supports('media-query(some-feature > 1px)') (range), or CSS.supports('media-query(some-feature)') (boolean).

cdoublev avatar Oct 09 '23 19:10 cdoublev