405186 - [FIX]"ASSERTION: Have parent context and shouldn't" with tables, XBL

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Attachment: testcase

Loading the testcase triggers:

###!!! ASSERTION: Have parent context and shouldn't: 'Error', file /Users/jruderman/trunk/mozilla/layout/base/nsFrameManager.cpp, line 834
frame: TableCell(td)(-1) (0x2538720) style: 0x2538630 {}
Has parent context:  style: 0x25381b4 {}
Should be null

This might be related to bug 405184, but note that the assertion there is a DOM assertion rather than a style system assertion.

Comment 1

[Jesse Ruderman]

See also bug 404635, which triggers the same assertion as this testcase.

Comment 2

[Boris Zbarsky [:bzbarsky]]

This is kind of bad.  Basically, the flattened tree looks like this:

  <table>
    <div style="position: fixed">
      <tr>
        <td>

and the DOM looks like this:

  <table>
    <tr>
      <td>


Then we remove the <td> from the DOM.  But GetPrimaryFrameFor() returns null, since we don't add internal table elements to the primary frame map, and FindPrimaryFrameFor doesn't walk through placeholders, so can't find the primary frame for the <tr>: it starts the walk at the content parent (the <table>), but needs to end up under the fixed-pos descendant (the <div>) and never does.  I wonder whether we can walk through placeholders somehow without making something like nested floats O(2^N) (due to walking both the float childlist and the placeholder).  Perhaps we should do a walk similar to the walk that frame destruction does?   That is, walk through placeholders and skip walking certain frame lists?

Comment 3

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

Walk through placeholders and skip the out-of-flow (abspos and float) lists you mean? That sounds reasonable

Comment 4

[Boris Zbarsky [:bzbarsky]]

Yeah, skip float, abspos, fixed lists.  Then we can back out the patch we landed for bug 403245 too...

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Works great, less filling

It's sooo tempting to try to clean up this code.  Maybe for moz2.  :(

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 290342 [details] [diff] [review]
Works great, less filling

This fixes a range of cases where the frames for a node would not get removed when it got removed from the DOM, leading to crashes and the like.

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 290342 [details] [diff] [review]
Works great, less filling

a=beltzner for drivers

Comment 8

[Boris Zbarsky [:bzbarsky]]

Checked in, including reftest.  We still need to land the assertion test.

Comment 9

[Jesse Ruderman]

Crashtest checked in.