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

VERIFIED FIXED in mozilla1.9alpha1

Status

()

defect
P1
critical
VERIFIED FIXED
13 years ago
8 years ago

People

(Reporter: martijn.martijn, Assigned: bzbarsky)

Tracking

(Blocks 1 bug, 4 keywords)

Trunk
mozilla1.9alpha1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8.1.1 +
blocking1.8.0.9 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:nse] reveals 306663 testcase, crash signature)

Attachments

(3 attachments)

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?
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.
Posted file 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]
Summary: Crash [@ nsBlockBandData::Init] with unminimised stirdom mathml/xul testcase → Crash [@ nsBlockBandData::Init] [@ nsBlockBandData::~nsBlockBandData] with mathml/xul testcase
Posted patch Proposed fixSplinter Review
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....
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #240087 - Flags: superreview?(roc)
Attachment #240087 - Flags: review?(roc)
I think this is worth taking on branch too.
Flags: blocking1.8.1.1?
OS: Windows XP → All
Priority: -- → P1
Hardware: PC → All
Summary: Crash [@ nsBlockBandData::Init] [@ nsBlockBandData::~nsBlockBandData] with mathml/xul testcase → [FIX]Crash [@ nsBlockBandData::Init] [@ nsBlockBandData::~nsBlockBandData] with mathml/xul testcase
Target Milestone: --- → mozilla1.9alpha
(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.
Yeah, I'd like to do this on branches, then think about how to do this better on trunk, maybe.
Attachment #240087 - Flags: superreview?(roc)
Attachment #240087 - Flags: superreview+
Attachment #240087 - Flags: review?(roc)
Attachment #240087 - Flags: review+
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Attachment #240087 - Flags: approval1.8.0.9?
If people feel that this should go in the 1.8.0.8 or 1.8.1 releases, lemme know.
Whiteboard: [sg:nse] reveals 306663 testcase
Flags: blocking1.8.0.9?
Martijn-  Can you verify this fix?  Thx
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20061102 Minefield/3.0a1
Status: RESOLVED → VERIFIED
Attachment #240087 - Flags: approval1.8.1.1?
Flags: blocking1.8.1.1?
Flags: blocking1.8.1.1+
Flags: blocking1.8.0.9?
Flags: blocking1.8.0.9+
Comment on attachment 240087 [details] [diff] [review]
Proposed fix

Approved for 1.8.0/1.8 branches, a=dveditz for drivers
Attachment #240087 - Flags: approval1.8.1.1?
Attachment #240087 - Flags: approval1.8.1.1+
Attachment #240087 - Flags: approval1.8.0.9?
Attachment #240087 - Flags: approval1.8.0.9+
Fixed for 1.8.1, 1.8.0.9
I meant 1.8.1.1.
Keywords: fixed1.8.1fixed1.8.1.1
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).
Group: security
crash test landed
http://hg.mozilla.org/mozilla-central/rev/ad84a71b23b2
Flags: in-testsuite+
Crash Signature: [@ nsBlockBandData::Init] [@ nsBlockBandData::~nsBlockBandData]
You need to log in before you can comment on or make changes to this bug.