[mediaqueries] add tests for custom media queries#48480
Conversation
dbaron
left a comment
There was a problem hiding this comment.
I think these largely look good. I made a few suggestions to add additional tests that seem like they'd be useful. It's fine to add those now -- but if you'd prefer to just get these tests landed and maybe add those later, that's also ok too.
| <head> | ||
| <title>Test: custom media queries that are undefined behave different from queries that evaluate to false</title> | ||
| <link rel="author" title="Romain Menke" href="https://github.com/romainmenke"> | ||
| <link rel="help" href="https://www.w3.org/TR/mediaqueries-5/#at-ruledef-custom-media"> |
There was a problem hiding this comment.
Maybe also link to both https://www.w3.org/TR/mediaqueries-4/#evaluating and https://www.w3.org/TR/mediaqueries-4/#error-handling for this test?
There was a problem hiding this comment.
Good suggestion!
I've added these links.
| <!doctype html> | ||
| <html> | ||
| <head> | ||
| <title>Test: support for custom media queries</title> |
There was a problem hiding this comment.
It might be nice to pair this test with a set of 5 similar things that all don't match. (For example, a custom query that isn't defined, a false one, the not of a true one, etc.
There was a problem hiding this comment.
I've added this in a separate file (and shifted all the file numbers).
| <head> | ||
| <title>Test: custom media queries with cycles are invalid</title> | ||
| <link rel="author" title="Romain Menke" href="https://github.com/romainmenke"> | ||
| <link rel="help" href="https://www.w3.org/TR/mediaqueries-5/#at-ruledef-custom-media"> |
There was a problem hiding this comment.
In this test it might be good to also test that cyclic custom media queries become undefined even when they have or values that also make them clearly true. For example, these are undefined per spec:
@custom-media --test1-a screen or (--test1-b);
@custom-media --test1-b screen or (--test1-a);There was a problem hiding this comment.
Good catch!
screen or (--test1-b) isn't valid, but screen, (--test4-b) is and I think it also covers what you want to test.
|
Thank you for reviewing these @dbaron I also realized that some tests were missing:
So I've also added some tests for these aspects. |
|
|
||
| /* by source order */ | ||
| @custom-media --test1 false; | ||
| @media (--test1) { |
There was a problem hiding this comment.
Should this evaluate to false? We resolved source order for @Custom-Media: w3c/csswg-drafts#12536 (comment)
There was a problem hiding this comment.
Correct!
Thank you for catching this.
I'll update this to match that resolution
| @media not ((color) or (monochrome)) { | ||
| .test1 { background: green; } | ||
| } |
There was a problem hiding this comment.
Sorry, I don't understand, what is it needed for?
There was a problem hiding this comment.
I honestly don't remember and also don't understand why it is needed.
I'll remove it
Add tests for https://www.w3.org/TR/mediaqueries-5/#at-ruledef-custom-media
see: web-platform-tests/interop#714 (comment)