[css-zoom?] Zoom and inheritance. #9397
Comments
|
Chrome 116 (M1 Mac) reports the following computed styles of this element: |
|
To confirm this is not line-height specific I tried inheriting border-left-width explicitly, which also ends up with not applying zoom in Chrome. |
|
Implicit inheritance of line-height also doesn't work. |
|
And getComputedStyle() un-zooms based on the effective zoom which means it returns 8px for the inherited line-height/width etc. in the case above. I'm not too concerned about fixing this for explicit inheritance of non-inherited properties in Blink, but adjusting inherited properties when the effective zoom changes will probably be expensive. Hopefully, changing the zoom property won't happen on many elements. |
|
Yeah, my preference would be not to adjust on any inheritance (so, removing the font-size special-case). |
|
I don't think we have evidence that changes of zoom are common or performance-sensitive, so I am not worried about performance. I don't know of any use cases for zoom that need a super fast relayout on change of zoom. On the contrary, the use cases I know of are all about getting responsive design right, which I think is more important than performance in this case. I am, however, worried about web compat and usefulness of this API. From that perspective, applying zoom to all inherited lengths (and not just font size) that are not percentages is consistent and makes the most sense. |
|
Note that "changes of zoom" here doesn't mean just dynamic changes. Any change of zoom from ancestor to descendant would incur that extra performance cost. So |
A lot of extra work due to processing inherited lengths? I don't get how, could you explain? In that situation it seems you'd have to multiply all the lengths by some numbers regardless. (also, |
|
Right, so the work is the same as increasing the font-size, but times the amount of inherited length properties, right? Given engines rely on heavy sharing of inherited computed style data in order to have somewhat reasonable memory usage, it seems problematic, though I haven't checked by hand the number of affected properties. |
|
But yeah I guess changing zoom to something != 1 might be uncommon enough in practice. Still I've seen pages with hundreds of individual icons zoomed in/out. |
|
Another thing I haven't thought in depth about is custom properties. Presumably such adjustment would have to be done for typed inherited custom properties, but not for untyped ones (which are more common). I think that would be rather confusing for authors tho... |
Yes. But is that big deal? Also, can't the browsers optimize it by computing it once and passing along a cached value to descendants that also inherit?
Wouldn't sharing still be possible, because descendants that don't change font-size or other lengths can reuse each others' values through caching? |
|
I also don't quite understand how zoom would work at all without this, even without any elements specifying |
Not quite sure I follow. Consider the case of a bunch of leaf elements (images) that have a zoom value. I've seen this in the wild. The problem has nothing to do with descendants, but for each of those images now you need to do a significantly larger amount of work.
The problem is not inheriting to further descendants IMO, it's generally breaking the parent-ancestor sharing at zoom boundaries too. Again in the case above, none of those images would share memory for the relevant data structures if we did this. |
|
The CSS Working Group just discussed
The full IRC log of that discussion<fantasai> emilio: This one is tricky<fantasai> emilio: mostly due to the way zoom implemented, inheritance behaves really oddly <fantasai> emilio: webkit and blink have a special case, if you apply zoom to an element <fantasai> emilio: they zoom inherited font-size but not any other inherited property <fantasai> emilio: seems the consensus is either zoom all the inherited lengths or we don't zoom any of them <fantasai> emilio: I think it's clear that ideal behavior is zooming all inherited lengths <fantasai> emilio: but I am worried, especially about perf cost, when you have lots of tiny elements with zoom <fantasai> emilio: which I have seen in the wild while working on this <Rossen_> q? <fantasai> fantasai: [example of p { font-size: 10px; zoom: 2; } ] <Rossen_> ack fantasai <fantasai> fantasai: if inheriting unzoomed font-size, as soon as I put a SPAN in the paragraph, it will have 10px font-size <fantasai> emilio: that works due to UA magic <fantasai> emilio: what you inherit is 20px <fantasai> emilio: if you have a span inside that has zoom as well <fantasai> emilio: that will get font-size of 20px, instead of 40px as you expect <fantasai> emilio: right now WebKit and Blink will zoom font-size correctly, but not e.g. line-height <fantasai> emilio: that's messed up <chrishtr> q+ <fantasai> emilio: so it's edge-casy but... on the other hand it's unfortunate that it doesn't work <fantasai> emilio: I'd rather remove the font-size special case <fantasai> emilio: no property does magic inherited value <fantasai> emilio: I'd be fine with zooming every inherited length, just concerned about performance <fantasai> Rossen_: Not understanding perf issue, let's make sure we have at least the right user-expected behavior before considering potential perf <fantasai> Rossen_: prefer to measure and then discuss <fantasai> Rossen_: I am unclear with your proposal what would happen to 'em' resolution <fantasai> emilio: it has no effect from existing behavior <fantasai> Rossen_: so if I have a 1em height, it'll be 40px or 20px or 10px? <fantasai> emilio: it would be whatever the font-size is <fantasai> Rossen_: what do we want to have, from user PoV <fantasai> emilio: relative lengths reflect final zoomed font-size <fantasai> Rossen_: I didn't want to make font-size exceptional here <fantasai> emilio: that's my proposal <Rossen_> ack chrishtr <fantasai> chrishtr: I feel like from the user point of view, making it apply to all inherited lengths makes sense <fantasai> chrishtr: so I do think I would recommend resolving to do all inherited value <fantasai> chrishtr: if during implementation we come up with an unaviodable perf problem, can discuss again <fantasai> emilio: I'm ok with that <fantasai> emilio: can we resolve in any case, don't make font-size special <fantasai> RESOLVED: font-size is not special wrt zoom <fantasai> emilio: other issue with chrishtr's proposal to zoom all computed values is <fantasai> emilio: you need to figure out what to do with all those, whether it's fine if those px values came from relative lengths <fantasai> emilio: assuming fine, then seems fine <dholbert> fantasai (IRC): relative lengths are all absolutized before they're inherited <dholbert> fantasai (IRC): if your font size changes and you wanted ems, then you wanted ems and they'll get converted to absolute size <dholbert> emilio (IRC): I think it's ok <dholbert> ScribeNick: dholbert <Rossen_> ack fantasai <fantasai> PROPOSED: All computed lengths are zoomed when inheriting <dholbert> fantasai (IRC): more precisely, when you absolutize a length to make a computed value, you apply a zoom. You have to undo that for getComputedStyle <dholbert> fantasai (IRC): this is needed for animation to work <dholbert> emilio (IRC): that's how impls work now <dholbert> emilio (IRC): tricky thing is when you're inheriting into a thing with a different zoom <dholbert> emilio (IRC): then you apply the zoom of the thing you're inheriting to on top, and that's this resolution <dholbert> fantasai (IRC): that seems like the right thing to do <dholbert> emilio (IRC): i agree from a user perspective. skeptical that we can make it fast but happy to try <dholbert> chrishtr (IRC): proposed resln is all computed lengths, but we should exclude percentages <vmpstr> q+ <dholbert> emilio (IRC): all computed absolute lengths, let's say <Rossen_> ack vmpstr <dholbert> vmpstr (IRC): concretely, if I have width:100px and zoom:2 and a child that inherits that width, it would inherit 200px? <dholbert> emilio (IRC): that already happens <dholbert> emilio (IRC): the issue is when zoom is specified on the child. then what's the width of the child <dholbert> emilio (IRC): with this resolution, it would be 200 <chrishtr> thanks! <dholbert> RESOLVED: All computed absolute lengths are zoomed when inheriting <chrishtr> I don't see the resolution recorded in the issue yet? |
Based on CL:5302760 by chrishtr@.
In Blink, lengths are stored on the ComputedStyle with "premultiplied
zoom", meaning that for e.g. width:20px;zoom:200%, the width is stored
as "40". Currently, we do not account for this during inheritance:
we inherit that "40" as-is, even if the effective zoom of the element
we're inheriting into is not the same. Issue 9397 [1] clarified that
this is not correct, and therefore we need to adjust our zoomed values
when inheriting.
This CL solves this problem by using our existing code paths
for creating the computed CSSValue (which already does unzooming)
and for applying that CSSValue onto a ComputedStyle (which already
does zooming).
We need two things for this to work:
1. Special behavior in Longhand::ApplyInherit, which detects if
the effective zoom changed, and if so, inherits via the computed
CSSValue path instead of the regular ComputedStyle-to-ComputedStyle
path. However, Longhand::ApplyInherit is only reached if an explicit
inherit/unset exists in the cascade, but we also need to make the
adjustment for *inherited* properties (e.g. line-height).
Therefore:
2. During cascade expansion, if the effective zoom changes vs. the
parent zoom, we insert explicit 'unset' values at the bottom
of the cascade. This ensures that we always reach ApplyInherit
for affected properties.
This CL only enables the zoom adjustment for one inherited property
(line-height), and one non-inherited property (width) to establish
the pattern. Future CLs will incrementally add the 'affected_by_zoom'
flag to the relevant properties.
Bug: 40946858
[1] w3c/csswg-drafts#9397
Change-Id: Iab4dd978143e56264a5c78377055ecbb0363b276
Based on CL:5302760 by chrishtr@.
In Blink, lengths are stored on the ComputedStyle with "premultiplied
zoom", meaning that for e.g. width:20px;zoom:200%, the width is stored
as "40". Currently, we do not account for this during inheritance:
we inherit that "40" as-is, even if the effective zoom of the element
we're inheriting into is not the same. Issue 9397 [1] clarified that
this is not correct, and therefore we need to adjust our zoomed values
when inheriting.
This CL solves this problem by using our existing code paths
for creating the computed CSSValue (which already does unzooming)
and for applying that CSSValue onto a ComputedStyle (which already
does zooming).
We need two things for this to work:
1. Special behavior in Longhand::ApplyInherit, which detects if
the effective zoom changed, and if so, inherits via the computed
CSSValue path instead of the regular ComputedStyle-to-ComputedStyle
path. However, Longhand::ApplyInherit is only reached if an explicit
inherit/unset exists in the cascade, but we also need to make the
adjustment for *inherited* properties (e.g. line-height).
Therefore:
2. During cascade expansion, if the effective zoom changes vs. the
parent zoom, we insert explicit 'unset' values at the bottom
of the cascade. This ensures that we always reach ApplyInherit
for affected properties.
This CL only enables the zoom adjustment for one inherited property
(line-height), and one non-inherited property (width) to establish
the pattern. Future CLs will incrementally add the 'affected_by_zoom'
flag to the relevant properties.
Bug: 40946858
[1] w3c/csswg-drafts#9397
Change-Id: Iab4dd978143e56264a5c78377055ecbb0363b276
Based on CL:5302760 by chrishtr@.
In Blink, lengths are stored on the ComputedStyle with "premultiplied
zoom", meaning that for e.g. width:20px;zoom:200%, the width is stored
as "40". Currently, we do not account for this during inheritance:
we inherit that "40" as-is, even if the effective zoom of the element
we're inheriting into is not the same. Issue 9397 [1] clarified that
this is not correct, and therefore we need to adjust our zoomed values
when inheriting.
This CL solves this problem by using our existing code paths
for creating the computed CSSValue (which already does unzooming)
and for applying that CSSValue onto a ComputedStyle (which already
does zooming).
We need two things for this to work:
1. Special behavior in Longhand::ApplyInherit, which detects if
the effective zoom changed, and if so, inherits via the computed
CSSValue path instead of the regular ComputedStyle-to-ComputedStyle
path. However, Longhand::ApplyInherit is only reached if an explicit
inherit/unset exists in the cascade, but we also need to make the
adjustment for *inherited* properties (e.g. line-height).
Therefore:
2. During cascade expansion, if the effective zoom changes vs. the
parent zoom, we insert explicit 'unset' values at the bottom
of the cascade. This ensures that we always reach ApplyInherit
for affected properties.
This CL only enables the zoom adjustment for one inherited property
(line-height), and one non-inherited property (width) to establish
the pattern. Future CLs will incrementally add the 'affected_by_zoom'
flag to the relevant properties.
Bug: 40946858
[1] w3c/csswg-drafts#9397
Change-Id: Iab4dd978143e56264a5c78377055ecbb0363b276
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5323090
Commit-Queue: Chris Harrelson <[email protected]>
Reviewed-by: Rune Lillesveen <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1267277}
Based on CL:5302760 by chrishtr@.
In Blink, lengths are stored on the ComputedStyle with "premultiplied
zoom", meaning that for e.g. width:20px;zoom:200%, the width is stored
as "40". Currently, we do not account for this during inheritance:
we inherit that "40" as-is, even if the effective zoom of the element
we're inheriting into is not the same. Issue 9397 [1] clarified that
this is not correct, and therefore we need to adjust our zoomed values
when inheriting.
This CL solves this problem by using our existing code paths
for creating the computed CSSValue (which already does unzooming)
and for applying that CSSValue onto a ComputedStyle (which already
does zooming).
We need two things for this to work:
1. Special behavior in Longhand::ApplyInherit, which detects if
the effective zoom changed, and if so, inherits via the computed
CSSValue path instead of the regular ComputedStyle-to-ComputedStyle
path. However, Longhand::ApplyInherit is only reached if an explicit
inherit/unset exists in the cascade, but we also need to make the
adjustment for *inherited* properties (e.g. line-height).
Therefore:
2. During cascade expansion, if the effective zoom changes vs. the
parent zoom, we insert explicit 'unset' values at the bottom
of the cascade. This ensures that we always reach ApplyInherit
for affected properties.
This CL only enables the zoom adjustment for one inherited property
(line-height), and one non-inherited property (width) to establish
the pattern. Future CLs will incrementally add the 'affected_by_zoom'
flag to the relevant properties.
Bug: 40946858
[1] w3c/csswg-drafts#9397
Change-Id: Iab4dd978143e56264a5c78377055ecbb0363b276
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5323090
Commit-Queue: Chris Harrelson <[email protected]>
Reviewed-by: Rune Lillesveen <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1267277}
Based on CL:5302760 by chrishtr@.
In Blink, lengths are stored on the ComputedStyle with "premultiplied
zoom", meaning that for e.g. width:20px;zoom:200%, the width is stored
as "40". Currently, we do not account for this during inheritance:
we inherit that "40" as-is, even if the effective zoom of the element
we're inheriting into is not the same. Issue 9397 [1] clarified that
this is not correct, and therefore we need to adjust our zoomed values
when inheriting.
This CL solves this problem by using our existing code paths
for creating the computed CSSValue (which already does unzooming)
and for applying that CSSValue onto a ComputedStyle (which already
does zooming).
We need two things for this to work:
1. Special behavior in Longhand::ApplyInherit, which detects if
the effective zoom changed, and if so, inherits via the computed
CSSValue path instead of the regular ComputedStyle-to-ComputedStyle
path. However, Longhand::ApplyInherit is only reached if an explicit
inherit/unset exists in the cascade, but we also need to make the
adjustment for *inherited* properties (e.g. line-height).
Therefore:
2. During cascade expansion, if the effective zoom changes vs. the
parent zoom, we insert explicit 'unset' values at the bottom
of the cascade. This ensures that we always reach ApplyInherit
for affected properties.
This CL only enables the zoom adjustment for one inherited property
(line-height), and one non-inherited property (width) to establish
the pattern. Future CLs will incrementally add the 'affected_by_zoom'
flag to the relevant properties.
Bug: 40946858
[1] w3c/csswg-drafts#9397
Change-Id: Iab4dd978143e56264a5c78377055ecbb0363b276
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5323090
Commit-Queue: Chris Harrelson <[email protected]>
Reviewed-by: Rune Lillesveen <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1267277}
… the effective zoom changes, a=testonly
Automatic update from web-platform-tests
[zoom] Adjust line-height and width when the effective zoom changes
Based on CL:5302760 by chrishtr@.
In Blink, lengths are stored on the ComputedStyle with "premultiplied
zoom", meaning that for e.g. width:20px;zoom:200%, the width is stored
as "40". Currently, we do not account for this during inheritance:
we inherit that "40" as-is, even if the effective zoom of the element
we're inheriting into is not the same. Issue 9397 [1] clarified that
this is not correct, and therefore we need to adjust our zoomed values
when inheriting.
This CL solves this problem by using our existing code paths
for creating the computed CSSValue (which already does unzooming)
and for applying that CSSValue onto a ComputedStyle (which already
does zooming).
We need two things for this to work:
1. Special behavior in Longhand::ApplyInherit, which detects if
the effective zoom changed, and if so, inherits via the computed
CSSValue path instead of the regular ComputedStyle-to-ComputedStyle
path. However, Longhand::ApplyInherit is only reached if an explicit
inherit/unset exists in the cascade, but we also need to make the
adjustment for *inherited* properties (e.g. line-height).
Therefore:
2. During cascade expansion, if the effective zoom changes vs. the
parent zoom, we insert explicit 'unset' values at the bottom
of the cascade. This ensures that we always reach ApplyInherit
for affected properties.
This CL only enables the zoom adjustment for one inherited property
(line-height), and one non-inherited property (width) to establish
the pattern. Future CLs will incrementally add the 'affected_by_zoom'
flag to the relevant properties.
Bug: 40946858
[1] w3c/csswg-drafts#9397
Change-Id: Iab4dd978143e56264a5c78377055ecbb0363b276
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5323090
Commit-Queue: Chris Harrelson <[email protected]>
Reviewed-by: Rune Lillesveen <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1267277}
--
wpt-commits: 87e7f1cf0f39c0ec2ce195cce8d637fbd6d25ea0
wpt-pr: 44808
Zoom has some interesting behavior that makes no sense to me (live):
That for some reason ends up with:
line-heightof16pxfont-sizeof32pxwidthof100pxheightof200pxThat just makes no sense. This happens because the way
zoomis implemented in browsers is by mutating the computed value, so inheritance doesn't account for zoom since it copies the un-zoomed computed value.But then somehow
font-size(but notline-height) is special, and gets recomputed on zoom changes??cc: #5623 @chrishtr @lilles @smfr @tabatkins @atanassov
The text was updated successfully, but these errors were encountered: