Fix vertical alignment at the root of an IFC (#31636)

At the root of an inline formatting context, we used its vertical-align in order to compute the strut. That was wrong, since vertical-align on a block container shouldn't affect the contents, it should only affect the alignment of the block container (if it's inline-level) within the parent IFC. This was only working well if the block container was block-level, since effective_vertical_align_for_inline_layout returned `baseline` for block-level boxes. Instead of the outer display type, this patch changes the logic to check whether we are at the root of the IFC.
2025-07-23 15:23:42 +01:00 · 2024-03-13 11:39:01 +01:00 · 2024-03-13 11:39:01 +01:00 · 0860deba05
commit 0860deba05
parent 63527f56ca
7 changed files with 223 additions and 58 deletions
--- a/components/layout_2020/flow/inline.rs
+++ b/components/layout_2020/flow/inline.rs
@ -76,12 +76,13 @@ use gfx::font::FontMetrics;
 use gfx::text::glyph::GlyphStore;
 use serde::Serialize;
 use servo_arc::Arc;
+use style::computed_values::vertical_align::T as VerticalAlign;
 use style::computed_values::white_space::T as WhiteSpace;
 use style::context::QuirksMode;
 use style::logical_geometry::WritingMode;
 use style::properties::ComputedValues;
-use style::values::computed::{Length, LengthPercentage};
-use style::values::generics::box_::{GenericVerticalAlign, VerticalAlignKeyword};
+use style::values::computed::Length;
+use style::values::generics::box_::VerticalAlignKeyword;
 use style::values::generics::text::LineHeight;
 use style::values::specified::text::{TextAlignKeyword, TextDecorationLine};
 use style::values::specified::{TextAlignLast, TextJustify};
