diff options
author | Oriol Brufau <obrufau@igalia.com> | 2024-10-25 19:13:07 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-10-25 17:13:07 +0000 |
commit | dfe1c95aa6fe00bb1a251ddc03d760fbb78152d2 (patch) | |
tree | 5b3b039c32a7615122601a397b0423d5bd665130 | |
parent | 43c8441f6ce0a9f41fbf4ff020454ef120c8dab0 (diff) | |
download | servo-dfe1c95aa6fe00bb1a251ddc03d760fbb78152d2.tar.gz servo-dfe1c95aa6fe00bb1a251ddc03d760fbb78152d2.zip |
Avoid crash in replaced layout, and fix behavior for non-auto aspect-ratio (#34006)
Also, it was assuming that the aspect ratio would work with the content
box dimensions, but that isn't the case for `aspect-ratio: <ratio>` with
`box-sizing: border-box`.
Signed-off-by: Oriol Brufau <obrufau@igalia.com>
4 files changed, 140 insertions, 8 deletions
diff --git a/components/layout_2020/replaced.rs b/components/layout_2020/replaced.rs index 290d792c5c2..ed740e9fc3b 100644 --- a/components/layout_2020/replaced.rs +++ b/components/layout_2020/replaced.rs @@ -614,13 +614,13 @@ impl ReplacedContent { }, // Rows 6-7. (Violation::Above(max_inline_size), Violation::Above(max_block_size)) => { - if max_inline_size.0 * block_size.0 <= max_block_size.0 * inline_size.0 { + let transferred_block_size = + ratio.compute_dependent_size(Direction::Block, max_inline_size); + if transferred_block_size <= max_block_size { // Row 6. LogicalVec2 { inline: max_inline_size, - block: ratio - .compute_dependent_size(Direction::Block, max_inline_size) - .max(min_box_size.block), + block: transferred_block_size.max(min_box_size.block), } } else { // Row 7. @@ -634,12 +634,12 @@ impl ReplacedContent { }, // Rows 8-9. (Violation::Below(min_inline_size), Violation::Below(min_block_size)) => { - if min_inline_size.0 * block_size.0 <= min_block_size.0 * inline_size.0 { + let transferred_inline_size = + ratio.compute_dependent_size(Direction::Inline, min_block_size); + if min_inline_size <= transferred_inline_size { // Row 8. LogicalVec2 { - inline: ratio - .compute_dependent_size(Direction::Inline, min_block_size) - .clamp_below_max(max_box_size.inline), + inline: transferred_inline_size.clamp_below_max(max_box_size.inline), block: min_block_size, } } else { diff --git a/tests/wpt/meta/MANIFEST.json b/tests/wpt/meta/MANIFEST.json index 6da76d13452..131e04dd43b 100644 --- a/tests/wpt/meta/MANIFEST.json +++ b/tests/wpt/meta/MANIFEST.json @@ -553,6 +553,17 @@ ] ] } + }, + "visudet": { + "crashtests": { + "canvas-huge-min-max-sizes.html": [ + "63413085347c0d85a9e8c0b5f2b7c13467dba962", + [ + null, + {} + ] + ] + } } }, "compositing": { @@ -569016,6 +569027,13 @@ {} ] ], + "replaced-element-042.html": [ + "2a301c3e1930d3351435ad17938b19c67ad85e8d", + [ + null, + {} + ] + ], "sign-function-aspect-ratio.html": [ "e5ba1a8321a42918cccee4ee164527fa25078e4f", [ diff --git a/tests/wpt/tests/css/CSS2/visudet/crashtests/canvas-huge-min-max-sizes.html b/tests/wpt/tests/css/CSS2/visudet/crashtests/canvas-huge-min-max-sizes.html new file mode 100644 index 00000000000..63413085347 --- /dev/null +++ b/tests/wpt/tests/css/CSS2/visudet/crashtests/canvas-huge-min-max-sizes.html @@ -0,0 +1,21 @@ +<!DOCTYPE html> +<meta charset="utf-8"> +<title>Don't crash with canvas with big min or max sizes</title> +<link rel="help" href="https://drafts.csswg.org/css2/visudet.html#min-max-widths"> +<link rel="help" href="https://github.com/servo/servo/issues/33961"> +<link rel="author" title="Oriol Brufau" href="mailto:obrufau@igalia.com"> + +<!-- +According to https://drafts.csswg.org/css2/visudet.html#min-max-widths, +we need to check whether max-width/w ≤ max-height/h holds. +Or equivalently we can check whether max-width*h ≤ max-height*w holds, +but 100000 * 100001 = 10000200001 doesn't fit in a 32-bit integer. +--> +<canvas height="100001" width="100001" style="max-width: 100000px; max-height: 100000px;"></canvas> + +<!-- +Similarly, here we need to check whether min-width/w ≤ min-height/h holds. +Or equivalently we can check whether min-width*h ≤ min-height*w holds, +but again 100001 * 100000 = 10000200001 doesn't fit in a 32-bit integer. +--> +<canvas height="100000" width="100000" style="min-width: 100001px; min-height: 100001px;"></canvas> diff --git a/tests/wpt/tests/css/css-sizing/aspect-ratio/replaced-element-042.html b/tests/wpt/tests/css/css-sizing/aspect-ratio/replaced-element-042.html new file mode 100644 index 00000000000..2a301c3e193 --- /dev/null +++ b/tests/wpt/tests/css/css-sizing/aspect-ratio/replaced-element-042.html @@ -0,0 +1,93 @@ +<!DOCTYPE html> +<meta charset="utf-8"> +<title>CSS aspect-ratio: replaced element with box-sizing</title> +<link rel="help" href="https://drafts.csswg.org/css-sizing-4/#aspect-ratio"> +<link rel="help" href="https://drafts.csswg.org/css2/visudet.html#min-max-widths"> +<link rel="author" title="Oriol Brufau" href="mailto:obrufau@igalia.com"> + +<style> +canvas { + box-sizing: border-box; + aspect-ratio: 1; + width: auto; + height: auto; + background: black; +} +</style> + +<div id="log"></div> + +<!-- +From https://drafts.csswg.org/css2/visudet.html#min-max-widths + +> for replaced elements with an intrinsic ratio and both `width` and `height` specified as `auto`, +> the algorithm is as follows: +> +> Select from the table the resolved height and width values for the appropriate constraint violation. +> Take the max-width and max-height as max(min, max) so that min ≤ max holds true. +> In this table w and h stand for the results of the width and height computations +> ignoring the `min-width`, `min-height`, `max-width` and `max-height` properties. +> Normally these are the intrinsic width and height, but they may not be +> in the case of replaced elements with intrinsic ratios. + +Note the testcases below ensure that w/h is 1 to match the provided `aspect-ratio`, +which didn't exist in CSS2. + +Also note that `aspect-ratio: 1` refers to the border-box due to `box-sizing`, +so we need to perform all calculations using border-box sizes. +--> + +<!-- +w = 100 + 0 = 100 +h = 50 + 50 = 100 +max-width = 50 +max-height = 70 +Constraint Violation = "(w > max-width) and (h > max-height), where (max-width/w ≤ max-height/h)" +Resolved Width = max-width = 50 +Resolved Height = max(min-height, max-width * h/w) = max(0, 50*100/100) = 50 +--> +<canvas width="100" height="50" style="max-width: 50px; max-height: 70px; padding-top: 50px" + data-expected-width="50" data-expected-height="50"></canvas> + +<!-- +w = 50 + 50 = 100 +h = 100 + 0 = 100 +max-width = 70 +max-height = 50 +Constraint Violation = "(w > max-width) and (h > max-height), where (max-width/w > max-height/h)" +Resolved Width = max(min-width, max-height * w/h) = max(0, 50*100/100) = 50 +Resolved Height = max-height = 50 +--> +<canvas width="50" height="100" style="max-width: 70px; max-height: 50px; padding-left: 50px" + data-expected-width="50" data-expected-height="50"></canvas> + +<!-- +w = 50 + 50 = 100 +h = 100 + 0 = 100 +min-width = 150 +min-height = 175 +Constraint Violation = "(w < min-width) and (h < min-height), where (min-width/w ≤ min-height/h)" +Resolved Width = min(max-width, min-height * w/h) = min(∞, 175*100/100) = 175 +Resolved Height = min-height = 175 +--> +<canvas width="50" height="100" style="min-width: 150px; min-height: 175px; padding-left: 50px" + data-expected-width="175" data-expected-height="175"></canvas> + +<!-- +w = 100 + 0 = 100 +h = 50 + 50 = 100 +min-width = 175 +min-height = 150 +Constraint Violation = "(w < min-width) and (h < min-height), where (min-width/w > min-height/h)" +Resolved Width = min-width = 175 +Resolved Height = min(max-height, min-width * h/w) = min(∞, 175*100/100) = 175 +--> +<canvas width="100" height="50" style="min-width: 175px; min-height: 150px; padding-top: 50px" + data-expected-width="175" data-expected-height="175"></canvas> + +<script src="/resources/testharness.js"></script> +<script src="/resources/testharnessreport.js"></script> +<script src="/resources/check-layout-th.js"></script> +<script> +checkLayout("canvas"); +</script> |