Summary: | [cssom] Some longhands indexed in computed style are not serialized | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Oriol Brufau <obrufau> | ||||||||
Component: | CSS | Assignee: | Oriol Brufau <obrufau> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | clopez, darin, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, jbedard, joepeck, koivisto, macpherson, menard, ntim, simon.fraser, webkit-bug-importer, youennf | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
See Also: | https://github.com/web-platform-tests/wpt/pull/33899 | ||||||||||
Attachments: |
|
Description
Oriol Brufau
2022-04-22 12:51:29 PDT
Created attachment 458684 [details]
Patch
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess Created attachment 458700 [details]
Patch
Is this good, or should I instead exclude these longhands from being indexed in the computed style? Comment on attachment 458700 [details]
Patch
My issue with this is that perspective-origin-x/y and transform-origin-x/y/z aren't standard. Can we try to unship those (at least the unprefixed versions)?
I'm afraid of compat risks of completely unshipping them. But the option of not indexing them in computed styles would be closer to that. (In reply to Oriol Brufau from comment #7) > I'm afraid of compat risks of completely unshipping them. > But the option of not indexing them in computed styles would be closer to > that. Chrome ships the prefixed versions, but not the unprefixed ones. Seems like we could do a similar thing? For this bug specifically, perspective-origin should be indexed, but not the other 2 longhands. Created attachment 458713 [details]
Patch
(In reply to Tim Nguyen (:ntim) from comment #8) > (In reply to Oriol Brufau from comment #7) > > I'm afraid of compat risks of completely unshipping them. > > But the option of not indexing them in computed styles would be closer to > > that. > > Chrome ships the prefixed versions, but not the unprefixed ones. Seems like > we could do a similar thing? Chrome treats both perspective-origin and -webkit-perspective-origin-{x,y} as longhands that share a computed value. But that's so broken for revert-layer: https://bugs.chromium.org/p/chromium/issues/detail?id=1319414 So I prefer not to do that. > For this bug specifically, perspective-origin should be indexed, but not the > other 2 longhands. I have attached a new patch that does that, without unshipping the non-standard longhands. I don’t mean to override Tim’s judgement here. May want to hear from him before landing. To summarize the problem with unshipping perspective-origin-x/y while keeping -webkit-perspective-origin-x/y: a) If perspective-origin becomes a longhand, we have a problem with revert-layer (same as Blink). b) If perspective-origin becomes a shorthand of -webkit-perspective-origin-x/y, then we have the same problem as now: i) If -webkit-perspective-origin-x/y are indexed in computed styles instead of perspective-origin, then they should serialize. ii) Otherwise -webkit-perspective-origin-x/y should be skipped, and index perspective-origin instead. Option a) seems undesirable. And option b) seems orthogonal: whatever needs to be done with -webkit-perspective-origin-x/y when unshipping perspective-origin-x/y is the same that needs to be done to perspective-origin-x/y when not unshipping them. So I would land the patch as-is. Option b) can be considered in another bug. (In reply to Darin Adler from comment #11) > I don’t mean to override Tim’s judgement here. May want to hear from him > before landing. The current patch is fine as it is. I'll file a new bug to unship the unprefixed non-standard longhands. Committed r294002 (250438@main): <https://commits.webkit.org/250438@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 458713 [details]. |