1564308 - {inc}visibility:collapse code makes some adjustments multiple times when the insides of a table aren't reflowed

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

While debugging test failures from the work in bug 1351924 I found one of the failures is an independent, already-existing bug. I think the basic problem is that the visibility:collapse adjustments made by the table code if we skip the main part of nsTableFrame::Reflow and then call AdjustForCollapsingRowsCols, since AdjustForCollapsingRowsCols is not idempotent.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: testcase

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The above testcase is a variant of layout/tables/reftests/1031934.html, which is a test that failed with the patch.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a task, but please change it back in case of error.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: Bug 1564308 - Add reftest. r=dholbert

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: Bug 1564308 - Always reflow all row groups and rows if we're doing visibility:collapse adjustments. r=dholbert

Depends on D37466

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Here's a try run.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

And a new try run, since it turns out the assertion actually fired in a crashtest that was testing large widths. (It showed some existing NS_WARN_IF_FALSE failures about the same thing.)

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cb68677410a5
Add reftest.  r=dholbert
https://hg.mozilla.org/integration/autoland/rev/514565e608cb
Always reflow all row groups and rows if we're doing visibility:collapse adjustments.  r=dholbert

Comment 9

[Dorel Luca [:dluca]]

Backed out 2 changesets (bug 1564308) for Crashtest failures in reftest/tests/layout/generic/crashtests/421671.html

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=255636071&repo=autoland&lineNumber=70756

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=514565e608cba4d1c309dfd146c8d01746d62651

Backout:
https://hg.mozilla.org/integration/autoland/rev/b3964512ee41c14a9f9e50d22e7fd180336323a8

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a2f96a3cba12
Add reftest.  r=dholbert
https://hg.mozilla.org/integration/autoland/rev/5382e671e265
Always reflow all row groups and rows if we're doing visibility:collapse adjustments.  r=dholbert

Comment 11

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/a2f96a3cba12
https://hg.mozilla.org/mozilla-central/rev/5382e671e265