LibWeb: Fix table overflow issues

[ebanner]

Fix several table-related overflow issues.

Changes

1. Include vertical border spacing in row groups offset calculation so that it's axis-aligned with child row/cell elements:

This makes it so there isn't horizontal and vertical overflow caused by child row/cell elements.

2. Include horizontal border spacing in tr width calculations:

This makes it so tr elements don't have overflow anymore when there are multiple columns.

3. Apply vertical caption offset to row group top offset:

4. Don't double-count top padding when calculating vertical offset:

Fixes https://github.com/LadybirdBrowser/ladybird/issues/1525

Failing tests

• [x] css-pseudo-element-should-not-be-affected-by-presentational-hints.html
• [x] zero_percent_width_nested_table.html
• [x] table-fixup-with-rowspan.html
• [x] table/basic.html
• [x] table/width-distribution-and-constrained-columns-size-on-col.html
• [x] table/row-outer-size-with-computed-size.html
• [x] table/border-attribute.html
• [x] table/inline-table-width.html
• [x] table/cell-with-max-width.html
• [x] table/propagate-style-update-to-wrapper.html
• [x] table/borders.html
• [x] table/width-distribution-and-constrained-columns-increased-size-on-col.html
• [x] table/width-distribution-and-constrained-columns.html
• [x] table/table-width.html
• [x] table/border-spacing-with-percentage-width.html
• [x] table/stretch-to-fixed-height.html
• [x] table/multi-line-cell.html
• [x] table/columns-width-distribution-1.html
• [x] table/border-spacing-and-borders-table-width.html
• [x] table/style-invalidation-propagation-to-table-wrapper.html
• [x] table/border-spacing-colspan.html
• [x] table/bottom-caption.html
• [x] table/internal-alignment.html
• [x] table/table-cellpadding.html
• [x] table/nested-table-box-width.html
• [x] table/colgroup-with-two-cols.html
• [x] table/percentage-width-for-nested-table-is-like-auto.html
• [x] table/long-caption-increases-width.html
• [x] table/rowspan.html
• [x] table/border-attribute-overridden-by-css.html
• [x] table/top-caption-with-padding.html
• [x] table/nested-table-alignment.html
• [x] table/table-align-center-with-margin.html
• [x] table/tr-valign.html
• [ ] table/avoid-div-by-zero-in-table-measures.html
  • Rebaselining → tr+tbody is going from 0 width to 2 width
• [x] table/cell-relative-to-specified-table-width.html
• [x] table/td-valign.html
• [x] table/cell-auto-max-width-table-percentage-width.html
• [x] table/row-span-and-nested-tables.html
• [x] table/border-spacing.html
• [x] table/table-max-content-width.html
• [x] table/border-spacing-rowspan.html
• [x] table/table-align-center.html
• [x] table/abspos-pseudo-element-inside-table.html
  • Rebaselining - weird case
• [x] table/keyword-value-does-not-constrain-column.html
• [x] table/sum-of-percentage-column-widths-less-than-100.html
• [x] blockify-layout-internal-box-without-crashing.html
• [x] grid/floating-table-wrapper-width.html

[ladybird-bot]

Hello!

One or more of the commit messages in this PR do not match the Ladybird code submission policy, please check the lint_commits CI job for more details on which commits were flagged and why. Please do not close this PR and open another, instead modify your commit message(s) with git commit --amend and force push those changes to update this PR.

[AtkinsSJ]

The results are a big improvement and the code looks fine to me (though I'm not a layout person :sweat_smile: ) but of course a lot of tests still need rebaselining. You should be able to mass-update them with ./Meta/ladybird.sh run headless-browser --run-tests "./Tests/LibWeb" --rebaseline.

[AtkinsSJ]

I just noticed that you've described in each of the LibWeb: Fix test commits how the test was modified. You don't need to do that. :sweat_smile: Please just squash all the test-fixes into the commit that makes the change, so that all tests pass before and after that commit. Basically, I think this should all be one commit.

[ebanner]

Yeah, I am planning on squashing them into a single commit - I just wanted to keep a running log where I verify and gut-check for myself whether each test is erroneously failing before I modify the test.

Just so I understand though - there will probably still be some tests that are legitimately failing (e.g. table/nested-table-box-width.html) in the end (not because of this change, but because they are incorrect (?) tests). Are you saying I should change them to make them pass anyway just to get all the tests to pass?

[AtkinsSJ]

Just so I understand though - there will probably still be some tests that are legitimately failing (e.g. table/nested-table-box-width.html) in the end (not because of this change, but because they are incorrect (?) tests). Are you saying I should change them to make them pass anyway just to get all the tests to pass?

All tests should pass always. If a test starts failing because its expectation is wrong, rebaseline it. If it starts failing otherwise, then that's a bug that should be fixed. I don't quite understand what you mean about a test being incorrect. :sweat_smile:

[ebanner]

Sounds good 👍

If a test['s]... expectation is wrong

That's what I meant by a test being incorrect.

[AtkinsSJ]

I haven't checked but I believe this will also fix #1510.

[ebanner]

Yeah, specifically point number 2 in the list should fix that.

[ebanner]

Ok made it through fixing and rebaselining the tests - ready for review!

[ebanner]

Thanks for checking!

All the cases you mentioned, the overflow is either legitimate or there's an upstream bug (in the case of the colspan example).

[AtkinsSJ]

Great! Thanks for the explanations. :^)

[ebanner]

Actually, I was totally wrong with those rowspan examples! The TD elements actually shouldn't produce overflow on the rows since the rows aren't the containing block for them (the table is).

https://github.com/LadybirdBrowser/ladybird/pull/1268 should fix those.