Skip to content
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

[cssom] [mediaqueries] Media query serialization doesn't work for newer spec features #5627

Closed
emilio opened this issue Oct 16, 2020 · 3 comments

Comments

@emilio
Copy link
Collaborator

emilio commented Oct 16, 2020

I recently became confused because WebKit / Blink sorted the media features in media query lists, but @lilles did point out to me that https://drafts.csswg.org/cssom/#serialize-a-media-query does say that:

  1. Sort the media features in lexicographical order.

But this has gone very out of date with newer mediaqueries features (like nested expressions, etc), which for now only Gecko supports. This also gets more interesting when we support the multi-value expression syntax in #2791...

Gecko has never implemented the sorting step, so I'd propose to drop it: we don't know of any compat issue due to that, it was just that I was looking at interop around MediaQueryList as a result of a Gecko bug. Also, Gecko's behavior is more intuitive to me but maybe I'm biased :)

If we don't drop it, we need to define how the sorting works for stuff like nested expressions and such. Basically, we'd need to define sorting among two arbitrary <media-condition>s.

Thoughts?

@lilles
Copy link
Member

lilles commented Oct 16, 2020

Drop sorting. I can't imagine anyone relying on non-interoperable re-ordering of media query expressions (famous last words).

Blink also currently removes duplicates, which isn't part of the algorithm, but is indicated by an example in the spec. We should remove that too.

@tabatkins
Copy link
Member

Yup, we should just preserve the expression as-is.

I'll go fix the example that shows off deduplication.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed MQ serialization, and agreed to the following:

  • RESOLVED: Do not sort or dedup MQs when serializing
The full IRC log of that discussion <TabAtkins> Topic: MQ serialization
<astearns> github: https://github.com//issues/5627
<TabAtkins> astearns: Proposed resolution is to drop the CSSOM-specified sorting of MQs in lexicogrpahic order when serializing
<fantasai> TabAtkins: No objection, but one example shows removing duplicates as well
<fantasai> TabAtkins: Should also make sure that is covered as well
<TabAtkins> astearns: Consensus in the issue, looks like
<TabAtkins> emilio: Don't think Gecko has ever sorted, and we don't seem to have an issue
<TabAtkins> astearns: Did you dedup?
<TabAtkins> emilio: Don't think so; maybe as part of MQList apis, but not serialization. will doule-check
<TabAtkins> astearns: So any objections?
<TabAtkins> RESOLVED: Do not sort or dedup MQs when serializing

emilio added a commit to emilio/web-platform-tests that referenced this issue Oct 19, 2020
emilio added a commit to emilio/web-platform-tests that referenced this issue Oct 20, 2020
emilio added a commit to web-platform-tests/wpt that referenced this issue Oct 20, 2020
@emilio emilio closed this as completed Oct 20, 2020
pull bot pushed a commit to Yannic/chromium that referenced this issue Oct 21, 2020
This was removed from the spec per resolution[1] and incompatible with
never media queries. Improves interop with Gecko which have not seen any
issues with the different serialization.

Removed fast/media test which is covered by existing wpt tests.

[1] w3c/csswg-drafts#5627 (comment)

Bug: 1138859
Change-Id: I1483008c81df90f8277dcad7e90c8036c5cc019b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2478992
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819090}
Hexcles added a commit to web-platform-tests/wpt that referenced this issue Oct 22, 2020
This was removed from the spec per resolution[1] and incompatible with
never media queries. Improves interop with Gecko which have not seen any
issues with the different serialization.

Removed fast/media test which is covered by existing wpt tests.

[1] w3c/csswg-drafts#5627 (comment)

Bug: 1138859
Change-Id: I1483008c81df90f8277dcad7e90c8036c5cc019b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2478992
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819090}
Hexcles added a commit to web-platform-tests/wpt that referenced this issue Oct 22, 2020
This was removed from the spec per resolution[1] and incompatible with
never media queries. Improves interop with Gecko which have not seen any
issues with the different serialization.

Removed fast/media test which is covered by existing wpt tests.

[1] w3c/csswg-drafts#5627 (comment)

Bug: 1138859
Change-Id: I1483008c81df90f8277dcad7e90c8036c5cc019b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2478992
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819090}
caiolima pushed a commit to caiolima/webkit that referenced this issue Oct 26, 2020
https://bugs.webkit.org/show_bug.cgi?id=217751

Reviewed by Simon Fraser.

LayoutTests/imported/w3c:

* web-platform-tests/css/cssom/mediaquery-sort-dedup-expected.txt: annotate progression

Source/WebCore:

This matches what Gecko has shipped for ages.

The spec used to contain the sorting but not the de-duplication.

Both the spec and Chromium have been updated to match Gecko, see
w3c/csswg-drafts#5627

* css/MediaQuery.cpp:
(WebCore::MediaQuery::MediaQuery): Don't sort / dedup

Tests: imported/w3c/web-platform-tests/css/cssom/mediaquery-sort-dedup.html

LayoutTests:

* fast/media/media-query-serialization.html: Adjust to match spec.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@268966 268f45cc-cd09-0410-ab3c-d52691b4dbfc
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Oct 27, 2020
…lization quirks., a=testonly

Automatic update from web-platform-tests
[cssom] Add a test for media query serialization quirks.

See w3c/csswg-drafts#5627 (comment)

--

wpt-commits: 3a2ec8e80160b0004a7a7e04e1b7efed12e1bbd3
wpt-pr: 26178
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Oct 27, 2020
…ia queries., a=testonly

Automatic update from web-platform-tests
Remove sorting and de-duplication in media queries.

This was removed from the spec per resolution[1] and incompatible with
never media queries. Improves interop with Gecko which have not seen any
issues with the different serialization.

Removed fast/media test which is covered by existing wpt tests.

[1] w3c/csswg-drafts#5627 (comment)

Bug: 1138859
Change-Id: I1483008c81df90f8277dcad7e90c8036c5cc019b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2478992
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819090}

--

wpt-commits: d004bddde373550fe9a563487adda3e17fc4c697
wpt-pr: 26242
sidvishnoi pushed a commit to sidvishnoi/gecko-webmonetization that referenced this issue Oct 27, 2020
…lization quirks., a=testonly

Automatic update from web-platform-tests
[cssom] Add a test for media query serialization quirks.

See w3c/csswg-drafts#5627 (comment)

--

wpt-commits: 3a2ec8e80160b0004a7a7e04e1b7efed12e1bbd3
wpt-pr: 26178
sidvishnoi pushed a commit to sidvishnoi/gecko-webmonetization that referenced this issue Oct 27, 2020
…ia queries., a=testonly

Automatic update from web-platform-tests
Remove sorting and de-duplication in media queries.

This was removed from the spec per resolution[1] and incompatible with
never media queries. Improves interop with Gecko which have not seen any
issues with the different serialization.

Removed fast/media test which is covered by existing wpt tests.

[1] w3c/csswg-drafts#5627 (comment)

Bug: 1138859
Change-Id: I1483008c81df90f8277dcad7e90c8036c5cc019b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2478992
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819090}

--

wpt-commits: d004bddde373550fe9a563487adda3e17fc4c697
wpt-pr: 26242
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
This was removed from the spec per resolution[1] and incompatible with
never media queries. Improves interop with Gecko which have not seen any
issues with the different serialization.

Removed fast/media test which is covered by existing wpt tests.

[1] w3c/csswg-drafts#5627 (comment)

Bug: 1138859
Change-Id: I1483008c81df90f8277dcad7e90c8036c5cc019b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2478992
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819090}
GitOrigin-RevId: 99589ad5ed41501ce54f1e39fd9fd17371336596
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants