RGBCube/serenity
1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-26 02:57:36 +00:00
Code Issues Activity Actions

LibWeb: Forbid using CSS::Length as reference value in resolved()

Browse source 
CSSPixels should not be wrapped into CSS::Length before being passed
to resolved() to end up resolving percentages without losing
precision.

Fixes thrashing layout when 33.3333% width is used together with
"box-sizing: border-box".
This commit is contained in:
Aliaksandr Kalenik 2024-01-07 01:09:09 +01:00 • committed by Andreas Kling
parent f2cd120fd3
commit 4bc38300ad
10 changed files with 40 additions and 17 deletions
Download patch file Download diff file Expand all files Collapse all files

13
Tests/LibWeb/Layout/expected/flex/columns-33-percent-width.txt Normal file
View file

@ -0,0 +1,13 @@
Viewport <#document> at (0,0) content-size 800x600 children: not-inline
BlockContainer <html> at (0,0) content-size 800x116 [BFC] children: not-inline
Box <body> at (8,8) content-size 500x100 flex-container(row) [FFC] children: not-inline
BlockContainer <div> at (8,8) content-size 166.65625x100 flex-item [BFC] children: not-inline
BlockContainer <div> at (174.65625,8) content-size 166.65625x100 flex-item [BFC] children: not-inline
BlockContainer <div> at (341.3125,8) content-size 166.65625x100 flex-item [BFC] children: not-inline
ViewportPaintable (Viewport<#document>) [0,0 800x600]
PaintableWithLines (BlockContainer<HTML>) [0,0 800x116]
PaintableBox (Box<BODY>) [8,8 500x100]
PaintableWithLines (BlockContainer<DIV>) [8,8 166.65625x100]
PaintableWithLines (BlockContainer<DIV>) [174.65625,8 166.65625x100]
PaintableWithLines (BlockContainer<DIV>) [341.3125,8 166.65625x100]

4
Tests/LibWeb/Layout/expected/grid/grid-item-percentage-margins.txt
View file

@ -3,9 +3,9 @@ Viewport <#document> at (0,0) content-size 800x600 children: not-inline
BlockContainer <body> at (8,8) content-size 784x17.46875 children: not-inline BlockContainer <body> at (8,8) content-size 784x17.46875 children: not-inline
BlockContainer <div> at (8,8) content-size 784x17.46875 children: not-inline BlockContainer <div> at (8,8) content-size 784x17.46875 children: not-inline
Box <div.grid> at (8,8) content-size 784x17.46875 [GFC] children: not-inline Box <div.grid> at (8,8) content-size 784x17.46875 [GFC] children: not-inline
BlockContainer <div.item> at (243.203125,8) content-size 313.59375x17.46875 [BFC] children: inline BlockContainer <div.item> at (243.1875,8) content-size 313.625x17.46875 [BFC] children: inline
line 0 width: 121.0625, height: 17.46875, bottom: 17.46875, baseline: 13.53125 line 0 width: 121.0625, height: 17.46875, bottom: 17.46875, baseline: 13.53125
frag 0 from TextNode start: 0, length: 16, rect: [243.203125,8 121.0625x17.46875] frag 0 from TextNode start: 0, length: 16, rect: [243.1875,8 121.0625x17.46875]
"A filthy t-shirt" "A filthy t-shirt"
TextNode <#text> TextNode <#text>

16
Tests/LibWeb/Layout/input/flex/columns-33-percent-width.html Normal file
View file

@ -0,0 +1,16 @@
<!doctype html><style>
* {
box-sizing: border-box;
}
body {
display: flex;
flex-wrap: wrap;
background: black;
width: 500px;
}
div {
flex-basis: 33.3333%;
height: 100px;
background: magenta;
}
</style><body><div></div><div></div><div></div>

BIN
Tests/LibWeb/Ref/reference/images/css-gradients-ref.png
View file

Binary file not shown.
Side by side Swipe Overlay

Before

Width:  |  Height:  |  Size: 350 KiB

After

Width:  |  Height:  |  Size: 541 KiB

Before After
Before After

4
Tests/LibWeb/Text/expected/css/getComputedStyle-transform.txt
View file

@ -1,8 +1,8 @@
none => none none => none
matrix(1, 2, 3, 4, 5, 6) => matrix(1, 2, 3, 4, 5, 6) matrix(1, 2, 3, 4, 5, 6) => matrix(1, 2, 3, 4, 5, 6)
matrix3d(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16) => matrix3d(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16) matrix3d(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16) => matrix3d(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16)
translate(1%, 2px) => matrix(1, 0, 0, 1, 7.84375, 2) translate(1%, 2px) => matrix(1, 0, 0, 1, 7.828125, 2)
translate3d(1%, 2px, 3em) => matrix3d(1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 7.84375, 2, 48, 1) translate3d(1%, 2px, 3em) => matrix3d(1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 7.828125, 2, 48, 1)
translateX(1px) => matrix(1, 0, 0, 1, 1, 0) translateX(1px) => matrix(1, 0, 0, 1, 1, 0)
translateY(1%) => matrix(1, 0, 0, 1, 0, 0) translateY(1%) => matrix(1, 0, 0, 1, 0, 0)
scale(1, 2) => matrix(1, 0, 0, 2, 0, 0) scale(1, 2) => matrix(1, 0, 0, 2, 0, 0)

4
Userland/Libraries/LibWeb/CSS/PercentageOr.h
View file

@ -97,10 +97,10 @@ public:
CSSPixels to_px(Layout::Node const& layout_node, CSSPixels reference_value) const CSSPixels to_px(Layout::Node const& layout_node, CSSPixels reference_value) const
{ {
return resolved(layout_node, Length::make_px(reference_value)).to_px(layout_node); return resolved(layout_node, reference_value).to_px(layout_node);
} }
T resolved(Layout::Node const& layout_node, T const& reference_value) const Angle resolved(Layout::Node const& layout_node, Angle reference_value) const
{ {
return m_value.visit( return m_value.visit(
[&](T const& t) { [&](T const& t) {

7
Userland/Libraries/LibWeb/CSS/Size.cpp
View file

@ -16,12 +16,7 @@ Size::Size(Type type, LengthPercentage length_percentage)
CSSPixels Size::to_px(Layout::Node const& node, CSSPixels reference_value) const CSSPixels Size::to_px(Layout::Node const& node, CSSPixels reference_value) const
{ {
return m_length_percentage.resolved(node, CSS::Length::make_px(reference_value)).to_px(node); return m_length_percentage.resolved(node, reference_value).to_px(node);
}
CSS::Length Size::resolved(Layout::Node const& node, Length const& reference_value) const
{
return m_length_percentage.resolved(node, reference_value);
} }
CSS::Length Size::resolved(Layout::Node const& node, CSSPixels reference_value) const CSS::Length Size::resolved(Layout::Node const& node, CSSPixels reference_value) const

1
Userland/Libraries/LibWeb/CSS/Size.h
View file

@ -46,7 +46,6 @@ public:
bool is_none() const { return m_type == Type::None; } bool is_none() const { return m_type == Type::None; }
// FIXME: This is a stopgap API that will go away once all layout code is aware of CSS::Size. // FIXME: This is a stopgap API that will go away once all layout code is aware of CSS::Size.
CSS::Length resolved(Layout::Node const&, Length const& reference_value) const;
CSS::Length resolved(Layout::Node const&, CSSPixels reference_value) const; CSS::Length resolved(Layout::Node const&, CSSPixels reference_value) const;
[[nodiscard]] CSSPixels to_px(Layout::Node const&, CSSPixels reference_value) const; [[nodiscard]] CSSPixels to_px(Layout::Node const&, CSSPixels reference_value) const;

6
Userland/Libraries/LibWeb/CSS/Transformation.cpp
View file

@ -20,7 +20,7 @@ Transformation::Transformation(TransformFunction function, Vector<TransformValue
Gfx::FloatMatrix4x4 Transformation::to_matrix(Painting::PaintableBox const& paintable_box) const Gfx::FloatMatrix4x4 Transformation::to_matrix(Painting::PaintableBox const& paintable_box) const
{ {
auto count = m_values.size(); auto count = m_values.size();
auto value = [&](size_t index, CSS::Length const& reference_length = CSS::Length::make_px(0)) -> float { auto value = [&](size_t index, CSSPixels const& reference_length = 0) -> float {
return m_values[index].visit( return m_values[index].visit(
[&](CSS::LengthPercentage const& value) -> double { [&](CSS::LengthPercentage const& value) -> double {
return value.resolved(paintable_box.layout_node(), reference_length).to_px(paintable_box.layout_node()).to_float(); return value.resolved(paintable_box.layout_node(), reference_length).to_px(paintable_box.layout_node()).to_float();
@ -34,8 +34,8 @@ Gfx::FloatMatrix4x4 Transformation::to_matrix(Painting::PaintableBox const& pain
}; };
auto reference_box = paintable_box.absolute_rect(); auto reference_box = paintable_box.absolute_rect();
auto width = CSS::Length::make_px(reference_box.width()); auto width = reference_box.width();
auto height = CSS::Length::make_px(reference_box.height()); auto height = reference_box.height();
switch (m_function) { switch (m_function) {
case CSS::TransformFunction::Matrix: case CSS::TransformFunction::Matrix:

2
Userland/Libraries/LibWeb/Painting/PaintableBox.cpp
View file

@ -509,7 +509,7 @@ void paint_text_decoration(PaintContext& context, Layout::Node const& text_node,
auto line_color = text_node.computed_values().text_decoration_color(); auto line_color = text_node.computed_values().text_decoration_color();
CSSPixels css_line_thickness = [&] { CSSPixels css_line_thickness = [&] {
CSS::Length computed_thickness = text_node.computed_values().text_decoration_thickness().resolved(text_node, CSS::Length(1, CSS::Length::Type::Em)); CSS::Length computed_thickness = text_node.computed_values().text_decoration_thickness().resolved(text_node, CSS::Length(1, CSS::Length::Type::Em).to_px(text_node));
if (computed_thickness.is_auto()) if (computed_thickness.is_auto())
return max(glyph_height.scaled(0.1), 1); return max(glyph_height.scaled(0.1), 1);