hr height < 0 - crash in printing, assert on display




17 years ago
17 years ago


(Reporter: jesup, Assigned: Marc Attinasi)


({crash, regression})

crash, regression

Firefox Tracking Flags

(Not tracked)





17 years ago
FreeBSD 4.1 20010105xx

Fresh pull; went to this site, registered (so it shows a list of digital
stations (sets a cookie so you don't need to register again), then clicked on
the print button.  Requester came up, told it to print, boom.

The bug appears to be in nsHRFrame.cpp line 210:

    thickness = aReflowState.mComputedHeight;
    // fix up thickness based on noshade and mode.  see bug 53568 and 54980
    if (eCompatibility_NavQuirks == mode)
      nscoord adjustment = + 
      thickness += adjustment;  // adjust for -moz-bg-inset 
      PRBool noShadeAttribute = GetNoShade();
      if (thickness != onePixel)
        if (!noShadeAttribute) { // this makes us compatible with Nav4, and one
pixel taller than IE5
          thickness += onePixel;

In this case, the thickness is -14; which is apparently from the moz-bg-inset.
mode is eCompatibility_Standard, which is why this doesn't get corrected.  In
any case, however, the size should not be negative!  I think there's a problem
with moz-bg-inset or it's use for HR's, or perhaps there shouldn't be a mode
test.  See the bugs referenced above (bug 53568 and bug 54980).

Here are comments from the end of 53568.  It appears my worries about it only
being compensated for in quirks mode are valid.

------- Additional Comments From Randell Jesup 2000-10-05 11:33 -------

Appears fixed on trunk FreeBSD 20001004xx

I looked at the patch - it appears to compensate for -moz-bg-inset within the HR
code.  Is that the "correct" fix, or is -moz-bg-inset inherently incorrect?
Also, it looked as if the code only compensated in quirks mode.  Is that right?

There's still a bug in rendering of noshade HR's with the endcaps, but that's
another issue.  I think someone got way too fancy with making the rounded

I'm REALLY glad that it got fixed for the PR3 release - Not fixing it would have
been asking to be pilloried.  I'm also glad I spent the time to track it through
the code.

------- Additional Comments From 2000-10-05 11:41 -------

Randell:  I'm glad you pushed on this one, too!
You are correct, I compensate for the inset computation inside the HR reflow
code in quirks mode.  That's what quirks mode is all about. This is to account
for Nav's bizzare sizing algorithm with HR heights, and Nav/IE's odd treatment
of HR's with respect to floaters.  Basically, legacy behavior is to treat an HR
as a block element unless it's impacted by a floater, in which case it's treated
like an inline element.  That's very tough for a CSS-compliant browser to
This isn't necessary in standard mode at all, where HR's are block elements,

Comment 1

17 years ago
Added crash, regression
Severity to major
Severity: normal → major
Keywords: crash, regression

Comment 2

17 years ago
FreeBSD 4.1 20010306xx re-verified bug exists (assertion on display, didn't try
crashing it on print)

Assertions (10 of them for height < 0) can be seen at

###!!! ASSERTION: Negative Height Result- very bad: 'mComputedHeight>=0', file
nsHTMLReflowState.cpp, line 2444

(height was -14 again, due to moz-bg-inset).  Note that this page is NOT marked
as standard (though it does use some simple styles).

Severity->critical (it's a crasher)
Changed subject

Related issue: on Linux ns4.7, shaded (normal) HR's are all 1 pixel too tall.  I
suspect this code: 

if (!noShadeAttribute) { // this makes us compatible with Nav4, and one pixel
taller than IE5
    thickness += onePixel;

We're one pixel taller than ns4.7 (Linux).  I suspect we're one pixel taller
than ns 4.x and ie5 on all platforms, and that this if should go away.

Again, I suggest that we remove this test:
    // fix up thickness based on noshade and mode.  see bug 53568 and 54980
    if (eCompatibility_NavQuirks == mode)

I think that the border thickness (moz-bg-inset) needs to be compensated for in
all cases.
Severity: major → critical
OS: FreeBSD → All
Summary: Printing crashes mozilla due to hr height < 0 → hr height < 0 - crash in printing, assert on display

Comment 3

17 years ago
reassigning to attinasi and marking m0.9.
Target Milestone: --- → mozilla0.9

Comment 4

17 years ago
Reassigning owner
Assignee: clayton → attinasi

Comment 5

17 years ago
I think that bug 71210 covers the remaining issue here (the assert, which has
been commented out for now since the layout code now handels it correctly).
There is no longer a crash printing AFAICT. Please re-open is you are still
seeing the crash on a current build - thanks!

*** This bug has been marked as a duplicate of 71210 ***
Last Resolved: 17 years ago
Resolution: --- → DUPLICATE

Comment 6

17 years ago
Verified dup.
You need to log in before you can comment on or make changes to this bug.