354133 - [FIX]Crash [@ nsBlockBandData::Init] [@ nsBlockBandData::~nsBlockBandData] with mathml/xul testcase

Status: VERIFIED FIXED

(Core :: Layout, defect, P1)

Comment 3

[Boris Zbarsky [:bzbarsky]]

Weird... I couldn't reproduce this in my debug build, and figured out that _removing_ the 

  *, * > *|* {
    position: static !important;
  }


rule in mathml.css makes this bug go away.  Presumably because something in the testcase ends up being positioned and hence not actually a child of the mathml element.

We could try reducing the testcase, or we could try to actually fix mathml once and for all.  rbs, which MathML frame types are allowed to contain blocks?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Although... Any block being created when there's no space manager around should end up with one.  Except this is a _really_ fucked up frame tree (a block is a child of a nsMathMLTokenFrame is a child of a nsScrollbarFrame is a child of the root element's block frame.  And nsScrollbarFrame is a reflow root, so there's no space manager in sight when we do the incremental reflow.

This we probably need to handle in block frame construction.  Of course I'd think that a reflow root would cut off the search for float containing blocks too.  It sure should.

Comment 5

[Martijn Wargers (dead)]

Attachment: testcase

Ok, I managed to minimize it to this. This gives a slightly different stacktrace.
Talkback ID: TB23746451W
nsBlockBandData::~nsBlockBandData  [mozilla\layout\generic\nsblockbanddata.cpp, line 60]
nsBlockFrame::Reflow  [mozilla\layout\generic\nsblockframe.cpp, line 803]

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed fix

This should be self-explanatory; we're just doing what we do in ConstructBlock().

But maybe this should be done for all blocks whose parent is not a float containing block?  Blocks already do something similar for the box-wrapped case....

Comment 7

[Boris Zbarsky [:bzbarsky]]

I think this is worth taking on branch too.

Comment 8

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

(In reply to comment #6)
> But maybe this should be done for all blocks whose parent is not a float
> containing block?  Blocks already do something similar for the box-wrapped
> case....

Yeah.

This patch is OK though.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Yeah, I'd like to do this on branches, then think about how to do this better on trunk, maybe.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Fixed.

Comment 11

[Boris Zbarsky [:bzbarsky]]

If people feel that this should go in the 1.8.0.8 or 1.8.1 releases, lemme know.

Comment 14

[Tim Riley [:timr]]

Martijn-  Can you verify this fix?  Thx

Comment 15

[Martijn Wargers (dead)]

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20061102 Minefield/3.0a1

Comment 16

[Daniel Veditz [:dveditz]]

Comment on attachment 240087 [details] [diff] [review]
Proposed fix

Approved for 1.8.0/1.8 branches, a=dveditz for drivers

Comment 17

[Boris Zbarsky [:bzbarsky]]

Attachment: Branch version of patch

Comment 18

[Boris Zbarsky [:bzbarsky]]

Fixed for 1.8.1, 1.8.0.9

Comment 19

[Boris Zbarsky [:bzbarsky]]

I meant 1.8.1.1.

Comment 20

[Jay Patel [:jay]]

v.fixed on 1.8.0 and 1.8.1 branches with 
Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.9pre) Gecko/20061128 Firefox/1.5.0.9pre 
and 
Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.1pre) Gecko/20061128 BonEcho/2.0.0.1pre

No crashes with either testcase (does crash FF2.0).

Comment 21

[Bob Clary [:bc] (inactive)]

crash test landed
http://hg.mozilla.org/mozilla-central/rev/ad84a71b23b2