1821893 - Update color-mix() serialization to better align with the spec.

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Tiaan Louw [:tlouw]]

color-mix() uses a calculation to determine whether a percentage in color-mix serialization can be omitted or not. here

The spec and wpt tests seems to just check whether the author added the percentage or not. spec

Comment 1

[Tiaan Louw [:tlouw]]

Attachment: WIP: Bug 1821893 - Fix color-mix() percentages when serializing r=emilio,#layout-reviewers

Now just keep track of whether the author omitted either of the
percentages and take that into account again when serializing.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

What in the spec says this? In general we use the shortest serialization principle to omit percentages that can be omitted.

Comment 3

[Tiaan Louw [:tlouw]]

The spec doesn't specifically say this, but it doesn't say anything about omitting them either. The wpt tests in css/css-color/parsing/color-valid-color-mix-function.html follows the "omit if author omitted" guideline.

Would you rather we clarify the spec or for now just pass the wpt tests?

Comment 4

[Emilio Cobos Álvarez [:emilio]]

I think we should fix the tests. Lacking an explicit spec quote the shortest serialization principle applies, from https://drafts.csswg.org/cssom/#serialize-a-css-value:

If component values can be omitted or replaced with a shorter representation without changing the meaning of the value, omit/replace them.

Comment 5

[Tiaan Louw [:tlouw]]

As all 3 other browsers on wpt follow the tests as is, I added a issue on csswg here: https://github.com/w3c/csswg-drafts/issues/8564 just to clarify, rather then break everyone's tests.

Do you think its worth landing this in the meantime?

Comment 6

[Tiaan Louw [:tlouw]]

Changing this bug to update the WPT tests in stead as per discussion in the w3c issue here: https://github.com/w3c/csswg-drafts/issues/8564

Comment 7

[Tiaan Louw [:tlouw]]

Attachment: Bug 1821893 - Fix wpt tests to follow shortest serialization principle r=emilio

After a discussion here https://github.com/w3c/csswg-drafts/issues/8564
I'm changing the tests to follow the shortest serialization principle
when serializing color-mix functions.

Biggest change is the left percentage is always present when the left
and right add up to 100%, even if the right was specified.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ab0d00ede27e
Fix wpt tests to follow shortest serialization principle r=emilio

Comment 9

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Failed to create upstream wpt PR due to merge conflicts. This requires fixup from a wpt sync admin.

Comment 10

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/ab0d00ede27e

Comment 11

[Daniel Holbert [:dholbert]]

(In reply to Web Platform Test Sync Bot (Matrix: #interop:mozilla.org) from comment #9)

Failed to create upstream wpt PR due to merge conflicts. This requires fixup
from a wpt sync admin.

Hmm, it looks like this^ never got resolved, so these changes never made it upstream.

As a result, wpt.fyi shows us failing a bunch of subtests in this test, with the first one being !EQ("color-mix(in hsl, red 60%, blue 40%)", "color-mix(in hsl, red 60%, blue)"), which was the first subtest that we fixed up here:
https://hg.mozilla.org/integration/autoland/rev/ab0d00ede27e#l2.12

jgraham, what do you suggest doing at this point?

(My guess: given that this test is interop-2023-score-impacting, I suspect we need to file a 'test change proposal' issue at https://github.com/web-platform-tests/interop/issues?page=1&q=is%3Aissue+label%3Atest-change-proposal+created%3A%3E2023-01-01 , correct? And then should we manually generate an upstream pull request to cover our diffs here, or something else?)

Comment 12

[Daniel Holbert [:dholbert]]

(preemptively ticking ni=tlouw to follow up on whatever jgraham recommends in response to comment 11)

Comment 13

[James Graham [:jgraham]]

Yeah, I think at this point it would be best if we could make an upstream PR for our changes, and file a test change proposal issue, so that they can go through the approval process. If we can do that today they'll probably come up in the meeting tomorrow (not necessary, but sometimes helpful).

Comment 14

[Tiaan Louw [:tlouw]]

I created this PR to sync the changes made here: https://github.com/web-platform-tests/wpt/pull/40500

I suspect we will get a conflict when pulling from upstream again once the changes have landed, but then we can just use whatever comes from upstream.

Comment 15

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/40808 for changes under testing/web-platform/tests

Comment 16

[James Graham [:jgraham]]

That PR should be empty and is just to convince the sync bot that there's nothing to do here.