@ -1259,7 +1260,12 @@ impl<'a, 'b> InlineFormattingContextState<'a, 'b> {
        let strut_size = if using_fallback_font {
            // TODO(mrobinson): This value should probably be cached somewhere.
            let container_state = self.current_inline_container_state();
-            let mut block_size = container_state.get_block_size_contribution(&font_metrics);
+            let vertical_align = effective_vertical_align(
+                &container_state.style,
+                self.inline_box_state_stack.last().map(|c| &c.base),
+            );
+            let mut block_size =
+                container_state.get_block_size_contribution(vertical_align, &font_metrics);
            block_size.adjust_for_baseline_offset(container_state.baseline_offset);
            block_size
        } else if quirks_mode && !is_collapsible_whitespace {
@ -1725,13 +1731,17 @@ impl InlineContainerState {
        let line_height = line_height(&style, &font_metrics);

        let mut baseline_offset = Au::zero();
-        let mut strut_block_sizes =
-            Self::get_block_sizes_with_style(&style, &font_metrics, line_height);
+        let mut strut_block_sizes = Self::get_block_sizes_with_style(
+            effective_vertical_align(&style, parent_container),
+            &style,
+            &font_metrics,
+            line_height,
+        );
        if let Some(parent_container) = parent_container {
            // The baseline offset from `vertical-align` might adjust where our block size contribution is
            // within the line.
            baseline_offset = parent_container.get_cumulative_baseline_offset_for_child(
-                style.effective_vertical_align_for_inline_layout(),
+                style.clone_vertical_align(),
                &strut_block_sizes,
            );
            strut_block_sizes.adjust_for_baseline_offset(baseline_offset);
@ -1756,11 +1766,11 @@ impl InlineContainerState {
    }

    fn get_block_sizes_with_style(
+        vertical_align: VerticalAlign,
        style: &ComputedValues,
        font_metrics: &FontMetrics,
        line_height: Length,
    ) -> LineBlockSizes {
-        let vertical_align = style.effective_vertical_align_for_inline_layout();
        if !is_baseline_relative(vertical_align) {
            return LineBlockSizes {
                line_height,
@ -1820,8 +1830,13 @@ impl InlineContainerState {
        }
    }

-    fn get_block_size_contribution(&self, font_metrics: &FontMetrics) -> LineBlockSizes {
+    fn get_block_size_contribution(
+        &self,
+        vertical_align: VerticalAlign,
+        font_metrics: &FontMetrics,
+    ) -> LineBlockSizes {
        Self::get_block_sizes_with_style(
+            vertical_align,
            &self.style,
            font_metrics,
            line_height(&self.style, font_metrics),
@ -1830,35 +1845,36 @@ impl InlineContainerState {

    fn get_cumulative_baseline_offset_for_child(
        &self,
-        child_vertical_align: GenericVerticalAlign<LengthPercentage>,
+        child_vertical_align: VerticalAlign,
        child_block_size: &LineBlockSizes,
    ) -> Au {
-        let block_size = self.get_block_size_contribution(&self.font_metrics);
+        let block_size =
+            self.get_block_size_contribution(child_vertical_align.clone(), &self.font_metrics);
        self.baseline_offset +
            match child_vertical_align {
                // `top` and `bottom are not actually relative to the baseline, but this value is unused
                // in those cases.
                // TODO: We should distinguish these from `baseline` in order to implement "aligned subtrees" properly.
                // See https://drafts.csswg.org/css2/#aligned-subtree.
-                GenericVerticalAlign::Keyword(VerticalAlignKeyword::Baseline) |
-                GenericVerticalAlign::Keyword(VerticalAlignKeyword::Top) |
-                GenericVerticalAlign::Keyword(VerticalAlignKeyword::Bottom) => Au::zero(),
-                GenericVerticalAlign::Keyword(VerticalAlignKeyword::Sub) => Au::from_f32_px(
+                VerticalAlign::Keyword(VerticalAlignKeyword::Baseline) |
+                VerticalAlign::Keyword(VerticalAlignKeyword::Top) |
+                VerticalAlign::Keyword(VerticalAlignKeyword::Bottom) => Au::zero(),
+                VerticalAlign::Keyword(VerticalAlignKeyword::Sub) => Au::from_f32_px(
                    block_size
                        .resolve()
                        .scale_by(FONT_SUBSCRIPT_OFFSET_RATIO)
                        .px(),
                ),
-                GenericVerticalAlign::Keyword(VerticalAlignKeyword::Super) => -Au::from_f32_px(
+                VerticalAlign::Keyword(VerticalAlignKeyword::Super) => -Au::from_f32_px(
                    block_size
                        .resolve()
                        .scale_by(FONT_SUPERSCRIPT_OFFSET_RATIO)
                        .px(),
                ),
-                GenericVerticalAlign::Keyword(VerticalAlignKeyword::TextTop) => {
+                VerticalAlign::Keyword(VerticalAlignKeyword::TextTop) => {
                    child_block_size.size_for_baseline_positioning.ascent - self.font_metrics.ascent
                },
-                GenericVerticalAlign::Keyword(VerticalAlignKeyword::Middle) => {
+                VerticalAlign::Keyword(VerticalAlignKeyword::Middle) => {
                    // "Align the vertical midpoint of the box with the baseline of the parent
                    // box plus half the x-height of the parent."
                    (child_block_size.size_for_baseline_positioning.ascent -
@ -1866,11 +1882,11 @@ impl InlineContainerState {
                        self.font_metrics.x_height)
                        .scale_by(0.5)
                },
-                GenericVerticalAlign::Keyword(VerticalAlignKeyword::TextBottom) => {
+                VerticalAlign::Keyword(VerticalAlignKeyword::TextBottom) => {
                    self.font_metrics.descent -
                        child_block_size.size_for_baseline_positioning.descent
                },
-                GenericVerticalAlign::Length(length_percentage) => {
+                VerticalAlign::Length(length_percentage) => {
                    Au::from_f32_px(-length_percentage.resolve(child_block_size.line_height).px())
                },
            }
@ -2156,11 +2172,25 @@ fn line_height(parent_style: &ComputedValues, font_metrics: &FontMetrics) -> Len
    }
 }

-fn is_baseline_relative(vertical_align: GenericVerticalAlign<LengthPercentage>) -> bool {
+fn effective_vertical_align(
+    style: &ComputedValues,
+    container: Option<&InlineContainerState>,
+) -> VerticalAlign {
+    if container.is_none() {
+        // If we are at the root of the inline formatting context, we shouldn't use the
+        // computed `vertical-align`, since it has no effect on the contents of this IFC
+        // (it can just affect how the block container is aligned within the parent IFC).
+        VerticalAlign::Keyword(VerticalAlignKeyword::Baseline)
+    } else {
+        style.clone_vertical_align()
+    }
+}
+
+fn is_baseline_relative(vertical_align: VerticalAlign) -> bool {
    !matches!(
        vertical_align,
-        GenericVerticalAlign::Keyword(VerticalAlignKeyword::Top) |
-            GenericVerticalAlign::Keyword(VerticalAlignKeyword::Bottom)
+        VerticalAlign::Keyword(VerticalAlignKeyword::Top) |
+            VerticalAlign::Keyword(VerticalAlignKeyword::Bottom)
    )
 }

--- a/components/layout_2020/flow/line.rs
+++ b/components/layout_2020/flow/line.rs
@ -10,7 +10,7 @@ use gfx::font::FontMetrics;
 use gfx::text::glyph::GlyphStore;
 use servo_arc::Arc;
 use style::properties::ComputedValues;
-use style::values::computed::{Length, LengthPercentage};
+use style::values::computed::Length;
 use style::values::generics::box_::{GenericVerticalAlign, VerticalAlignKeyword};
 use style::values::generics::text::LineHeight;
 use style::values::specified::box_::DisplayOutside;
@ -28,7 +28,7 @@ use crate::geom::{LogicalRect, LogicalVec2};
 use crate::positioned::{
    relative_adjustement, AbsolutelyPositionedBox, PositioningContext, PositioningContextLength,
 };
-use crate::style_ext::{ComputedValuesExt, PaddingBorderMargin};
+use crate::style_ext::PaddingBorderMargin;
 use crate::ContainingBlock;

 pub(super) struct LineMetrics {
@ -226,16 +226,10 @@ impl TextRunLineItem {
        // The block start of the TextRun is often zero (meaning it has the same font metrics as the
        // inline box's strut), but for children of the inline formatting context root or for
        // fallback fonts that use baseline relatve alignment, it might be different.
-        let mut start_corner = &LogicalVec2 {
+        let start_corner = &LogicalVec2 {
            inline: state.inline_position,
            block: (state.baseline_offset - self.font_metrics.ascent).into(),
        } - &state.parent_offset;
-        if !is_baseline_relative(
-            self.parent_style
-                .effective_vertical_align_for_inline_layout(),
-        ) {
-            start_corner.block = Length::zero();
-        }

        let rect = LogicalRect {
            start_corner,
@ -428,12 +422,11 @@ impl InlineBoxLineItem {
    /// Given the state for a line item layout and the space above the baseline for this inline
    /// box, find the block start position relative to the line block start position.
    fn calculate_block_start(&self, state: &LineItemLayoutState, space_above_baseline: Au) -> Au {
-        let vertical_align = self.style.effective_vertical_align_for_inline_layout();
        let line_gap = self.font_metrics.line_gap;

        // The baseline offset that we have in `Self::baseline_offset` is relative to the line
        // baseline, so we need to make it relative to the line block start.
-        match vertical_align {
+        match self.style.clone_vertical_align() {
            GenericVerticalAlign::Keyword(VerticalAlignKeyword::Top) => {
                let line_height: Au = line_height(&self.style, &self.font_metrics).into();
                (line_height - line_gap).scale_by(0.5)
@ -586,14 +579,6 @@ impl FloatLineItem {
    }
 }

-fn is_baseline_relative(vertical_align: GenericVerticalAlign<LengthPercentage>) -> bool {
-    !matches!(
-        vertical_align,
-        GenericVerticalAlign::Keyword(VerticalAlignKeyword::Top) |
-            GenericVerticalAlign::Keyword(VerticalAlignKeyword::Bottom)
-    )
-}
-
 fn line_height(parent_style: &ComputedValues, font_metrics: &FontMetrics) -> Length {
    let font_size = parent_style.get_font().font_size.computed_size();
    match parent_style.get_inherited_text().line_height {
--- a/components/layout_2020/style_ext.rs
+++ b/components/layout_2020/style_ext.rs
@ -13,9 +13,8 @@ use style::properties::longhands::column_span::computed_value::T as ColumnSpan;
 use style::properties::ComputedValues;
 use style::values::computed::image::Image as ComputedImageLayer;
 use style::values::computed::{Length, LengthPercentage, NonNegativeLengthPercentage, Size};
-use style::values::generics::box_::{GenericVerticalAlign, Perspective, VerticalAlignKeyword};
+use style::values::generics::box_::Perspective;
 use style::values::generics::length::MaxSize;
-use style::values::specified::box_::DisplayOutside as StyloDisplayOutside;
 use style::values::specified::{box_ as stylo, Overflow};
 use style::Zero;
 use webrender_api as wr;
@ -189,7 +188,6 @@ pub(crate) trait ComputedValuesExt {
    fn establishes_containing_block_for_all_descendants(&self) -> bool;
    fn background_is_transparent(&self) -> bool;
    fn get_webrender_primitive_flags(&self) -> wr::PrimitiveFlags;
-    fn effective_vertical_align_for_inline_layout(&self) -> GenericVerticalAlign<LengthPercentage>;
 }

 impl ComputedValuesExt for ComputedValues {
@ -560,18 +558,6 @@ impl ComputedValuesExt for ComputedValues {
            BackfaceVisiblity::Hidden => wr::PrimitiveFlags::empty(),
        }
    }
-
-    /// Get the effective `vertical-align` property for inline layout. Essentially, if this style
-    /// has outside block display, this is the inline formatting context root and `vertical-align`
-    /// doesn't come into play for inline layout.
-    fn effective_vertical_align_for_inline_layout(&self) -> GenericVerticalAlign<LengthPercentage> {
-        match self.clone_display().outside() {
-            StyloDisplayOutside::Block => {
-                GenericVerticalAlign::Keyword(VerticalAlignKeyword::Baseline)
-            },
-            _ => self.clone_vertical_align(),
-        }
-    }
 }

 impl From<stylo::Display> for Display {
--- a/tests/wpt/meta/MANIFEST.json
+++ b/tests/wpt/meta/MANIFEST.json
@ -63058,6 +63058,19 @@
        {}
       ]
      ],
+      "vertical-align-122.xht": [
+       "1d7c180f197b9262dd98aa162595b3eb914f60d1",
+       [
+        null,
+        [
+         [
+          "/css/CSS2/linebox/vertical-align-122-ref.xht",
+          "=="
+         ]
+        ],
+        {}
+       ]
+      ],
      "vertical-align-applies-to-001.xht": [
       "fca1480f7367209223f370ff01acbc45ef7b3778",
       [
@ -371510,6 +371523,10 @@
       "c0db9e0e8bbc2bbec91ffaa564e326b4744dd2fd",
       []
      ],
+      "vertical-align-122-ref.xht": [
+       "86b4856266285a3a9b66b5c426384d2699cbaceb",
+       []
+      ],
      "vertical-align-applies-to-001-ref.xht": [
       "8ed51e0c21a0613e67c49e3e36e488e176027165",
       []
--- a/tests/wpt/meta/css/CSS2/linebox/inline-formatting-context-011.xht.ini
+++ b/tests/wpt/meta/css/CSS2/linebox/inline-formatting-context-011.xht.ini
@ -1,2 +0,0 @@
-[inline-formatting-context-011.xht]
-  expected: FAIL
--- a/tests/wpt/tests/css/CSS2/linebox/vertical-align-122-ref.xht
+++ b/tests/wpt/tests/css/CSS2/linebox/vertical-align-122-ref.xht
@ -0,0 +1,87 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml">
+ <head>
+  <title>CSS Reftest Reference</title>
+  <link rel="author" title="Oriol Brufau" href="mailto:obrufau@igalia.com"/>
+
+  <link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
+  <style type="text/css">
+  <![CDATA[
+  body {
+    position: relative;
+    font-size: 20px;
+  }
+  .person {
+    position: absolute;
+    width: 6em;
+    height: 8.9em;
+  }
+  .face {
+    position: relative;
+    height: 4.2em;
+    background: currentcolor;
+  }
+  .face > div {
+    position: absolute;
+    background: white;
+  }
+  .face > .mouth {
+    left: 2em;
+    top: 3em;
+    width: 2em;
+    height: .2em;
+  }
+  .face > .eye {
+    top: 1em;
+    width: 1em;
+    height: 1.2em;
+  }
+  .face > .eye.left {
+    left: 1em;
+  }
+  .face > .eye.right {
+    right: 1em;
+  }
+  .torso {
+    margin-top: 0.5em;
+    height: 2.2em;
+    border: 1em solid;
+  }
+  ]]>
+  </style>
+ </head>
+ <body>
+  <div class="person" style="left: 0em; top: 0em">
+    <div class="face">
+      <div class="eye left"></div>
+      <div class="eye right"></div>
+      <div class="mouth"></div>
+    </div>
+    <div class="torso"></div>
+  </div>
+  <div class="person" style="left: 7em; top: 0em">
+    <div class="face">
+      <div class="eye left"></div>
+      <div class="eye right"></div>
+      <div class="mouth"></div>
+    </div>
+    <div class="torso"></div>
+  </div>
+  <div class="person" style="left: 14em; top: .5em">
+    <div class="face">
+      <div class="eye left"></div>
+      <div class="eye right"></div>
+      <div class="mouth"></div>
+    </div>
+    <div class="torso"></div>
+  </div>
+  <div class="person" style="left: 21em; top: .5em">
+    <div class="face">
+      <div class="eye left"></div>
+      <div class="eye right"></div>
+      <div class="mouth"></div>
+    </div>
+    <div class="torso"></div>
+  </div>
+ </body>
+</html>
--- a/tests/wpt/tests/css/CSS2/linebox/vertical-align-122.xht
+++ b/tests/wpt/tests/css/CSS2/linebox/vertical-align-122.xht
@ -0,0 +1,62 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml">
+ <head>
+  <title>CSS Test: vertical-align on inline-block</title>
+  <link rel="author" title="Oriol Brufau" href="mailto:obrufau@igalia.com"/>
+  <link rel="help" href="http://www.w3.org/TR/CSS21/visudet.html#line-height" />
+  <link rel="help" href="https://github.com/servo/servo/issues/31604" />
+  <link rel="help" href="https://github.com/servo/servo/issues/31634" />
+  <link rel="match" href="vertical-align-122-ref.xht" />
+  <meta name="assert" content="When vertical-align is used on an inline-block, it aligns the inline-block itself, not the contents inside it." />
+
+  <link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
+  <style type="text/css">
+  <![CDATA[
+  body {
+    font: 20px/1 Ahem;
+  }
+  .wrapper {
+    display: inline-block;
+    border: 1em solid;
+  }
+  .wrapper > div {
+    display: inline-block;
+    height: 2em;
+    width: 2em;
+    background: currentcolor;
+  }
+  .wrapper > canvas {
+    width: 4em;
+    height: 2em;
+  }
+  ]]>
+  </style>
+ </head>
+ <body>
+  <div class="wrapper" style="vertical-align: baseline">
+    X<div></div><span>X</span>
+  </div>
+  <div class="wrapper" style="vertical-align: top">
+    X<div></div><span>X</span>
+  </div>
+  <div class="wrapper" style="vertical-align: middle">
+    X<div></div><span>X</span>
+  </div>
+  <div class="wrapper" style="vertical-align: bottom">
+    X<div></div><span>X</span>
+  </div>
+  <br />
+  <div class="wrapper" style="vertical-align: baseline">
+    <canvas></canvas>
+  </div>
+  <div class="wrapper" style="vertical-align: top">
+    <canvas></canvas>
+  </div>
+  <div class="wrapper" style="vertical-align: middle">
+    <canvas></canvas>
+  </div>
+  <div class="wrapper" style="vertical-align: bottom">
+    <canvas></canvas>
+  </div>
+ </body>
+</html>