54980 - <hr> rendering too tall

Status: VERIFIED FIXED

(Core :: Layout, defect, P2)

Description

[Simon Fraser [no longer active]]

In the current PR3 builds, all <hr>s come out too tall. A regular <hr> with no 
height or CSS looks like it's 3-4 pixels tall, where 4.x renders it as 1 px tall.

Comment 1

[buster]

yes, I see a difference between the debug build on my machine and the PR3
release build.  I'll try to track it down.

Comment 2

[buster]

dammit, this is caused by me not getting the final version of my fix for bug
53568 in before beta3 deadline.  I have the fix in hand, and I didn't see the
bug myself because it ain't in my tree. (And I plain didn't notice doing my
dogfood testing.)
I'll attach the patch, get another review, approval, etc.  Trivial change to
make this right.
Karnaze and waterson, can you guys look over the diff I'm about to attach.
You've seen it already, this is just the cleaned-up version (the version that
checks the border width rather than using a hardcoded value.)

Comment 3

[buster]

Attachment: fix

Comment 4

[buster]

chris and chris:
The attached patch does 2 things:
1) it moves the prior fix up into the if (NS_UNCONSTRAINEDSIZE !=
aReflowState.mComputedHeight) case, so default sizing is uneffected.  This is
primarily what Simon was complaining about here.  I'm a dope for missing that
for beta3.
2) it adds in waterson's very reasonable request for basing the adjustment on
the actual top and bottom border, rather than hardcoding in the observed default
value.

The diff is a little hard to read because of (1).  But it's just moving a block
of code up into the "if" clause so the calculation in the "else" clause isn't
effected.

Comment 5

[karnaze (gone)]

r=karnaze

Comment 6

[Chris Waterson]

My only nit would be to move "PRBool noShadeAttribute = GetNoShade()" into the
"if" statement because it's only ever used "if (thickness != onePixel)". Your
call. sr=waterson

Comment 7

[buster]

adding rtm+ per karnaze's verbal approval to put on PDT radar.
very low risk, gives us much better 4xp on many top100 pages.  very visible problem.

Comment 8

[Phil Peterson]

PDT marking [rtm++] this week, but would be minus real soon now. Let's focus on
crashers.

Comment 9

[buster]

fix checked into branch

Comment 10

[buster]

checked into tip as well

Comment 11

[Chris Petersen]

marking verified in the Oct 6th build.