386401 - regressions related to scrollable rowgroups

Status: RESOLVED WORKSFORME

(Core :: Layout: Tables, defect, P1)

Description

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

I think there are a bunch of regressions related to scrollable rowgroups from the reflow branch, because I wasn't thinking about them when I wrote a lot of the code.  In bug 364512 comment 6, I wrote:

> if the table is IsGeometryDirty, we'll need to mark the row group (and
> scroll frame containing it?) as NS_FRAME_HAS_DIRTY_CHILDREN to make
> the reflow propagate down to it.
> 
> We may also need to propagate the mHResize flag the way
> nsTableRowGroupFrame::ReflowChildren does, although maybe we should
> replace that propagation with SetGeometryDirty.

Testcases needed.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Making P1 because of the uncertain scope of this bug. If we break it down into specific fixes with testcases, we could reduce the priority.

Comment 2

[Mike Schroepfer]

Roc - assigning to you - please bounce to someone else if need be..

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I was hoping that David would take this one, since he understands what needs to be done.

Comment 4

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

Attachment: testcases for reftests

So it turns out that my fear of additional bugs seems to be incorrect, largely because the standard optimization (checking dirty bits)that I expected to be in nsHTMLScrollFrame::Reflow is not present.  In other words, HTML scroll frames always propagate reflow through, which means there's nothing to fix here.

This patch adds the three testcases I wrote to check this to reftests.  I'll land it once the tree opens and then mark the bug worksforme.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

awesome!

Comment 6

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

Testcases checked in to trunk.  Marking worksforme.

Comment 7

[Daniel Holbert [:dholbert]]

The reference cases for this bug had an accidental onload="run()", which did nothing except cause these warnings to be printed to stderr when running reftests:
> JavaScript error: file:///scratch/work/builds/mozilla-central/mozilla-central.09-06-15.19-37/mozilla/layout/reftests/bugs/386401-1-ref.html, line 1: run is not defined
> JavaScript error: file:///scratch/work/builds/mozilla-central/mozilla-central.09-06-15.19-37/mozilla/layout/reftests/bugs/386401-2-ref.html, line 1: run is not defined
> JavaScript error: file:///scratch/work/builds/mozilla-central/mozilla-central.09-06-15.19-37/mozilla/layout/reftests/bugs/386401-3-ref.html, line 1: run is not defined

Fixed by removing the onload handler from reference cases:
http://hg.mozilla.org/mozilla-central/rev/a4dde15acca6