Skip to content

Commit

Permalink
Bug 1340715 - Change SVG max-content size for webcompat. r=emilio
Browse files Browse the repository at this point in the history
The goal of this patch is to change the SVG max-content size to match blink and
webkit to fix the webcompat issues in bug 1521882 and bug 1651754.

If a percentage width/height attribute on inline axis is explicitly set, we fall
back to the default replaced element size to avoid regressing bug 1162418.
Otherwise, we use 0 as SVG's max-content size in order to be compatible with
blink and webkit.

When computing replaced elements's max-content inline-size, its preferred
aspect-ratio and block-size can transfer to inline axis. The logic is the same
as if the element has an 'auto' inline-size. Hence the modification in
ComputeSizeWithIntrinsicDimensions().

This patch also removes the SVG's intrinsic size dependency on an arbitrary
container from the previous layout result in `SVGOuterSVGFrame::GetPrefISize()`.
bug 1340715 comment 1 explains why the code is unsound.

Differential Revision: https://phabricator.services.mozilla.com/D155998
  • Loading branch information
aethanyc committed Aug 31, 2022
1 parent 6e0e7b1 commit d660a50
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 33 deletions.
10 changes: 6 additions & 4 deletions layout/generic/nsContainerFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2343,13 +2343,15 @@ LogicalSize nsContainerFrame::ComputeSizeWithIntrinsicDimensions(

// Handle intrinsic sizes and their interaction with
// {min-,max-,}{width,height} according to the rules in
// http://www.w3.org/TR/CSS21/visudet.html#min-max-widths
// https://www.w3.org/TR/CSS22/visudet.html#min-max-widths and
// https://drafts.csswg.org/css-sizing-3/#intrinsic-sizes

// Note: throughout the following section of the function, I avoid
// a * (b / c) because of its reduced accuracy relative to a * b / c
// or (a * b) / c (which are equivalent).

const bool isAutoISize = styleISize.IsAuto();
const bool isAutoOrMaxContentISize =
styleISize.IsAuto() || styleISize.IsMaxContent();
const bool isAutoBSize =
nsLayoutUtils::IsAutoBSize(styleBSize, aCBSize.BSize(aWM));

Expand Down Expand Up @@ -2390,7 +2392,7 @@ LogicalSize nsContainerFrame::ComputeSizeWithIntrinsicDimensions(
const bool hasIntrinsicBSize = bsizeCoord.isSome();
nscoord intrinsicBSize = std::max(0, bsizeCoord.valueOr(0));

if (!isAutoISize) {
if (!isAutoOrMaxContentISize) {
iSize = ComputeISizeValue(aRenderingContext, aWM, aCBSize, boxSizingAdjust,
boxSizingToMarginEdgeISize, styleISize,
aSizeOverrides, aFlags)
Expand Down Expand Up @@ -2510,7 +2512,7 @@ LogicalSize nsContainerFrame::ComputeSizeWithIntrinsicDimensions(
"Our containing block must not have unconstrained inline-size!");

// Now calculate the used values for iSize and bSize:
if (isAutoISize) {
if (isAutoOrMaxContentISize) {
if (isAutoBSize) {
// 'auto' iSize, 'auto' bSize

Expand Down
28 changes: 10 additions & 18 deletions layout/svg/SVGOuterSVGFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,24 +178,16 @@ nscoord SVGOuterSVGFrame::GetPrefISize(gfxContext* aRenderingContext) {
ContainSizeAxesIfApplicable(this).ContainIntrinsicISize(*this)) {
result = *containISize;
} else if (isize.IsPercentage()) {
// It looks like our containing block's isize may depend on our isize. In
// that case our behavior is undefined according to CSS 2.1 section 10.3.2.
// As a last resort, we'll fall back to returning zero.
result = nscoord(0);

// Returning zero may be unhelpful, however, as it leads to unexpected
// disappearance of %-sized SVGs in orthogonal contexts, where our
// containing block wants to shrink-wrap. So let's look for an ancestor
// with non-zero size in this dimension, and use that as a (somewhat
// arbitrary) result instead.
nsIFrame* parent = GetParent();
while (parent) {
nscoord parentISize = parent->GetLogicalSize(wm).ISize(wm);
if (parentISize > 0 && parentISize != NS_UNCONSTRAINEDSIZE) {
result = parentISize;
break;
}
parent = parent->GetParent();
if (isize.IsExplicitlySet()) {
// Our containing block's inline-size depends on our inline-size. In this
// case, return the fallback intrinsic size per
// https://drafts.csswg.org/css-sizing-3/#intrinsic-sizes
result = wm.IsVertical() ? kFallbackIntrinsicSize.height
: kFallbackIntrinsicSize.width;
} else {
// If the width/height attribute in the inline axis is not set, return
// size 0 to match blink and webkit's behavior.
result = nscoord(0);
}
} else {
result = nsPresContext::CSSPixelsToAppUnits(isize.GetAnimValue(svg));
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<!DOCTYPE html>
<meta charset="utf-8">
<link rel="author" title="Daniel Holbert" href="mailto:[email protected]">
<link rel="author" title="Ting-Yu Lin" href="mailto:[email protected]">
<link rel="author" title="Mozilla" href="https://www.mozilla.org/">
<link rel="help" href="https://drafts.csswg.org/css-sizing-3/#intrinsic-sizes">
<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1638937">

<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/check-layout-th.js"></script>

<style>
.block {
display: block;
width: max-content;
border: 2px solid black;
line-height: 0;
}
svg {
background: teal;
}
</style>

<body onload="checkLayout('svg')">
No intrinsic attributes:
<div class="block">
<svg data-expected-client-width="300" data-expected-client-height="150"></svg>
</div>

viewBox and height:
<div class="block">
<svg height="40" viewBox="0 0 4 1"
data-expected-client-width="160" data-expected-client-height="40"></svg>
</div>

viewBox and width:
<div class="block">
<svg width="40" viewBox="0 0 4 1"
data-expected-client-width="40" data-expected-client-height="10"></svg>
</div>

viewBox, width, height:
<div class="block">
<svg width="40" height="40" viewBox="0 0 4 1"
data-expected-client-width="40" data-expected-client-height="40"></svg>
</div>

Just viewBox:
<div class="block">
<!-- Google Chrome 107 and Safari 16.0 both render this SVG as size 0x0. -->
<svg viewBox="0 0 4 1"
data-expected-client-width="0" data-expected-client-height="0"></svg>
</div>
</body>

0 comments on commit d660a50

Please sign in to comment.