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

[css-images] Clarify how gradients should be serialized in presence of the double-position color syntax #2714

Open
emilio opened this issue May 28, 2018 · 3 comments

Comments

@emilio
Copy link
Collaborator

emilio commented May 28, 2018

Current behavior is:

document.body.style.backgroundImage = "linear-gradient(black 0%, black 50%)"
"linear-gradient(black 0%, black 50%)";
document.body.style.backgroundImage // "linear-gradient(black 0%, black 50%)"

I'd expect that behavior to be preserved in presence of the double-position stop syntax, but right now in Blink's implementation that returns:

"linear-gradient(black 0% 50%)"

I think defining how gradients serialize in presence of this syntax is worth doing before implementations ships this.

There would be three options IIUC:

Keep the gradient as-specified.

This would be the nicest IMO, since it doesn't break old code and allows the new syntax.

Canonicalize gradients if possible.

This is what Blink does. This would be OK if we deem the compact impact is not relevant / enough, though then the algorithm to canonicalize them needs to be defined and tested.

Never canonicalize gradients.

All gradients would serialize with the "expanded" syntax. I'd be ok-ish with this, though it may not be that great.

@emilio
Copy link
Collaborator Author

emilio commented May 28, 2018

cc @LeaVerou

@css-meeting-bot
Copy link
Member

The Working Group just discussed https://github.com/w3c/csswg-drafts/issues/2714, and agreed to the following:

  • RESOLVED: always expand gradient syntax in serialization
The full IRC log of that discussion <dael> Topic: https://github.com//issues/2714
<fantasai> We should add an issue note in the draft tho
<dael> github: https://github.com//issues/2714
<dael> leaverou: A few weeks ago we cleared the 2 position color stop for shipping. Right now chrome serializes any gradient with 2 colors stops with same color Chrome u ses this syntax. Worried will b eak code.
<dael> leaverou: Options either gradient read back as it's specified or canonicalize gradients. If 2 color stops have same color they're compressed. Or never cannonicalize and always use the expanded syntax
<dael> leaverou: No strong opinion. Worried about cannonicalizing existing not spec this way. It could break parsing code. I w ould vote for another. Prob return expanded syntax in all cases. No strong opinion
<AmeliaBR> I'd also lean towards serializing with the older/explicit syntax, instead of the newer one. The new syntax is just sugar for declarations.
<dael> dbaron: I think 2 principles on how to define serialization. 1 is that you want to try and serialize to shorter form. THat's weaker in this case. Other is a syntax that's evolved o ver time you want to serialize to older.
<dael> dbaron: I think the older formprinciple should win over shorter. That still leaves remember how it was spec or always expand syntax. No strong opinion between those
<dael> TabAtkins: How long ago did we ship always collapse?
<dael> emilio: Hasn't yet.
<dael> leaverou: Intent to ship was a few days ago. It's Chrome 69.
<dael> emilio: It needs the three [missed] to ship. Needs to be resolved on this.
<chris> s/[missed]/LGTM's
<leaverou> Correction: That's conic-gradients that will ship in 69. The 2 position syntax may ship later.
<dael> TabAtkins: I was hoping we had shipped with no compat. Then I vote always exapnded because keeping track of syntax is an extra bit on every stop that's not otherwise needed
<dael> frremy: Always expand seems better
<dael> leaverou: Agree
<dael> emilio: Seems good to me. Forget everything at parse time.
<dael> TabAtkins: I assume that currently we're forgetting at parse time and then going back and we can delete that bit and make it simplier
<dael> astearns: Objections to always expand gradient syntax?
<dael> RESOLVED: always expand gradient syntax in serialization

aarongable pushed a commit to chromium/chromium that referenced this issue Jun 7, 2018
w3c/csswg-drafts#2714 clarified that
serialization should always use the expanded form.

Bug: 850537
Change-Id: I77e60df149fb490b0c2c1db78f020a7246ea7b5c
Reviewed-on: https://chromium-review.googlesource.com/1090820
Reviewed-by: Fredrik Söderquist <fs@opera.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565358}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Oct 2, 2018
…=emilio

This commit adds the multi-position gradient color-stops syntax.

GradientItem::parse_comma_separated is extended to attempt to parse
a LengthOrPercent after each color stop. If it succeeds, it appends
an additional color stop with a duplicate color and the specified
position.

This change is only to the parsing, serialization is left unchanged
as per [1].

[1] w3c/csswg-drafts#2714

Differential Revision: https://phabricator.services.mozilla.com/D7380

--HG--
extra : rebase_source : 88f2e9276c626bf4e195f10c5b5713861eeb5760
extra : source : 18e81538f6f439a8c02a03f21fff22cc5e6128f4
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 3, 2018
This commit adds the multi-position gradient color-stops syntax.

GradientItem::parse_comma_separated is extended to attempt to parse
a LengthOrPercent after each color stop. If it succeeds, it appends
an additional color stop with a duplicate color and the specified
position.

This change is only to the parsing, serialization is left unchanged
as per [1].

[1] w3c/csswg-drafts#2714

Differential Revision: https://phabricator.services.mozilla.com/D7380

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1352643
gecko-commit: e85309ff67f47c2ee52944499f6ababe1278fabd
gecko-integration-branch: mozilla-inbound
gecko-reviewers: emilio
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Oct 3, 2018
…=emilio

This commit adds the multi-position gradient color-stops syntax.

GradientItem::parse_comma_separated is extended to attempt to parse
a LengthOrPercent after each color stop. If it succeeds, it appends
an additional color stop with a duplicate color and the specified
position.

