Skip to content

Commit bb9006d

Browse files
dholbertdholbert
committed
Bug 1956578: Preserve the min-width/min-height 'auto' value in getComputedStyle, for elements with non-default 'aspect-ratio'. r=TYLin
This is the behavior that was resolved on in w3c/csswg-drafts#11716 I'm including a WPT that exercises a bunch of different cases where 'auto' is or isn't preserved in getComputedStyle. Differential Revision: https://phabricator.services.mozilla.com/D243234
1 parent 58dd687 commit bb9006d

3 files changed

Lines changed: 145 additions & 8 deletions

File tree

‎layout/generic/nsImageFrame.cpp‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,11 @@ using namespace mozilla::layers;
107107

108108
using mozilla::layout::TextDrawTarget;
109109

110+
#define WARN_CONTENT(msg) \
111+
if (XRE_IsContentProcess()) { \
112+
NS_WARNING(msg); \
113+
}
114+
110115
static constexpr wr::ImageKey kNoKey{{0}, 0};
111116

112117
class nsDisplayGradient final : public nsPaintedDisplayItem {
@@ -373,6 +378,7 @@ static bool SizeDependsOnIntrinsicSize(const ReflowInput& aReflowInput) {
373378
}
374379

375380
nsIFrame* NS_NewImageFrame(PresShell* aPresShell, ComputedStyle* aStyle) {
381+
WARN_CONTENT("****dholbert New image frame");
376382
return new (aPresShell) nsImageFrame(aStyle, aPresShell->GetPresContext(),
377383
nsImageFrame::Kind::ImageLoadingContent);
378384
}
@@ -843,9 +849,14 @@ static nscoord ListImageDefaultLength(const nsImageFrame& aFrame) {
843849

844850
IntrinsicSize nsImageFrame::ComputeIntrinsicSize(
845851
bool aIgnoreContainment) const {
852+
WARN_CONTENT("***dholbert Entering ComputeIntrinsicSize")
853+
846854
const auto containAxes =
847855
aIgnoreContainment ? ContainSizeAxes(false, false) : GetContainSizeAxes();
848856
if (containAxes.IsBoth()) {
857+
WARN_CONTENT(
858+
"***dholbert Entering Taking early return from "
859+
"ComputeIntrinsicSize due to containAxes.IsBoth!")
849860
return FinishIntrinsicSize(containAxes, IntrinsicSize(0, 0));
850861
}
851862

@@ -937,6 +948,7 @@ bool nsImageFrame::ShouldUseMappedAspectRatio() const {
937948
}
938949

939950
bool nsImageFrame::UpdateIntrinsicSize() {
951+
WARN_CONTENT("****dholbert Entering UpdateIntrinsicSize")
940952
IntrinsicSize oldIntrinsicSize = mIntrinsicSize;
941953
mIntrinsicSize = ComputeIntrinsicSize();
942954
return mIntrinsicSize != oldIntrinsicSize;
@@ -1016,6 +1028,7 @@ AspectRatio nsImageFrame::ComputeIntrinsicRatioForImage(
10161028
}
10171029

10181030
bool nsImageFrame::UpdateIntrinsicRatio() {
1031+
WARN_CONTENT("****dholbert Entering UpdateIntrinsicRatio")
10191032
AspectRatio oldIntrinsicRatio = mIntrinsicRatio;
10201033
mIntrinsicRatio = ComputeIntrinsicRatioForImage(mImage);
10211034
return mIntrinsicRatio != oldIntrinsicRatio;
@@ -1485,18 +1498,27 @@ bool nsImageFrame::IsForMarkerPseudo() const {
14851498
void nsImageFrame::EnsureIntrinsicSizeAndRatio() {
14861499
const auto containAxes = GetContainSizeAxes();
14871500
if (containAxes.IsBoth()) {
1501+
WARN_CONTENT(
1502+
"***dholbert EnsureIntrinsicSizeAndRatio nerfing the "
1503+
"aspect ratio!!!!!")
14881504
// If we have 'contain:size', then we have no intrinsic aspect ratio,
14891505
// and the intrinsic size is determined by contain-intrinsic-size,
14901506
// regardless of what our underlying image may think.
14911507
mIntrinsicSize = FinishIntrinsicSize(containAxes, IntrinsicSize(0, 0));
14921508
mIntrinsicRatio = AspectRatio();
14931509
return;
14941510
}
1511+
WARN_CONTENT(
1512+
"***dholbert EnsureIntrinsicSizeAndRatio taking the "
1513+
"regular codepath.")
14951514

14961515
// If mIntrinsicSize.width and height are 0, then we need to update from the
14971516
// image container. Note that we handle ::marker intrinsic size/ratio in
14981517
// DidSetComputedStyle.
14991518
if (mIntrinsicSize != IntrinsicSize(0, 0) && !IsForMarkerPseudo()) {
1519+
WARN_CONTENT(
1520+
"***dholbert EnsureIntrinsicSizeAndRatio taking early return "
1521+
"for nonzero mIntrinsicSize")
15001522
return;
15011523
}
15021524

‎layout/style/nsComputedDOMStyle.cpp‎

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1803,16 +1803,35 @@ already_AddRefed<CSSValue> nsComputedDOMStyle::DoGetMaxWidth() {
18031803
* getComputedStyle() result for the (default) "min-width: auto" and
18041804
* "min-height: auto" CSS values.
18051805
*
1806-
* As of this writing, the CSS Sizing draft spec says this "auto" value
1807-
* *always* computes to itself. However, for now, we only make it compute to
1808-
* itself for grid and flex items (the containers where "auto" has special
1809-
* significance), because those are the only areas where the CSSWG has actually
1810-
* resolved on this "computes-to-itself" behavior. For elements in other sorts
1811-
* of containers, this function returns false, which will make us resolve
1812-
* "auto" to 0.
1806+
* https://drafts.csswg.org/css-sizing-3/#valdef-width-auto says that
1807+
* "Unless otherwise defined by the relevant layout module ... it resolves to a
1808+
* used value of 0."
1809+
*
1810+
* Exceptions to this are explained in the various 'return true' early-returns
1811+
* here.
18131812
*/
18141813
bool nsComputedDOMStyle::ShouldHonorMinSizeAutoInAxis(PhysicalAxis aAxis) {
1815-
return mOuterFrame && mOuterFrame->IsFlexOrGridItem();
1814+
if (!mOuterFrame) {
1815+
// Per https://drafts.csswg.org/css-sizing/#valdef-width-auto
1816+
// min-{width,height}:auto "resolves to zero when no box is generated".
1817+
return false;
1818+
}
1819+
if (mOuterFrame->IsFlexOrGridItem()) {
1820+
// Flex and grid items have special significance for min-width:auto and
1821+
// min-height:auto, so we faithfully report "auto" from getComputedStyle
1822+
// for those frames:
1823+
return true;
1824+
}
1825+
if (mOuterFrame->StylePosition()->mAspectRatio != StyleAspectRatio::Auto()) {
1826+
// Frames with non-default CSS 'aspect-ratio' have special significance
1827+
// for min-{width,height}:auto as well, so we faithfully report "auto"
1828+
// for them too, per https://github.com/w3c/csswg-drafts/issues/11716
1829+
return true;
1830+
}
1831+
1832+
// If we're not in one of the special cases above, then we return false
1833+
// which means that "auto" will get reported as "0" in getComputedStyle.
1834+
return false;
18161835
}
18171836

18181837
already_AddRefed<CSSValue> nsComputedDOMStyle::DoGetMinHeight() {

‎testing/web-platform/tests/css/cssom/getComputedStyle-resolved-min-size-auto.html‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
<!doctype html>
2+
<meta charset="utf-8">
3+
<title>CSSOM: resolved value of min-width and min-height:auto should be 0 except in certain defined cases</title>
4+
<link rel="help" href="https://drafts.csswg.org/cssom/#resolved-value">
5+
<link rel="help" href="https://drafts.csswg.org/css-sizing-3/#valdef-width-auto">
6+
<link rel="help" href="https://drafts.csswg.org/css-sizing-4/#aspect-ratio">
7+
<link rel="help" href="https://github.com/w3c/csswg-drafts/issues/11716">
8+
<link rel="author" title="Daniel Holbert" href="mailto:dholbert@mozilla.com">
9+
<link rel="author" title="Mozilla" href="https://mozilla.org">
10+
<script src=/resources/testharness.js></script>
11+
<script src=/resources/testharnessreport.js></script>
12+
<!-- OVERVIEW:
13+
* Elements to be tested here must include an 'id'.
14+
* We use a "preservesAuto" attribute to tag the elements
15+
are expected to preserve "auto" in the resolved value of
16+
min-width and min-height.
17+
-->
18+
19+
<!-- The test content is in a scrollable div for cosmetic reasons, so that it
20+
can be inspected but also doesn't push the testharness output off the
21+
bottom of the screen: -->
22+
<div style="overflow: scroll; width: 300px; height: 200px">
23+
<div id="block-box"></div>
24+
<div id="inline-box"></div>
25+
26+
<!-- Elements with a nondefault aspect-ratio preserve 'auto': -->
27+
<div id="block-with-valid-aspect-ratio" style="aspect-ratio: 1/1"
28+
preservesAuto></div>
29+
<div id="block-with-degenerate-aspect-ratio" style="aspect-ratio: 0/1"
30+
preservesAuto></div>
31+
<div id="block-with-two-part-valid-aspect-ratio"
32+
style="aspect-ratio: auto 1/1" preservesAuto></div>
33+
<div id="block-with-two-part-degenerate-aspect-ratio"
34+
style="aspect-ratio: auto 0/1" preservesAuto></div>
35+
36+
<!-- Note: aspect-ratio doesn't apply to inline boxes, but we treat it as
37+
toggling getComputedStyle to return the true 'min-width:auto' value
38+
regardless. -->
39+
<span id="inline-with-valid-aspect-ratio" style="aspect-ratio: 1/1"
40+
preservesAuto></span>
41+
<span id="inline-with-degenerate-aspect-ratio" style="aspect-ratio: 0/1"
42+
preservesAuto></span>
43+
44+
<!-- flex and grid items should preserve 'auto': -->
45+
<div style="display: flex">
46+
<div id="flex-item-row" preservesAuto></div>
47+
</div>
48+
<div style="display: flex; flex-direction: column;">
49+
<div id="flex-item-column" preservesAuto></div>
50+
</div>
51+
<div style="display: grid">
52+
<div id="grid-item" preservesAuto></div>
53+
</div>
54+
55+
<!-- Per https://drafts.csswg.org/css-sizing/#valdef-width-auto
56+
the min-width/min-height:auto value "resolves to zero when no box is
57+
generated" (regardless of aspect-ratio & flex/grid-item special cases)
58+
-->
59+
<div id="display-none" style="display: none"></div>
60+
<div id="display-none-valid-aspect-ratio"
61+
style="display: none; aspect-ratio:2/1"></div>
62+
<div style="display:none">
63+
<div id="display-none-subtree-valid-aspect-ratio"
64+
style="aspect-ratio:2/1"></div>
65+
</div>
66+
<div style="display: none">
67+
<div style="display: flex">
68+
<div id="display-none-subtree-flex-item"></div>
69+
</div>
70+
</div>
71+
<div style="display: none">
72+
<div style="display: grid">
73+
<div id="display-none-subtree-grid-item"></div>
74+
</div>
75+
</div>
76+
</div>
77+
78+
<script>
79+
for (const e of document.querySelectorAll("[id]")) {
80+
test(function() {
81+
const cs = getComputedStyle(e);
82+
const doesPreserveAuto = e.hasAttribute("preservesAuto");
83+
const expectedVal = doesPreserveAuto ? "auto" : "0px";
84+
const expectationMessage = doesPreserveAuto ?
85+
"should round-trip through getComputedStyle" :
86+
`should be converted to ${expectedVal} in getComputedStyle`;
87+
88+
for (const prop of ["min-width", "min-height"]) {
89+
assert_equals(cs[prop], expectedVal, `${e.id}: '${prop}:auto' ${expectationMessage} (as initial value)`);
90+
e.style.setProperty(prop, "auto");
91+
assert_equals(cs[prop], expectedVal, `${e.id}: '${prop}:auto' ${expectationMessage} (as specified value)`);
92+
e.style.removeProperty(prop);
93+
}
94+
}, `Resolved value of min-width & min-height 'auto' keyword behaves as expected on element with id="${e.id}"`);
95+
}
96+
</script>

0 commit comments

Comments
 (0)