LibWeb: Respect font-size specified by CSS in "em" length calculations

"5em" means 5*font-size, but by forcing "em" to mean the presentation size of the bitmap font actually used, we broke a bunch of layouts that depended on a correct interpretation of "em". This means that "em" units will no longer be relative to the exact size of the bitmap font in use, but I think that's a compromise we'll have to make, since accurate layouts are more important. This yields a visual progression on both ACID2 and ACID3. :^)
cubiclove · Feb 21, 2022 · c61747f · c61747f
1 parent 2615728
commit c61747f
Show file tree

Hide file tree

Showing 6 changed files with 32 additions and 13 deletions.
diff --git a/Userland/Libraries/LibWeb/CSS/ComputedValues.h b/Userland/Libraries/LibWeb/CSS/ComputedValues.h
@@ -14,6 +14,8 @@ namespace Web::CSS {
 
 class InitialValues {
 public:
+ static float font_size() { return 10; }
+ static int font_weight() { return 400; }
  static CSS::Float float_() { return CSS::Float::None; }
  static CSS::Clear clear() { return CSS::Clear::None; }
  static CSS::Cursor cursor() { return CSS::Cursor::Auto; }
@@ -146,6 +148,9 @@ class ComputedValues {
 
  Vector<CSS::Transformation> transformations() const { return m_noninherited.transformations; }
 
+ float font_size() const { return m_inherited.font_size; }
+ int font_weight() const { return m_inherited.font_weight; }
+
  ComputedValues clone_inherited_values() const
  {
  ComputedValues clone;
@@ -155,6 +160,8 @@ class ComputedValues {
 
 protected:
  struct {
+ float font_size { InitialValues::font_size() };
+ int font_weight { InitialValues::font_weight() };
  Color color { InitialValues::color() };
  CSS::Cursor cursor { InitialValues::cursor() };
  CSS::ImageRendering image_rendering { InitialValues::image_rendering() };
@@ -217,6 +224,8 @@ class ImmutableComputedValues final : public ComputedValues {
 
 class MutableComputedValues final : public ComputedValues {
 public:
+ void set_font_size(float font_size) { m_inherited.font_size = font_size; }
+ void set_font_weight(int font_weight) { m_inherited.font_weight = font_weight; }
  void set_color(const Color& color) { m_inherited.color = color; }
  void set_cursor(CSS::Cursor cursor) { m_inherited.cursor = cursor; }
  void set_image_rendering(CSS::ImageRendering value) { m_inherited.image_rendering = value; }

diff --git a/Userland/Libraries/LibWeb/CSS/Length.cpp b/Userland/Libraries/LibWeb/CSS/Length.cpp
@@ -65,13 +65,13 @@ Length Length::resolved(Layout::Node const& layout_node) const
  return *this;
 }
 
-float Length::relative_length_to_px(Gfx::IntRect const& viewport_rect, Gfx::FontMetrics const& font_metrics, float root_font_size) const
+float Length::relative_length_to_px(Gfx::IntRect const& viewport_rect, Gfx::FontMetrics const& font_metrics, float font_size, float root_font_size) const
 {
  switch (m_type) {
  case Type::Ex:
  return m_value * font_metrics.x_height;
  case Type::Em:
- return m_value * font_metrics.size;
+ return m_value * font_size;
  case Type::Ch:
  // FIXME: Use layout_node.font().glyph_height() when writing-mode is not horizontal-tb (it has to be implemented first)
  return m_value * (font_metrics.glyph_width + font_metrics.glyph_spacing);
@@ -101,7 +101,7 @@ float Length::to_px(Layout::Node const& layout_node) const
  auto* root_element = layout_node.document().document_element();
  if (!root_element || !root_element->layout_node())
  return 0;
- return to_px(viewport_rect, layout_node.font().metrics('M'), root_element->layout_node()->font().presentation_size());
+ return to_px(viewport_rect, layout_node.font().metrics('M'), layout_node.computed_values().font_size(), root_element->layout_node()->computed_values().font_size());
 }
 
 const char* Length::unit_name() const

diff --git a/Userland/Libraries/LibWeb/CSS/Length.h b/Userland/Libraries/LibWeb/CSS/Length.h
@@ -77,12 +77,12 @@ class Length {
 
  float to_px(Layout::Node const&) const;
 
- ALWAYS_INLINE float to_px(Gfx::IntRect const& viewport_rect, Gfx::FontMetrics const& font_metrics, float root_font_size) const
+ ALWAYS_INLINE float to_px(Gfx::IntRect const& viewport_rect, Gfx::FontMetrics const& font_metrics, float font_size, float root_font_size) const
  {
  if (is_auto())
  return 0;
  if (is_relative())
- return relative_length_to_px(viewport_rect, font_metrics, root_font_size);
+ return relative_length_to_px(viewport_rect, font_metrics, font_size, root_font_size);
  if (is_calculated())
  VERIFY_NOT_REACHED(); // We can't resolve a calculated length from here. :^(
  return absolute_length_to_px();
@@ -129,7 +129,7 @@ class Length {
  return !(*this == other);
  }
 
- float relative_length_to_px(Gfx::IntRect const& viewport_rect, Gfx::FontMetrics const& font_metrics, float root_font_size) const;
+ float relative_length_to_px(Gfx::IntRect const& viewport_rect, Gfx::FontMetrics const& font_metrics, float font_size, float root_font_size) const;
 
 private:
  const char* unit_name() const;

diff --git a/Userland/Libraries/LibWeb/CSS/MediaQuery.cpp b/Userland/Libraries/LibWeb/CSS/MediaQuery.cpp
@@ -153,12 +153,13 @@ bool MediaFeature::compare(DOM::Window const& window, MediaFeatureValue left, Co
 
  // FIXME: This isn't right - we want to query the initial-value font, which is the one used
  // if no author styles are defined.
- auto const& font = window.associated_document().root().layout_node()->font();
+ auto& root_layout_node = *window.associated_document().root().layout_node();
+ auto const& font = root_layout_node.font();
  Gfx::FontMetrics const& font_metrics = font.metrics('M');
- float root_font_size = font.presentation_size();
+ float root_font_size = root_layout_node.computed_values().font_size();
 
- left_px = left.length().to_px(viewport_rect, font_metrics, root_font_size);
- right_px = right.length().to_px(viewport_rect, font_metrics, root_font_size);
+ left_px = left.length().to_px(viewport_rect, font_metrics, root_font_size, root_font_size);
+ right_px = right.length().to_px(viewport_rect, font_metrics, root_font_size, root_font_size);
  }
 
  switch (comparison) {

diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp
@@ -770,7 +770,7 @@ void StyleComputer::compute_font(StyleProperties& style, DOM::Element const* ele
  if (value->is_length()) {
  auto length = static_cast<LengthStyleValue const&>(*value).to_length();
  if (length.is_absolute() || length.is_relative())
- parent_font_size = length.to_px(viewport_rect, font_metrics, root_font_size);
+ parent_font_size = length.to_px(viewport_rect, font_metrics, size, root_font_size);
  }
  }
  maybe_length = Length::make_px(percentage.as_fraction() * parent_font_size);
@@ -785,7 +785,7 @@ void StyleComputer::compute_font(StyleProperties& style, DOM::Element const* ele
  // FIXME: Support font-size: calc(...)
  // Theoretically we can do this now, but to resolve it we need a layout_node which we might not have. :^(
  if (!maybe_length->is_calculated()) {
- auto px = maybe_length.value().to_px(viewport_rect, font_metrics, root_font_size);
+ auto px = maybe_length.value().to_px(viewport_rect, font_metrics, size, root_font_size);
  if (px != 0)
  size = px;
  }
@@ -874,6 +874,9 @@ void StyleComputer::compute_font(StyleProperties& style, DOM::Element const* ele
 
  FontCache::the().set(font_selector, *found_font);
 
+ style.set_property(CSS::PropertyID::FontSize, LengthStyleValue::create(CSS::Length::make_px(size)));
+ style.set_property(CSS::PropertyID::FontWeight, NumericStyleValue::create_integer(weight));
+
  style.set_computed_font(found_font.release_nonnull());
 }
 
@@ -883,13 +886,16 @@ void StyleComputer::absolutize_values(StyleProperties& style, DOM::Element const
  auto font_metrics = style.computed_font().metrics('M');
  // FIXME: Get the root element font.
  float root_font_size = 10;
+ float font_size = style.property(CSS::PropertyID::FontSize).value()->to_length().to_px(viewport_rect, font_metrics, root_font_size, root_font_size);
 
  for (auto& value_slot : style.m_property_values) {
  if (!value_slot)
  continue;
  value_slot->visit_lengths([&](Length& length) {
+ if (length.is_px())
+ return;
  if (length.is_absolute() || length.is_relative()) {
- auto px = length.to_px(viewport_rect, font_metrics, root_font_size);
+ auto px = length.to_px(viewport_rect, font_metrics, font_size, root_font_size);
  length = Length::make_px(px);
  }
  });

diff --git a/Userland/Libraries/LibWeb/Layout/Node.cpp b/Userland/Libraries/LibWeb/Layout/Node.cpp
@@ -322,6 +322,9 @@ void NodeWithStyle::apply_style(const CSS::StyleProperties& specified_style)
 
  computed_values.set_box_sizing(specified_style.box_sizing());
 
+ computed_values.set_font_size(specified_style.property(CSS::PropertyID::FontSize).value()->to_length().to_px(*this));
+ computed_values.set_font_weight(specified_style.property(CSS::PropertyID::FontWeight).value()->to_integer());
+
  // FIXME: BorderXRadius properties are now BorderRadiusStyleValues, so make use of that.
  auto border_bottom_left_radius = specified_style.property(CSS::PropertyID::BorderBottomLeftRadius);
  if (border_bottom_left_radius.has_value() && border_bottom_left_radius.value()->is_border_radius())