w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.5k stars 661 forks source link

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

Open emilio opened 6 years ago

emilio commented 6 years ago

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 commented 6 years ago

cc @LeaVerou

css-meeting-bot commented 6 years ago

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

The full IRC log of that discussion <dael> Topic: https://github.com/w3c/csswg-drafts/issues/2714
<fantasai> We should add an issue note in the draft tho
<dael> github: https://github.com/w3c/csswg-drafts/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
svgeesus commented 8 months ago

@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?

svgeesus commented 3 months ago

I made some suggestions for the one-stop case