Open andruud opened 1 year ago
Other than CSSOM it's problematic for toggle()
as it's currently specified, see #6764
Longhand enumeration is the biggest change, yeah. I think there might be others around web animations. I think if you transition a shorthand you might get multiple CSSTransition objects rather than one? This is from memory, but should be easy to check (not at a computer atm).
For what it's worth, there are precedents like text-decoration
, which was a longhand in CSS 2.1 but got turned into a shorthand in CSS Text Decoration 3. Also text-align
got turned into a shorthand in CSS Text 3 while it was a longhand in CSS 2.1. And to go away from text related styling, overflow
was initially defined as a longhand in CSS 2.1. In CSS Overflow 3 it then got turned into a shorthand.
So this is nothing new. So, any changes to CSSOM should take that into account. An API change like adding shorthands to the enumeration might break some logic. Admittedly, the risk for that is probably very low, though it should be considered, nonetheless.
Sebastian
I initially thought the pending-substitution value might be a problem
Well, I guess it can be a problem if you are trying to copy all the specified styles. Iterating them will just provide the longhands, and they will serialize as empty string. cssText
is typically safer, but an author who only used variables on longhands may not have thought about this.
So this is nothing new. So, any changes to CSSOM should take that into account. An API change like adding shorthands to the enumeration might break some logic. Admittedly, the risk for that is probably very low, though it should be considered, nonetheless.
Actually this seems fairly risky, both performance-wise and breakage-wise.
If we keep the enumeration order alphabetic, there's a chance that a shorthand is enumerated after its longhands, and serializes as the empty string. So something like a loop calling setPropertyValue(prop, value)
deletes all the shorthand's properties already set. Unless you enumerate shorthands before longhands, but then you're back at square one.
To serialize as an empty string, the site would need to use the new longhands (which it doesn't know about), no? Also, using setPropertyValue
with an empty string has no effect, doesn't it? But yes, even though I didn't quite follow the above example, I see that variations of that could break. E.g. setting white-space
first, then e.g. setting text-wrap:initial
through enumeration. (That's not alphabetical order, but ignore that ..)
To serialize as an empty string, the site would need to use the new longhands
Not necessarily explicitly, e.g. this would work if overflow
wasn't a shorthand, without directly using overflow-x/y
:
var style = document.createElement("div").style;
style.overflow = "var(--overflow)";
var clone = document.createElement("div").style;
for (let p of style) {
clone.setProperty(p, style.getPropertyValue(p), style.getPropertyPriority(p));
}
clone.cssText // "" :(
using
setPropertyValue
with an empty string has no effect
It has effect, it removes the previous value (for all longhands in case of a shorthand):
document.body.style.cssText = "grid: 1px / 2px; grid-auto-flow: column";
var cs = getComputedStyle(document.body);
var clone = document.createElement("div").style;
for (let p of cs) { // Imagine this iterates `grid` after its longhands, like [...cs, "grid"]
clone.setProperty(p, cs.getPropertyValue(p), cs.getPropertyPriority(p));
}
[...clone].includes("grid-template-rows"); // Would be false
This is observable in that e.g.
Array.from(getComputedStyle(e))
will no longer containwhite-space
.
Whether gCS should include shorthands is #2877.
This could be fixed and improve DX in one fell swoop by simply including shorthands that can be serialized in getComputedStyle()
. Shorthands not being included has been a huge PITA for authors.
I recall @tabatkins saying that it is well defined whether a shorthand can be serialized from longhands, but I can't remember where.
The PITA was getPropertyValue()
failing to serialize shorthands in a computed style, which used to happen in Firefox, but works now as resolved in #2529.
Not including shorthands in item()
seems desirable to me.
Whether a shorthand can be serialized is defined in https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-getpropertyvalue, basically:
!important
, or no longhand has !important
(this is always the case for getComputedStyle()
).Blink keeps deciding to ship weird things that are not spec-compliant and using this issue as an excuse, so adding it to the agenda.
Blink keeps deciding to ship weird things that are not spec-compliant and using this issue as an excuse, so adding it to the agenda.
…can you actually give at least one example here, rather than just vaguely alluding to things here? For those of us not closely paying attention to everything, we lack context, which isn't an ideal place to be going into a meeting.
@gsnedders See thread at https://groups.google.com/a/chromium.org/g/blink-dev/c/f5eLz6PIXaI/m/a9OGhvaNAAAJ for the latest
I've seen 3 such discussions: baseline-source, text-wrap: balance thread #1, text-wrap: balance thread #2 (cited above).
Also, baseline-source
is tied in to a longhand-to-shorthand refactoring that has a very complicated history and is thus significantly more risky (particularly for existing SVG content, I think) than a "normal" refactoring of a longhand into a shorthand.
I think there might be others around web animations. I think if you transition a shorthand you might get multiple CSSTransition objects rather than one?
Yes, that's right. Not only will there be one CSSTransition
per animated longhand but CSS Transitions will also dispatch one TransitionEvent
per animated longhand (as exposed by TransitionEvent
's propertyName
member).
You can see an example here: https://codepen.io/birtles/pen/wvYrdPo
Web Animations is able to handle shorthands, but CSS Transitions is defined in terms of transitions on longhand properties hence the behavior here.
This is probably fine for white-space
since we only just recently made it possible to transition discrete-value properties, but in general changing longhands to shorthands can break code that is expecting to find a TransitionEvent
with a given propertyName
or CSSTransition
object for a given transitionProperty
.
For example, code such as the following:
// Trigger a transition on the white-space property
elem.style.whiteSpace = 'normal';
elem.style.transition = 'white-space 2s';
getComputedStyle(elem);
elem.style.whiteSpace = 'pre';
// Fetch the corresponding CSSTransition object
const whiteSpaceTransition = elem
.getAnimations()
.find(
(anim) =>
anim instanceof CSSTransition && anim.transitionProperty === 'white-space'
);
whiteSpaceTransition.finished.then(() => {
// Update the rest of the UI
});
This will break once white-space
becomes a shorthand. Similar code can be written in terms of TransitionEvent
s.
I suppose we could introduce some sort of mapping in CSS Transitions for "legacy longhands" to maintain the existing behavior.
There's also potential breakage in the keyframes exposed for CSS Animations since they end up getting expanded to longhands as part of the cascade.
Hello 👋🏼 — I'm Tai an engineer at Webflow. Within this context, I'm a developer building a visual abstraction for HTML/CSS using the underlying browser APIs (like getComputedStyle).
We recently had a regression due to changes to white-space
. Here's the report https://bugs.chromium.org/p/chromium/issues/detail?id=1449763.
We've found a workaround to unblock our users in the meantime, but just wanted to drop by and provide feedback that these types of changes may unexpectedly break sites that assumed that APIs worked a certain way.
Thank you all! 🙇🏼
Hi @anhtaiH! 👋🏼 It’s great to have participation here from tools like Webflow, hope to see more of y'all around!
You could use the newer computedStyleMap()
, which does support shorthands when they are serializable 😊
For the case in https://github.com/w3c/csswg-drafts/issues/8398#issuecomment-1568802501, I think the underlying issue is not whether the property is present in the result of getComputedStyle()
(it should be) but whether you see the property when enumerating that result (it is not).
I initially thought css-typed-om might be a problem, since there is no shorthand support there. (But then I found that the spec seems to allow it, it's just Blink's incomplete implementation that doesn't support it).
@kojiishi points out that this is actually a (spec) problem too:
https://drafts.css-houdini.org/css-typed-om/#computed-stylepropertymapreadonly-objects:
If this’s [[computedStyleMapCache]] internal slot is set to null, set its value to a new StylePropertyMapReadOnly object, whose [[declarations]] internal slot are the name and computed value of every longhand CSS property supported by the User Agent, every registered custom property, and every non-registered custom property which is not set to its initial value on this, in the standard order.
Just like for getComputedStyle
, I think that only having the longhands is reasonable. It's just that StylePropertyMapReadOnly.get()
should somehow handle shorthands, like CSSStyleDeclaration.getPropertyValue()
@kojiishi points out that this is actually a (spec) problem too: [spec says the map only contains longhand properties]
Huh, I'm not sure why I wrote that, actually. Weird. Definitely should be fixed.
The CSS Working Group just discussed [cssom] How safe is it really to shorthandify properties?
.
Okay, so if I'm correctly re-reading the above, whenever you shorthandify a property, it changes behavior in the following two back-compat-relevant ways:
The longer a property has been not a shorthand, the more likely it is that one or both of the above will break in deployed code, thus making it incompatible to shorthand the property. (See, for example, https://g-issues.chromium.org/issues/40269890 reporting that (1) caused breakage for a site due to white-space
becoming a shorthand.)
Chrome is, in fact, doing something custom for white-space
and text-wrap
rather than shorthanding normally, to avoid some of these issues. (I'm not sure of the exact details; Koji or Ian would know better.) We're fairly certain we'll have similar problems with position
(from #10321).
We have a big set of existing shorthands that we also can't change the behavior of - they need to not enumerate, and not show up in transitions.
So, the simplest answer is just: we define, for a small set of compat-required new shorthands, that they do enumerate and do show up in transition JS objects, despite being shorthands.
Enumeration ordering is likely important to preserve. Alphabetical ordering will usually automatically place the shorthand before its longhands, but when that isn't the case, we'll have to explicitly specify a break from alphabetical ordering to maintain the "shorthand comes first" property. Assuming the shorthand is the legacy property, we'd leave it where it always was and say that the new longhands, which would otherwise precede it, instead come right after it.
Open question: is this a behavior we want to apply to all shorthands going forward (and thus we require an explicit, fairly large, list of grandfathered shorthands that don't serialize)? Or do we want to let new stuff stay consistent with old stuff, and only invoke this special behavior when we're doing a compat-constrained "upgrade" shorthandification? I suspect we want the latter; it's far fewer cases overall, and the existing behavior is well-established across dozens of shorthand properties.
Is there anything else we'd need to define for these "upgraded" properties? Or is that it?
How does the indexing work if you have both the shorthand and its longhands? For example:
var {style} = document.createElement("div");
style.cssText = "white-space: pre";
[...style]; // Presumably [ "white-space", "white-space-collapse", "text-wrap-mode", "text-wrap-style" ] ?
s.removeProperty("text-wrap-style");
[...style]; // Maybe [ "white-space-collapse", "text-wrap-mode" ] ???
// Or [ "white-space", "white-space-collapse", "text-wrap-mode" ] ???
s.removeProperty("white-space-collapse");
s.removeProperty("text-wrap-mode");
[...style]; // Hopefully not [ "white-space" ] ??????
It seems this would need extra logic in setProperty
and removeProperty
to automatically insert or remove a shorthand if necessary (maybe when all longhands are present, or when any longhand is present). Need to ensure this doesn't cause perf regressions.
I'm also concerned that the proposal might affect code doing something along these lines:
for (let i = style.length - 1; i >= 0; i -= 1) {
style.removeProperty(style[i]);
}
If removing longhands can remove a preceding shorthands, this will mess the indices, making style[i]
go out-of-bounds, or if only some properties are removed, possibly revisit an already iterated one. The specific code above won't throw nor anything, but if the author is doing additional things with style[i]
, it could be bad.
@Loirooriol this particular code snippet would also be broken even with only longhands. steelmanning the argument: the problem is that having both shorthands and longhands would make it extremely hard to "do the right thing" in a loop.
But one could argue that removing things in indexed loops is always a bit cursed.
Yeah, if you were doing that you'd have to index in reverse for safety, but the shorthand disappearing would still mess things up.
@fserb How is it broken with only longhands? Iterating backwards should be fine, currently.
The CSS Working Group just discussed [cssom] How safe is it really to shorthandify properties?
.
Overall, the Chrome team's consensus position is that shorthandifying well-established properties after the fact is risky, has significant implementation and developer-churn/confusion cost, and generally doesn't provide enough developer benefit to justify this risk, churn and cost. So we're opposed to doing that generally.
Therefore, I suggest we resolve the issue (8398) with this design guidance: "don't shorthandify well-established CSS properties".
(Aside: in addition, Tab commented here about additional reasons beyond the above why we should't shorthandify position
for anchor positioning. However, his arguments have some relationship to my points above, since they are an example of how shorthands can be more confusing than the alternative.)
The CSS Working Group just discussed [cssom] How safe is it really to shorthandify properties?
.
The proposed resolution the minutes are referring to doesn't seem to have been captured but strong -1 to what @chrishtr proposed above. Shorthandifying properties are one of the very few paths we have to evolve CSS without balooning its number of primitives. Unless we come up with an alternate path (e.g. actually deprecating properties with the intent to later remove), I don’t think it’s a good idea to stop doing this.
If the issue is the CSS OM inconsistencies, let’s fix the CSS OM, and add introspection so that authors can read a data structure about what is a shorthand of what.
Not sure this is relevant, but WebKit has logic to enumerate all longhands instead of just enumerating the shorthand: https://searchfox.org/wubkat/rev/b36cbce69fddb7da33823f316bd8ead5bebee970/Source/WebCore/css/StyleProperties.cpp#201
Apparently this behavior is already defined: https://www.w3.org/TR/css-cascade-5/#legacy-shorthand
Can't we reuse this for whatever new shorthands we introduce?
I'm trying to understand how risky it is to change an already-established longhand into a shorthand. I'm opening this issue against cssom, because it's the only spec I can think of (at the moment) which makes problematic distinctions on longhand/shorthand.
Example:
white-space
is a longhand in css-text-3, but a shorthand in css-text-4 which expands totext-space-collapse
,text-wrap
,text-space-trim
.This is observable in that e.g.
Array.from(getComputedStyle(e))
will no longer containwhite-space
.Are there other problems?
So the only real problem I see is when enumerating things in cssom. It does not seem easy to create a use-counter for this or otherwise investigate the impact of such a change. At the same time, being able to make a longhand into a shorthand seems quite important for future spec development, so the question is: should we do something to make the longhand->shorthand transition (near-)zero-risk? For example, include shorthands in the enumeration? (Or perhaps just those that have been converted).
@emilio @Loirooriol @tabatkins (can you think of other places where the conversion to shorthand is problematic?)