This change is only to the parsing, serialization is left unchanged
as per [1].

[1] w3c/csswg-drafts#2714

Differential Revision: https://phabricator.services.mozilla.com/D7380

--HG--
extra : rebase_source : c580e4bd0e4013d19d8418870aa04eb56fa29303
extra : intermediate-source : e85309ff67f47c2ee52944499f6ababe1278fabd
extra : source : 18e81538f6f439a8c02a03f21fff22cc5e6128f4
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 3, 2018
This commit adds the multi-position gradient color-stops syntax.

GradientItem::parse_comma_separated is extended to attempt to parse
a LengthOrPercent after each color stop. If it succeeds, it appends
an additional color stop with a duplicate color and the specified
position.

This change is only to the parsing, serialization is left unchanged
as per [1].

[1] w3c/csswg-drafts#2714

Differential Revision: https://phabricator.services.mozilla.com/D7380

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1352643
gecko-commit: 984902ba6faf1864c04fef525f6764983439ce1b
gecko-integration-branch: central
gecko-reviewers: emilio
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 4, 2018
This commit adds the multi-position gradient color-stops syntax.

GradientItem::parse_comma_separated is extended to attempt to parse
a LengthOrPercent after each color stop. If it succeeds, it appends
an additional color stop with a duplicate color and the specified
position.

This change is only to the parsing, serialization is left unchanged
as per [1].

[1] w3c/csswg-drafts#2714

Differential Revision: https://phabricator.services.mozilla.com/D7380

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1352643
gecko-commit: 984902ba6faf1864c04fef525f6764983439ce1b
gecko-integration-branch: central
gecko-reviewers: emilio
emilio pushed a commit to emilio/servo that referenced this issue Oct 9, 2018
This commit adds the multi-position gradient color-stops syntax.

GradientItem::parse_comma_separated is extended to attempt to parse
a LengthOrPercent after each color stop. If it succeeds, it appends
an additional color stop with a duplicate color and the specified
position.

This change is only to the parsing, serialization is left unchanged
as per [1].

[1] w3c/csswg-drafts#2714

Differential Revision: https://phabricator.services.mozilla.com/D7380
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 3, 2019
…=emilio

This commit adds the multi-position gradient color-stops syntax.

GradientItem::parse_comma_separated is extended to attempt to parse
a LengthOrPercent after each color stop. If it succeeds, it appends
an additional color stop with a duplicate color and the specified
position.

This change is only to the parsing, serialization is left unchanged
as per [1].

[1] w3c/csswg-drafts#2714

Differential Revision: https://phabricator.services.mozilla.com/D7380

UltraBlame original commit: e85309ff67f47c2ee52944499f6ababe1278fabd
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 3, 2019
…=emilio

This commit adds the multi-position gradient color-stops syntax.

GradientItem::parse_comma_separated is extended to attempt to parse
a LengthOrPercent after each color stop. If it succeeds, it appends
an additional color stop with a duplicate color and the specified
position.

This change is only to the parsing, serialization is left unchanged
as per [1].

[1] w3c/csswg-drafts#2714

Differential Revision: https://phabricator.services.mozilla.com/D7380

UltraBlame original commit: 984902ba6faf1864c04fef525f6764983439ce1b
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 3, 2019
…=emilio

This commit adds the multi-position gradient color-stops syntax.

GradientItem::parse_comma_separated is extended to attempt to parse
a LengthOrPercent after each color stop. If it succeeds, it appends
an additional color stop with a duplicate color and the specified
position.

This change is only to the parsing, serialization is left unchanged
as per [1].

[1] w3c/csswg-drafts#2714

Differential Revision: https://phabricator.services.mozilla.com/D7380

UltraBlame original commit: e85309ff67f47c2ee52944499f6ababe1278fabd
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 3, 2019
…=emilio

This commit adds the multi-position gradient color-stops syntax.

GradientItem::parse_comma_separated is extended to attempt to parse
a LengthOrPercent after each color stop. If it succeeds, it appends
an additional color stop with a duplicate color and the specified
position.

This change is only to the parsing, serialization is left unchanged
as per [1].

[1] w3c/csswg-drafts#2714

Differential Revision: https://phabricator.services.mozilla.com/D7380

UltraBlame original commit: 984902ba6faf1864c04fef525f6764983439ce1b
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 3, 2019
…=emilio

This commit adds the multi-position gradient color-stops syntax.

GradientItem::parse_comma_separated is extended to attempt to parse
a LengthOrPercent after each color stop. If it succeeds, it appends
an additional color stop with a duplicate color and the specified
position.

This change is only to the parsing, serialization is left unchanged
as per [1].

[1] w3c/csswg-drafts#2714

Differential Revision: https://phabricator.services.mozilla.com/D7380

UltraBlame original commit: e85309ff67f47c2ee52944499f6ababe1278fabd
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 3, 2019
…=emilio

This commit adds the multi-position gradient color-stops syntax.

GradientItem::parse_comma_separated is extended to attempt to parse
a LengthOrPercent after each color stop. If it succeeds, it appends
an additional color stop with a duplicate color and the specified
position.

This change is only to the parsing, serialization is left unchanged
as per [1].

[1] w3c/csswg-drafts#2714

Differential Revision: https://phabricator.services.mozilla.com/D7380

UltraBlame original commit: 984902ba6faf1864c04fef525f6764983439ce1b
@svgeesus
Copy link
Contributor

@emilio wrote:

though then the algorithm to canonicalize them needs to be defined and tested.

Does that still need to be defined, before the edits to gradient serialization are made?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants