145531 - border applied inside a <hr/>

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Kai Lahmann (is there, where MNG is)]

If I have a <hr/> with height:5px;border-width:2px;, the border is applied
inside this height, as on MSIE's box-bug.

Comment 1

[Christopher Aillon (sabbatical, not receiving bugmail)]

Our rendering of <hr> is kinda screwy anyway IIRC.  Testcase in URL.  Confirming
on a week old linux CVS build.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Hmm.. <hr> claims to have -moz-box-sizing:content-box... that looks ok...

Comment 3

[Kai Lahmann (is there, where MNG is)]

-moz-border-sizing:border-box makes it even worse, the <hr/> has no even the
given height and is still way smaler, then a <div> with the same settings

Comment 4

[Chris Petersen]

Changing QA Contact to Moied

Comment 5

[Boris Zbarsky [:bzbarsky]]

In case anyone cares, the problem is the weirdness that is nsHRFrame::Paint(). 
In particular, note that it paints completely inside mBorderPadding -- that's wrong.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

I've been looking at this, and I have a patch that seems to fix it. The problem
was that nsCSSRendering::PaintBorder expected the full rect in which it could
paint its border in. In all cases, it was receiving a height of |mThickness| and
a y0 of (mRect.height - mThickness) / 2. This was causing it to paint the 100px
borders in a space of 200px, shrinking the actual hr to a mere pixel. The change
is a bit more complex than just changing HRuleFrame::Paint() to give it the
correct height of |mRect.height|. This is because if |mThickness| + 2px was
smaller than a "line" (as determined by the font size), |mRect.height| would
grow bigger than |mThickness|. In this case, we still needed to use the old
method of painting. My patch adds an internal flag to |HRuleFrame| which
indicates which size it should use for painting.

On another note, I've never seen |mBorderPadding.anything| be anything other
than 0. My patch doesn't use it because of this, is there any case in which
mBorderPadding can change?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Ian's patch in bug 38370 should fix this by making the abomination that is
nsHRFrame go away....

And mBorderPadding is nonzero if the styles are set accordingly and content-box
sizing is used....

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

Because that patch is better, and mine may still have problems, might as well
leave this bug alone and wait for that bug to be fixed.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Fixed by bug 38370