If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

"ASSERTION: How did our kid's height change if nothing was dirty?"

RESOLVED WORKSFORME

Status

()

Core
Layout: Tables
RESOLVED WORKSFORME
11 years ago
10 years ago

People

(Reporter: Jesse Ruderman, Unassigned)

Tracking

(Blocks: 1 bug, {assertion, testcase})

Trunk
assertion, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

11 years ago
Created attachment 255421 [details]
testcase

Loading this testcase triggers:

###!!! ASSERTION: How did our kid's height change if nothing was dirty?: 'oldVisibleHeight == GetScrolledFrame()->GetSize().height', file /Users/jruderman/trunk/mozilla/layout/forms/nsListControlFrame.cpp, line 667

Last month, bz fixed another bug that triggered the same assertion (bug 366537).
This seems like a table or scrollframe bug to me... at the end of the first reflow (or rather at the beginning of the second one), the frame tree inside that dropdown has:

Area(select)(3)@0xb1843dc [view=0xb185690] {120,60,59820,0} [state=00946008] [overflow=0,0,59820,1020] sc=0xb184ae8(i=1,b=1) pst=:-moz-scrolled-content<
  line 0xb18b030: count=1 state=block,clean,prevmarginclean,not impacted,not
  wrapped,before:nobr,after:nobr[0x2088] {0,0,59820,0} ca={0,0,540,1020} <
    TableOuter(select)(3)@0xb185028 {0,0,59820,0} [state=00054008]
    [content=0xb186960] [overflow=0,0,540,1020] [sc=0xb184f7c]
    pst=:-moz-table-outer<
      Table(select)(3)@0xb185070 {0,0,540,0} [state=00054008] [content=0xb186960]
      [overflow=0,0,540,1020] [sc=0xb184e80] pst=:-moz-table<
        HTMLScroll(option)(1)@0xb18514c [view=0xb185748] {0,0,540,1020}
        [state=00046000] [content=0xb076cf8] [sc=0xb1894e0]<

Note that heights -- the inner table has a height of 0, as does the outer table, etc.  But the scrollframe inside there has a height of 1020!  That's the height everything in sight ends up with after the reflow, and the height it should be to start with, no?
Actually, looks like a tables bug and a reflow branch regression.  The first time through nsTableFrame::Reflow collapsing the rowgroup decreases our height, but the second time it does not.  Probably because nsTableRowFrame::CollapseRowIfNecessary munges the row height _and_ the cell heights, so the next time we come through we get a 0 for the height to collapse out.

Not sure where the 1020 height on the second reflow comes from, but the second time around we definitely do NOT call nsTableRowFrame::Reflow.  In fact, we don't even reflow the rowgroup.  In nsTableFrame::Reflow, we aren't dirty and don't have dirty kids, so we skip reflowing all that stuff.  Then we call nsTableFrame::CalcDesiredHeight and recompute our height.  But this doesn't look at the _real_ rowgroups.  It looks at the scrollframes containing those rowgroups (which makes sense).  But the scrollframe has its original rect, because the collapsing happens from inside nsTableRowGroupFrame!  Which probably actually means we collapse out too much if the rowgroup actually has to scroll, but anyway...

Anyway, that inconsistency is what we end up asserting about in the end.
Blocks: 300030
Component: Layout → Layout: Tables
OS: Mac OS X → All
QA Contact: layout → layout.tables
Hardware: PC → All
(Reporter)

Comment 3

10 years ago
WFM, Mac trunk.

bz, does anything from your analysis (comment 1 and comment 2) need to become a separate bug?
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → WORKSFORME
Uh... could be.  The behavior here changed because bug 367706 changed CalcDesiredHeight look at the actual rowgroups, not at the scrollframes.  I'm not sure that's a correct change, as I said in that bug.  Bernd, do you know what that function is trying to do?  Can we somehow get it to screw up?

Comment 5

10 years ago
I am not sure that the mechanism for collapsing both for rows, row groups and col and col groups is compatible with the reflow branch change at all. And more sadly I have no idea how to implement this correctly.
Uh... That doesn't actually answer my question.  My question is what the mechanism is trying to accomplish, in particular if a rowgroup is scrollable what it should be doing.  Once we have that figured out, we can worry about incremental reflow.  ;)
Bernd and I talked about this.  Basically, bug 367706 caused a regression.  That's bug 387344.  Once that's fixed, this bug will reappear.

I filed bug 387343 on the underlying problem described in comment 2.
Depends on: 387343
(Reporter)

Comment 8

10 years ago
Crashtest checked in.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.