From decc071060a900a802c0b4b8d34add2125a4337f Mon Sep 17 00:00:00 2001
From: Shannon Booth <shannon@serenityos.org>
Date: Sun, 8 Oct 2023 10:55:51 +1300
Subject: [PATCH] LibWeb: Return a scroll offset of 0 for colgroup elements

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.
---
 .../expected/scroll-left-and-top-on-colgroup.txt |  2 ++
 .../input/scroll-left-and-top-on-colgroup.html   | 13 +++++++++++++
 Userland/Libraries/LibWeb/DOM/Element.cpp        | 16 ++++++++++++++++
 3 files changed, 31 insertions(+)
 create mode 100644 Tests/LibWeb/Text/expected/scroll-left-and-top-on-colgroup.txt
 create mode 100644 Tests/LibWeb/Text/input/scroll-left-and-top-on-colgroup.html

diff --git a/Tests/LibWeb/Text/expected/scroll-left-and-top-on-colgroup.txt b/Tests/LibWeb/Text/expected/scroll-left-and-top-on-colgroup.txt
new file mode 100644
index 0000000000..3f514e04d2
--- /dev/null
+++ b/Tests/LibWeb/Text/expected/scroll-left-and-top-on-colgroup.txt
@@ -0,0 +1,2 @@
+     scroll left = 0
+scroll top = 0
diff --git a/Tests/LibWeb/Text/input/scroll-left-and-top-on-colgroup.html b/Tests/LibWeb/Text/input/scroll-left-and-top-on-colgroup.html
new file mode 100644
index 0000000000..7c2c0464d1
--- /dev/null
+++ b/Tests/LibWeb/Text/input/scroll-left-and-top-on-colgroup.html
@@ -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>
diff --git a/Userland/Libraries/LibWeb/DOM/Element.cpp b/Userland/Libraries/LibWeb/DOM/Element.cpp
index 7d62a0b592..133209b503 100644
--- a/Userland/Libraries/LibWeb/DOM/Element.cpp
+++ b/Userland/Libraries/LibWeb/DOM/Element.cpp
@@ -1085,8 +1085,16 @@ double Element::scroll_top() const
     if (!layout_node() || !is<Layout::Box>(layout_node()))
         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.
     // FIXME: Is this correct?
+    VERIFY(paintable_box());
     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()))
         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.
     // FIXME: Is this correct?
+    VERIFY(paintable_box());
     return paintable_box()->scroll_offset().x().to_double();
 }