Commit 2908abe6 authored by Aleks Totic's avatar Aleks Totic Committed by Commit Bot

[LayoutNG] ComputeReplacedSize edge case fix

Edge case for "have aspect ratio, but no intrinsic size"
If there was no intrinsic size, we treated it as no size at all.
But, if there is a css size, we should use that instead.

Bug: 1015311
Change-Id: Ia8f12bd23cda5c18b9aae318eee91906840475d3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1877595
Commit-Queue: Aleks Totic <atotic@chromium.org>
Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709134}
parent 4c1d7692
......@@ -618,23 +618,29 @@ void ComputeReplacedSize(const NGLayoutInputNode& node,
intrinsic_inline = ((*intrinsic_block - border_padding.BlockSum()) *
aspect_ratio.inline_size / aspect_ratio.block_size) +
border_padding.InlineSum();
} else {
} else if (!replaced_inline && !replaced_block) {
// No sizes available, return only the aspect ratio.
*out_aspect_ratio = aspect_ratio;
return;
}
}
if (!intrinsic_block) {
if (intrinsic_inline && !intrinsic_block) {
DCHECK(!aspect_ratio.IsEmpty());
intrinsic_block = ((*intrinsic_inline - border_padding.InlineSum()) *
aspect_ratio.block_size / aspect_ratio.inline_size) +
border_padding.BlockSum();
}
// At this point, both intrinsic_inline and intrinsic_block have value.
DCHECK(intrinsic_inline || intrinsic_block || replaced_inline ||
replaced_block);
// If we only know one length, the other length gets computed wrt one we know.
auto ComputeBlockFromInline = [&replaced_inline, &aspect_ratio,
&border_padding](LayoutUnit default_block) {
DCHECK(default_block >= border_padding.BlockSum());
if (aspect_ratio.IsEmpty())
if (aspect_ratio.IsEmpty()) {
DCHECK_GE(default_block, border_padding.BlockSum());
return default_block;
}
return ((*replaced_inline - border_padding.InlineSum()) *
aspect_ratio.block_size / aspect_ratio.inline_size) +
border_padding.BlockSum();
......@@ -642,20 +648,25 @@ void ComputeReplacedSize(const NGLayoutInputNode& node,
auto ComputeInlineFromBlock = [&replaced_block, &aspect_ratio,
&border_padding](LayoutUnit default_inline) {
DCHECK(default_inline >= border_padding.InlineSum());
if (aspect_ratio.IsEmpty())
if (aspect_ratio.IsEmpty()) {
DCHECK_GE(default_inline, border_padding.InlineSum());
return default_inline;
}
return ((*replaced_block - border_padding.BlockSum()) *
aspect_ratio.inline_size / aspect_ratio.block_size) +
border_padding.InlineSum();
};
if (replaced_inline) {
DCHECK(!replaced_block);
replaced_block = ComputeBlockFromInline(*intrinsic_block);
DCHECK(intrinsic_block || !aspect_ratio.IsEmpty());
replaced_block =
ComputeBlockFromInline(intrinsic_block.value_or(kIndefiniteSize));
replaced_block = ConstrainByMinMax(*replaced_block, block_min, block_max);
} else if (replaced_block) {
DCHECK(!replaced_inline);
replaced_inline = ComputeInlineFromBlock(*intrinsic_inline);
DCHECK(intrinsic_inline || !aspect_ratio.IsEmpty());
replaced_inline =
ComputeInlineFromBlock(intrinsic_inline.value_or(kIndefiniteSize));
replaced_inline =
ConstrainByMinMax(*replaced_inline, inline_min, inline_max);
} else {
......
......@@ -284,6 +284,18 @@
data-expected-width="238" data-expected-height="52" data-offset-y="141" data-offset-x="159"
>
</div>
<!-- Viewbox + css height. Has aspect_ratio and height. Width is scaled -->
<div class="container">
<img class="target" style="height:20px" src="data:image/svg+xml,%3Csvg viewBox='0 0 100 10' xmlns='http://www.w3.org/2000/svg' %3E%3Crect width='100%' height='100%' style='fill:rgb(0,255,0);'/%3E%3C/svg%3E"
data-expected-width="238" data-expected-height="52" data-offset-y="141" data-offset-x="159"
>
</div>
<!-- Viewbox + css width. Has aspect_ratio and width. Height is scaled -->
<div class="container">
<img class="target" style="width:238px" src="data:image/svg+xml,%3Csvg viewBox='0 0 100 10' xmlns='http://www.w3.org/2000/svg' %3E%3Crect width='100%' height='100%' style='fill:rgb(0,255,0);'/%3E%3C/svg%3E"
data-expected-width="276" data-expected-height="56" data-offset-y="137" data-offset-x="121"
>
</div>
<script>
// initialize png images with 200x150 green png
let pngSrc="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAMgAAACWAQMAAAChElVaAAAABGdBTUEAALGPC/xhBQAAACBjSFJNAAB6JgAAgIQAAPoAAACA6AAAdTAAAOpgAAA6mAAAF3CculE8AAAABlBMVEUAgAD///8UPy9PAAAAAWJLR0QB/wIt3gAAAAd0SU1FB+MBDwkdA1Cz/EMAAAAbSURBVEjH7cGBAAAAAMOg+VPf4ARVAQAAAM8ADzwAAeM8wQsAAAAldEVYdGRhdGU6Y3JlYXRlADIwMTktMDEtMTVUMTc6Mjk6MDMtMDg6MDCYDy9IAAAAJXRFWHRkYXRlOm1vZGlmeQAyMDE5LTAxLTE1VDE3OjI5OjAzLTA4OjAw6VKX9AAAAABJRU5ErkJggg=="
......
......@@ -38,5 +38,7 @@ FAIL minmax replaced IMG 36 assert_equals: incorrect offsetWidth expected "188"
FAIL minmax replaced IMG 37 assert_equals: incorrect offsetWidth expected "188" but got "338"
PASS minmax replaced IMG 38
PASS minmax replaced IMG 39
PASS minmax replaced IMG 40
PASS minmax replaced IMG 41
Harness: the test ran to completion.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment