beenull/ladybird
1
0
Fork 0
mirror of https://github.com/LadybirdBrowser/ladybird.git synced 2024-11-29 19:10:26 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

LibWeb: Return a scroll offset of 0 for colgroup elements

Browse source 
Ideally we would not create a layout node at all for these elements so
that every layout node would always have a paintable associated with it.
But for now, to fix the crash, just leave a FIXME and special case this
element.

Also leave a VERIFY to make it easier to debug this type of crash in the
future.

Fixes a crash seen on codecov.io for my 'patch' project.
This commit is contained in:
Shannon Booth 2023-10-08 10:55:51 +13:00 committed by Alexander Kalenik
parent eb1c99bd72
commit decc071060
Notes: sideshowbarker 2024-07-16 23:34:44 +09:00 
Author: https://github.com/shannonbooth
Commit: https://github.com/SerenityOS/serenity/commit/decc071060
Pull-request: https://github.com/SerenityOS/serenity/pull/21373
Reviewed-by: https://github.com/kalenikaliaksandr
3 changed files with 31 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
Tests/LibWeb/Text/expected/scroll-left-and-top-on-colgroup.txt Normal file
View file

@ -0,0 +1,2 @@
scroll left = 0
scroll top = 0

13
Tests/LibWeb/Text/input/scroll-left-and-top-on-colgroup.html Normal file
View file

@ -0,0 +1,13 @@
<table>
<colgroup id="my-group">
<col width="40">
<col></colgroup>
<tbody>
<script src="include.js"></script>
<script>
test(() => {
const element = document.getElementById('my-group');
println(`scroll left = ${element.scrollLeft}`);
println(`scroll top = ${element.scrollTop}`);
});
</script>

16
Userland/Libraries/LibWeb/DOM/Element.cpp
View file

@ -1085,8 +1085,16 @@ double Element::scroll_top() const
if (!layout_node() || !is<Layout::Box>(layout_node())) if (!layout_node() || !is<Layout::Box>(layout_node()))
return 0.0; return 0.0;
// FIXME: Ideally we would stop creating a layout node for column group so that a layout node would always have
// a paintable, but in the meantime, special case this node.
if (layout_node()->display().is_table_column_group()) {
VERIFY(!paintable_box());
return 0.0;
}
// 9. Return the y-coordinate of the scrolling area at the alignment point with the top of the padding edge of the element. // 9. Return the y-coordinate of the scrolling area at the alignment point with the top of the padding edge of the element.
// FIXME: Is this correct? // FIXME: Is this correct?
VERIFY(paintable_box());
return paintable_box()->scroll_offset().y().to_double(); return paintable_box()->scroll_offset().y().to_double();
} }
@ -1125,8 +1133,16 @@ double Element::scroll_left() const
if (!layout_node() || !is<Layout::Box>(layout_node())) if (!layout_node() || !is<Layout::Box>(layout_node()))
return 0.0; return 0.0;
// FIXME: Ideally we would stop creating a layout node for column group so that a layout node would always have
// a paintable, but in the meantime, special case this node.
if (layout_node()->display().is_table_column_group()) {
VERIFY(!paintable_box());
return 0.0;
}
// 9. Return the x-coordinate of the scrolling area at the alignment point with the left of the padding edge of the element. // 9. Return the x-coordinate of the scrolling area at the alignment point with the left of the padding edge of the element.
// FIXME: Is this correct? // FIXME: Is this correct?
VERIFY(paintable_box());
return paintable_box()->scroll_offset().x().to_double(); return paintable_box()->scroll_offset().x().to_double();
} }