Bug 1527392 - Do not clamp computed width and height by min-/max- values. r=mats

The spec says that when there's no box or the property doesn't apply, the
computed value should be returned.

That's not what we're doing right now, we're clamping by min-/max- values, which
is wrong.

This patch makes us match other engines and the spec, and it's an attempt to get
interop on resolved values in:

  https://github.com/w3c/csswg-drafts/issues/3678

WebKit fails the WPT test, but due to a different reason:

  https://bugs.webkit.org/show_bug.cgi?id=197814

Differential Revision: https://phabricator.services.mozilla.com/D30780

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Emilio Cobos Álvarez 2019-05-11 18:01:50 +00:00
Родитель 02db1205c0
Коммит 8308eb7ea1
6 изменённых файлов: 60 добавлений и 54 удалений

Просмотреть файл

@ -15,7 +15,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=522632
<table border="1">
<tr>
<td style="height: 200px; padding: 0; margin: 0; border: none">
<span style="height: 0; min-height: 50%" id="foo"></span>
<div style="height: 0; min-height: 50%" id="foo"></div>
</td>
<tr>
</table>

Просмотреть файл

@ -1842,17 +1842,7 @@ already_AddRefed<CSSValue> nsComputedDOMStyle::DoGetHeight() {
val->SetAppUnits(mInnerFrame->GetContentRect().height +
adjustedValues.TopBottom());
} else {
const nsStylePosition* positionData = StylePosition();
nscoord minHeight =
StyleCoordToNSCoord(positionData->mMinHeight,
&nsComputedDOMStyle::GetCBContentHeight, 0, true);
nscoord maxHeight = StyleCoordToNSCoord(
positionData->mMaxHeight, &nsComputedDOMStyle::GetCBContentHeight,
nscoord_MAX, true);
SetValueToSize(val, positionData->mHeight, minHeight, maxHeight);
SetValueToSize(val, StylePosition()->mHeight);
}
return val.forget();
@ -1879,17 +1869,7 @@ already_AddRefed<CSSValue> nsComputedDOMStyle::DoGetWidth() {
val->SetAppUnits(mInnerFrame->GetContentRect().width +
adjustedValues.LeftRight());
} else {
const nsStylePosition* positionData = StylePosition();
nscoord minWidth =
StyleCoordToNSCoord(positionData->mMinWidth,
&nsComputedDOMStyle::GetCBContentWidth, 0, true);
nscoord maxWidth = StyleCoordToNSCoord(
positionData->mMaxWidth, &nsComputedDOMStyle::GetCBContentWidth,
nscoord_MAX, true);
SetValueToSize(val, positionData->mWidth, minWidth, maxWidth);
SetValueToSize(val, StylePosition()->mWidth);
}
return val.forget();
@ -2219,8 +2199,7 @@ void nsComputedDOMStyle::SetValueToExtremumLength(nsROCSSPrimitiveValue* aValue,
}
void nsComputedDOMStyle::SetValueToSize(nsROCSSPrimitiveValue* aValue,
const StyleSize& aSize, nscoord aMin,
nscoord aMax) {
const StyleSize& aSize) {
if (aSize.IsAuto()) {
return aValue->SetIdent(eCSSKeyword_auto);
}
@ -2228,8 +2207,7 @@ void nsComputedDOMStyle::SetValueToSize(nsROCSSPrimitiveValue* aValue,
return SetValueToExtremumLength(aValue, aSize.AsExtremumLength());
}
MOZ_ASSERT(aSize.IsLengthPercentage());
SetValueToLengthPercentage(aValue, aSize.AsLengthPercentage(), true, aMin,
aMax);
SetValueToLengthPercentage(aValue, aSize.AsLengthPercentage(), true);
}
void nsComputedDOMStyle::SetValueToMaxSize(nsROCSSPrimitiveValue* aValue,
@ -2256,14 +2234,13 @@ void nsComputedDOMStyle::SetValueToLengthPercentageOrAuto(
void nsComputedDOMStyle::SetValueToLengthPercentage(
nsROCSSPrimitiveValue* aValue, const mozilla::LengthPercentage& aLength,
bool aClampNegativeCalc, nscoord aMin, nscoord aMax) {
bool aClampNegativeCalc) {
if (aLength.ConvertsToLength()) {
nscoord result = aLength.ToLength();
if (aClampNegativeCalc) {
result = std::max(result, 0);
}
// TODO(emilio, bug 1527392): It's wrong to clamp here.
return aValue->SetAppUnits(std::max(aMin, std::min(result, aMax)));
return aValue->SetAppUnits(result);
}
if (aLength.ConvertsToPercentage()) {
float result = aLength.ToPercentage();
@ -2294,7 +2271,7 @@ void nsComputedDOMStyle::SetValueToLengthPercentage(
void nsComputedDOMStyle::SetValueToCoord(
nsROCSSPrimitiveValue* aValue, const nsStyleCoord& aCoord,
bool aClampNegativeCalc, PercentageBaseGetter aPercentageBaseGetter,
const KTableEntry aTable[], nscoord aMinAppUnits, nscoord aMaxAppUnits) {
const KTableEntry aTable[]) {
MOZ_ASSERT(aValue, "Must have a value to work with");
switch (aCoord.GetUnit()) {
@ -2312,8 +2289,7 @@ void nsComputedDOMStyle::SetValueToCoord(
(this->*aPercentageBaseGetter)(percentageBase)) {
nscoord val =
NSCoordSaturatingMultiply(percentageBase, aCoord.GetPercentValue());
aValue->SetAppUnits(
std::max(aMinAppUnits, std::min(val, aMaxAppUnits)));
aValue->SetAppUnits(val);
} else {
aValue->SetPercent(aCoord.GetPercentValue());
}
@ -2325,7 +2301,7 @@ void nsComputedDOMStyle::SetValueToCoord(
case eStyleUnit_Coord: {
nscoord val = aCoord.GetCoordValue();
aValue->SetAppUnits(std::max(aMinAppUnits, std::min(val, aMaxAppUnits)));
aValue->SetAppUnits(val);
} break;
case eStyleUnit_Integer:
@ -2350,8 +2326,7 @@ void nsComputedDOMStyle::SetValueToCoord(
MOZ_ASSERT(aCoord.IsCalcUnit(), "parser should have rejected value");
val = 0;
}
aValue->SetAppUnits(
std::max(aMinAppUnits, std::min(val, aMaxAppUnits)));
aValue->SetAppUnits(val);
} else if (aPercentageBaseGetter &&
(this->*aPercentageBaseGetter)(percentageBase)) {
nscoord val = aCoord.ComputeCoordPercentCalc(percentageBase);
@ -2359,8 +2334,7 @@ void nsComputedDOMStyle::SetValueToCoord(
MOZ_ASSERT(aCoord.IsCalcUnit(), "parser should have rejected value");
val = 0;
}
aValue->SetAppUnits(
std::max(aMinAppUnits, std::min(val, aMaxAppUnits)));
aValue->SetAppUnits(val);
} else {
nsStyleCoord::Calc* calc = aCoord.GetCalcValue();
SetValueToCalc(calc, aValue);

Просмотреть файл

@ -295,9 +295,7 @@ class nsComputedDOMStyle final : public nsDOMCSSDeclaration,
void SetValueToURLValue(const mozilla::css::URLValue* aURL,
nsROCSSPrimitiveValue* aValue);
void SetValueToSize(nsROCSSPrimitiveValue* aValue, const mozilla::StyleSize&,
nscoord aMinAppUnits = nscoord_MIN,
nscoord aMaxAppUnits = nscoord_MAX);
void SetValueToSize(nsROCSSPrimitiveValue* aValue, const mozilla::StyleSize&);
void SetValueToLengthPercentageOrAuto(nsROCSSPrimitiveValue* aValue,
const LengthPercentageOrAuto&,
@ -305,9 +303,7 @@ class nsComputedDOMStyle final : public nsDOMCSSDeclaration,
void SetValueToLengthPercentage(nsROCSSPrimitiveValue* aValue,
const LengthPercentage&,
bool aClampNegativeCalc,
nscoord aMinAppUnits = nscoord_MIN,
nscoord aMaxAppUnits = nscoord_MAX);
bool aClampNegativeCalc);
void SetValueToMaxSize(nsROCSSPrimitiveValue* aValue, const StyleMaxSize&);
@ -331,9 +327,7 @@ class nsComputedDOMStyle final : public nsDOMCSSDeclaration,
void SetValueToCoord(nsROCSSPrimitiveValue* aValue,
const nsStyleCoord& aCoord, bool aClampNegativeCalc,
PercentageBaseGetter aPercentageBaseGetter = nullptr,
const KTableEntry aTable[] = nullptr,
nscoord aMinAppUnits = nscoord_MIN,
nscoord aMaxAppUnits = nscoord_MAX);
const KTableEntry aTable[] = nullptr);
/**
* If aCoord is a eStyleUnit_Coord returns the nscoord. If it's

Просмотреть файл

@ -236,9 +236,9 @@ doATest("width", "minmaxwidth1-", 320, 0, true);
doATest("width", "minmaxwidth2-", 320, 0, true);
doATest("width", "minmaxwidth3-", 600, 0, true);
doATest("width", "minmaxwidth4-", 600, 0, true);
doATest("width", "minmaxwidth5-", 320, 0, true);
doATest("width", "minmaxwidth6-", 320, 0, true);
doATest("width", "minmaxwidth7-", 600, 0, true);
doATest("width", "minmaxwidth5-", 400, 0, true);
doATest("width", "minmaxwidth6-", 400, 0, true);
doATest("width", "minmaxwidth7-", 400, 0, true);
doATest("width", "minmaxwidth8-", 320, 0, true);
doATest("width", "minmaxwidth9-", 320, 0, true);
doATest("width", "minmaxwidth10-", 600, 0, true);
@ -268,9 +268,9 @@ doATest("height", "minmaxheight1-", 320, 0, true);
doATest("height", "minmaxheight2-", 320, 0, true);
doATest("height", "minmaxheight3-", 600, 0, true);
doATest("height", "minmaxheight4-", 600, 0, true);
doATest("height", "minmaxheight5-", 320, 0, true);
doATest("height", "minmaxheight6-", 320, 0, true);
doATest("height", "minmaxheight7-", 600, 0, true);
doATest("height", "minmaxheight5-", 400, 0, true);
doATest("height", "minmaxheight6-", 400, 0, true);
doATest("height", "minmaxheight7-", 400, 0, true);
doATest("height", "minmaxheight8-", 200, 0, true);
doATest("height", "minmaxheight9-", 200, 0, true);
doATest("height", "minmaxheight10-", 600, 0, true);

Просмотреть файл

@ -13,7 +13,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=391221
<p id="display">
<div id="width-ref" style="width: 2ch"></div>
</p>
<div id="content" style="display: none">
<div id="content">
<div id="one" style="width: 1000px; max-width: 2ch"></div>
<div id="two" style="width: 0px; min-width: 2ch"></div>

Просмотреть файл

@ -0,0 +1,38 @@
<!doctype html>
<meta charset="utf-8">
<title>CSSOM: resolved values of the width and height when the element generates no box or are a non-replaced inline aren't clamped by min-width / max-width</title>
<link rel="help" href="https://drafts.csswg.org/cssom/#resolved-value">
<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
<link rel="author" title="Mozilla" href="https://mozilla.org">
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>
<span id="non-replaced-inline"></span>
<div id="display-none" style="display: none"></div>
<div id="display-contents" style="display: contents"></div>
<script>
test(function() {
for (const e of document.querySelectorAll("[id]")) {
const cs = getComputedStyle(e);
for (const prop of ["width", "height"]) {
e.style.setProperty("min-" + prop, "10px");
e.style.setProperty("max-" + prop, "50px");
e.style.setProperty(prop, "10%");
assert_equals(cs[prop], "10%", `${e.id}: ${prop} with percentages returns percentages`);
e.style.setProperty(prop, "15px");
assert_equals(cs[prop], "15px", `${e.id}: ${prop} with value in range returns computed value`);
e.style.setProperty(prop, "1px");
assert_equals(cs[prop], "1px", `${e.id}: ${prop} with value out of range isn't clamped by min-${prop}`);
e.style.setProperty(prop, "60px");
assert_equals(cs[prop], "60px", `${e.id}: ${prop} with value out of range isn't clamped by max-${prop}`);
e.style.removeProperty(prop);
e.style.removeProperty("min-" + prop);
e.style.removeProperty("max-" + prop);
}
}
}, "Resolved value of width / height when there's no used value isn't clamped by min/max properties");
</script>