Skip to content

Commit

Permalink
Avoid crash in replaced layout, and fix behavior for non-auto aspect-…
Browse files Browse the repository at this point in the history
…ratio

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 <[email protected]>
  • Loading branch information
Loirooriol authored and servo-wpt-sync committed Oct 25, 2024
1 parent c300d39 commit 8be9ad5
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 0 deletions.
21 changes: 21 additions & 0 deletions css/CSS2/visudet/crashtests/canvas-huge-min-max-sizes.html
Original file line number Diff line number Diff line change
@@ -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:[email protected]">

<!--
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>
93 changes: 93 additions & 0 deletions css/css-sizing/aspect-ratio/replaced-element-042.html
Original file line number Diff line number Diff line change
@@ -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:[email protected]">

<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>

0 comments on commit 8be9ad5

Please sign in to comment.