550889 - "ASSERTION: Invalid computed height: 'aComputedHeight >= 0'" with negative margins and display:block in XUL

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Markus Stange [:mstange]]

Attachment: testcase

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a3pre) Gecko/20100307 Minefield/3.7a3pre

Loading the testcase causes nsFrame::BoxReflow to pass negative values to SetComputedHeight.

I'm hitting this assertion in bug 547787 because I'm using ::before and ::after with negative margins. (I need to set display: block on them because display: -moz-box will give me the warning "XUL box for _moz_generated_content_before element contained an inline #text child, forcing all its children to be wrapped in a block.")

Comment 1

[Markus Stange [:mstange]]

Attachment: testcase

the other testcase was wrong

Comment 2

[Boris Zbarsky [:bzbarsky]]

> Loading the testcase causes nsFrame::BoxReflow to pass negative values to
> SetComputedHeight.

Sounds like a bug in nsFrame::BoxReflow.  Want to fix?

Comment 3

[Markus Stange [:mstange]]

I'm not sure how to. Why does it add the margins to the width / height at all?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Possibly because in the XUL box model there's really no good place to put the margins?  One of the neils might know better...

Comment 5

[Markus Stange [:mstange]]

Neil, do you know what to do here?

Comment 6

[Markus Stange [:mstange]]

Ah, some lines further down there's this code:

 computedHeight = NS_MAX(computedHeight, 0);
 reflowState.SetComputedHeight(computedHeight); 

Should we just do the same here?

Comment 7

[Boris Zbarsky [:bzbarsky]]

That should generally be done at SetComputedHeight callsites that can't guarantee non-negative numbers, yes.

Comment 8

[Markus Stange [:mstange]]

Attachment: v1

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 438255 [details] [diff] [review]
v1

r=bzbarsky.  Sorry for the terrible lag here...

Comment 10

[Markus Stange [:mstange]]

http://hg.mozilla.org/mozilla-central/rev/985cdfad1c7e