From 1891c5cfafe5e589d6bf828deb8ea1ad14f70f61 Mon Sep 17 00:00:00 2001 From: Oriol Brufau Date: Wed, 30 Oct 2024 15:01:47 +0100 Subject: [PATCH] Properly transfer min/max constraints on auto-sized replaced elements (#34026) We were following CSS2, which didn't handle `aspect-ratio`. This patch simplifies the logic and handles it correctly. Unfortunately this makes 2 tests fail, but I'm pretty sure they aren't spec-compliant. I'm leaving them as-is for now since they are part of interop-2021, and Gecko, Blink and WebKit pass them (because of some non-interoperable incorrect behaviors). I'm adding a new test that is fully passed by Servo and WebKit. Signed-off-by: Oriol Brufau --- components/layout_2020/replaced.rs | 145 ++---- tests/wpt/meta/MANIFEST.json | 7 + .../replaced-element-013.html.ini | 2 + .../replaced-element-014.html.ini | 2 + .../aspect-ratio/replaced-element-043.html | 464 ++++++++++++++++++ 5 files changed, 506 insertions(+), 114 deletions(-) create mode 100644 tests/wpt/meta/css/css-sizing/aspect-ratio/replaced-element-013.html.ini create mode 100644 tests/wpt/meta/css/css-sizing/aspect-ratio/replaced-element-014.html.ini create mode 100644 tests/wpt/tests/css/css-sizing/aspect-ratio/replaced-element-043.html diff --git a/components/layout_2020/replaced.rs b/components/layout_2020/replaced.rs index 0f65d30f822..f97093519f6 100644 --- a/components/layout_2020/replaced.rs +++ b/components/layout_2020/replaced.rs @@ -5,7 +5,7 @@ use std::fmt; use std::sync::{Arc, Mutex}; -use app_units::Au; +use app_units::{Au, MAX_AU}; use base::id::{BrowsingContextId, PipelineId}; use canvas_traits::canvas::{CanvasId, CanvasMsg, FromLayoutMsg}; use data_url::DataUrl; @@ -564,119 +564,36 @@ impl ReplacedContent { if let (AuOrAuto::Auto, AuOrAuto::Auto, Some(ratio)) = (box_size.inline, box_size.block, intrinsic_ratio) { - let LogicalVec2 { - inline: inline_size, - block: block_size, - } = get_tentative_size(box_size); - enum Violation { - None, - Below(Au), - Above(Au), - } - let violation = |size: Au, min_size: Au, mut max_size: Option| { - if let Some(max) = max_size.as_mut() { - max.max_assign(min_size); - } - if size < min_size { - return Violation::Below(min_size); - } - match max_size { - Some(max_size) if size > max_size => Violation::Above(max_size), - _ => Violation::None, - } - }; - return match ( - violation(inline_size, min_box_size.inline, max_box_size.inline), - violation(block_size, min_box_size.block, max_box_size.block), - ) { - // Row 1. - (Violation::None, Violation::None) => LogicalVec2 { - inline: inline_size, - block: block_size, - }, - // Row 2. - (Violation::Above(max_inline_size), Violation::None) => LogicalVec2 { - inline: max_inline_size, - block: ratio - .compute_dependent_size(Direction::Block, max_inline_size) - .max(min_box_size.block), - }, - // Row 3. - (Violation::Below(min_inline_size), Violation::None) => LogicalVec2 { - inline: min_inline_size, - block: ratio - .compute_dependent_size(Direction::Block, min_inline_size) - .clamp_below_max(max_box_size.block), - }, - // Row 4. - (Violation::None, Violation::Above(max_block_size)) => LogicalVec2 { - inline: ratio - .compute_dependent_size(Direction::Inline, max_block_size) - .max(min_box_size.inline), - block: max_block_size, - }, - // Row 5. - (Violation::None, Violation::Below(min_block_size)) => LogicalVec2 { - inline: ratio - .compute_dependent_size(Direction::Inline, min_block_size) - .clamp_below_max(max_box_size.inline), - block: min_block_size, - }, - // Rows 6-7. - (Violation::Above(max_inline_size), Violation::Above(max_block_size)) => { - 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: transferred_block_size.max(min_box_size.block), - } - } else { - // Row 7. - LogicalVec2 { - inline: ratio - .compute_dependent_size(Direction::Inline, max_block_size) - .max(min_box_size.inline), - block: max_block_size, - } - } - }, - // Rows 8-9. - (Violation::Below(min_inline_size), Violation::Below(min_block_size)) => { - let transferred_inline_size = - ratio.compute_dependent_size(Direction::Inline, min_block_size); - if min_inline_size <= transferred_inline_size { - // Row 8. - LogicalVec2 { - inline: transferred_inline_size.clamp_below_max(max_box_size.inline), - block: min_block_size, - } - } else { - // Row 9. - LogicalVec2 { - inline: min_inline_size, - block: ratio - .compute_dependent_size(Direction::Block, min_inline_size) - .clamp_below_max(max_box_size.block), - } - } - }, - // Row 10. - (Violation::Below(min_inline_size), Violation::Above(max_block_size)) => { - LogicalVec2 { - inline: min_inline_size, - block: max_block_size, - } - }, - // Row 11. - (Violation::Above(max_inline_size), Violation::Below(min_block_size)) => { - LogicalVec2 { - inline: max_inline_size, - block: min_block_size, - } - }, - }; + let tentative_size = get_tentative_size(box_size); + let max_box_size = max_box_size.map(|max_size| max_size.unwrap_or(MAX_AU)); + // This is a simplification of the CSS2 algorithm in a way that properly handles `aspect-ratio`. + // We transfer min and max constraints from the other axis, and apply them in addition to + // non-transferred min and max constraints. In case of conflict, + // - Non-transferred constraints take precedence over transferred ones. + // - Min constraints take precedence over max ones from the same axis. + // + // + let inline = tentative_size.inline.clamp_between_extremums( + ratio + .compute_dependent_size(Direction::Inline, min_box_size.block) + .clamp_between_extremums(min_box_size.inline, Some(max_box_size.inline)), + Some( + ratio + .compute_dependent_size(Direction::Inline, max_box_size.block) + .min(max_box_size.inline), + ), + ); + let block = tentative_size.block.clamp_between_extremums( + ratio + .compute_dependent_size(Direction::Block, min_box_size.inline) + .clamp_between_extremums(min_box_size.block, Some(max_box_size.block)), + Some( + ratio + .compute_dependent_size(Direction::Block, max_box_size.inline) + .min(max_box_size.block), + ), + ); + return LogicalVec2 { inline, block }; } // https://drafts.csswg.org/css2/#min-max-widths "The following algorithm describes how the two properties diff --git a/tests/wpt/meta/MANIFEST.json b/tests/wpt/meta/MANIFEST.json index 02c46d4e455..ad5f0472deb 100644 --- a/tests/wpt/meta/MANIFEST.json +++ b/tests/wpt/meta/MANIFEST.json @@ -570268,6 +570268,13 @@ {} ] ], + "replaced-element-043.html": [ + "9ad35c1a316dfdd5163e63f671f58f3c85892100", + [ + null, + {} + ] + ], "sign-function-aspect-ratio.html": [ "e5ba1a8321a42918cccee4ee164527fa25078e4f", [ diff --git a/tests/wpt/meta/css/css-sizing/aspect-ratio/replaced-element-013.html.ini b/tests/wpt/meta/css/css-sizing/aspect-ratio/replaced-element-013.html.ini new file mode 100644 index 00000000000..e31d7aaca38 --- /dev/null +++ b/tests/wpt/meta/css/css-sizing/aspect-ratio/replaced-element-013.html.ini @@ -0,0 +1,2 @@ +[replaced-element-013.html] + expected: FAIL diff --git a/tests/wpt/meta/css/css-sizing/aspect-ratio/replaced-element-014.html.ini b/tests/wpt/meta/css/css-sizing/aspect-ratio/replaced-element-014.html.ini new file mode 100644 index 00000000000..330d09977bd --- /dev/null +++ b/tests/wpt/meta/css/css-sizing/aspect-ratio/replaced-element-014.html.ini @@ -0,0 +1,2 @@ +[replaced-element-014.html] + expected: FAIL diff --git a/tests/wpt/tests/css/css-sizing/aspect-ratio/replaced-element-043.html b/tests/wpt/tests/css/css-sizing/aspect-ratio/replaced-element-043.html new file mode 100644 index 00000000000..9ad35c1a316 --- /dev/null +++ b/tests/wpt/tests/css/css-sizing/aspect-ratio/replaced-element-043.html @@ -0,0 +1,464 @@ + + +CSS aspect-ratio: replaced element with various sizing properties + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
width + + + + + + +
height + + + + + + +
min-width + + + + + + +
min-height + + + + + + +
max-width + + + + + + +
max-height + + + + + + +
min-width, min-height + + + + + + +
+ + + + + + +
+ + + + + + +
+ + + + + + +
+ + + + + + +
+ + + + + + +
min-width, max-height + + + + + + +
+ + + + + + +
+ + + + + + +
+ + + + + + +
+ + + + + + +
+ + + + + + +
max-width, min-height + + + + + + +
+ + + + + + +
+ + + + + + +
+ + + + + + +
+ + + + + + +
+ + + + + + +
max-width, max-height + + + + + + +
+ + + + + + +
+ + + + + + +
+ + + + + + +
+ + + + + + +
+ + + + + + +
+ + + + +