1692836 - [wpt-sync] Sync PR 27631 - [TablesNG] clusterfuzz multicol causes illegal table layout trees

Status: RESOLVED FIXED

(Core :: Layout: Tables, task, P4)

Description

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Sync web-platform-tests PR 27631 into mozilla-central (this bug is closed when the sync is complete).

PR: https://github.com/web-platform-tests/wpt/pull/27631
Details from upstream follow.

b'Aleks Totic <[e-mail]>' wrote:

[TablesNG] clusterfuzz multicol causes illegal table layout trees

The cause of the crash is legacy LayoutTableColumn inside
LayoutNGTable. This triggers bad typecast, wrong methods get called...

I am not sure what the correct fix is,
my knowledge of usage of flags that force Legacy layout is
limited, and that code seems very unforgiving.

I've added a DCHECK that traps the error before random
memory overwrite.

I've minimized a testcase. This is my understanding of what went wrong:

The dom tree looks like this at start.

START IN MULTICOL

fieldset MC (Multicol)
span MC
embed
table MC
colgroup

MUTATION:
Span gets cloned and insterted into colgroup.

colgroup
span.cloned

span.cloned Element::UpdateForceLegacyLayout gets called.
This calls ForceLegacyLayoutInFormattingContext, which traverses
ancestors, and calls:
colgroup.SetShouldForceLegacyLayoutForChild()

END WITHOUT MULTICOL

Multicol styles get removed from the document.
Colgroup still has SetShouldForceLegacyLayoutForChild set to true.

But, COLGROUP did not force legacy originally because its SpecifiesColumns()
was false. So it never calls UpdateForceLegacyLayout(), and its flag
SetShouldForceLegacyLayoutForChild never gets reset.

When COLGROUP LayoutObject gets created, it is forced to Legacy because
of this flag, gets inserted into LayoutNGTable and asserts.

My "fix" is really just a hack because I do not understand the framework.
I believe mstensho might know the correct way to fix this.

Bug: 1178263
Change-Id: Ifebaaccbad4006819ca66fed8310ddeb59e1aa64
Reviewed-on: https://chromium-review.googlesource.com/2692390
WPT-Export-Revision: d7f5b3e6e21fcf52411929148db52f84c3a0b209

Comment 1

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Pushed to try (stability) https://treeherder.mozilla.org/#/jobs?repo=try&revision=9b35009f8000ca26b66c3aaa38a495675a7cec85

Comment 2

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

CI Results

Ran 14 Firefox configurations based on mozilla-central, and Firefox, Chrome, and Safari on GitHub CI

Total 1 tests

Status Summary

Firefox

PASS: 1

Chrome

PASS: 1

Safari

PASS: 1

Links

Gecko CI (Treeherder)
GitHub PR Head
GitHub PR Base

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/68a51f6b2e24
[wpt PR 27631] - [TablesNG] clusterfuzz multicol causes illegal table layout trees, a=testonly

Comment 4

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/68a51f6b2e24