403657 - [FIX]HTML list with empty second entry renders second line on top of first line

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Ville Pohjanheimo (inactive)]

Attachment: testcase w ordered list

An un/ordered list renders incorrectly when the second entry is empty. ie. the HTML list:

1. Some text
2.
3.

renders with the first and second line partially over each other. Test case included with and ordered list. Same behaviour with an unordered list.

Tested with most recent build: Build identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2pre) Gecko/2007111304 Minefield/3.0b2pre

2.0.0.9 doesn't have this problem.

Comment 1

[Ria Klaassen (not reading all bugmail)]

Regression range for this is: http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1169516400&maxdate=1169525519
Caused by Bug 367332?

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: testcase that shows we're misplacing the empty list items

Comment 3

[Damon Sicore (:damons)]

+'ing w/ P4.  Dbaron, please adjust priority if needed.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Raised to P3; we had a lot of complaints when we had similar bugs in releases in the past.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed fix

We were actually placing the block parts of the list-items just fine.  But we were positioning the bullet with its baseline at the top of the list-item.  That doesn't really make much sense.  Either we should put the baseline at the bottom (hard, because we haven't computed final size yet) or we should just leave the bullet right below the top padding (what this patch does).

That's all in the case when the list items has no lines, of course; if it has one, we'll use its baseline.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 290848 [details] [diff] [review]
Proposed fix

r+sr=dbaron.

I'm presuming the bullet influences the height of the first line if it needs to, so that this will guarantee the bullet doesn't overflow (unless, say, the bullet's text overflows its frame).

Though it almost seems like if a bullet contributes to the height of the block, it should be returned by GetFirstLineBaseline (or GetLastLineBaseline), even if the block is otherwise empty.  Maybe that deserves a followup bug?

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(Or would that be an easy fix now?)

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

And a reftest would be good -- comparing a bullet on an empty block to a bullet next to an  , maybe?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Checked in, with reftest.

Filed bug 406512 on our IRC conversation.

Comment 10

[Boris Zbarsky [:bzbarsky]]

The reftest fails on Mac.  The empty <li>s are not as tall as the ones with an &nbsp; in them.  :(

I disabled it for now; not sure how to write a good reftest here...