118501 - vertical-align:top aligns top of box not loose subtree [INLINE]

Status: RESOLVED FIXED

(Core :: Layout: Block and Inline, defect, P1)

Description

[Hixie (not reading bugmail)]

STEPS TO REPRODUCE
   Vertical-align an inline element top, then insert a baseline inline element 
   in the first with a bigger line height. The top of the line box should align
   with the top of the inner inline element according to dibm.
      http://mozilla.org/docs/web-developer/quirks/

ACTUAL RESULTS
   We currently implement the non-sensical CSS2 per-spec behaviour, namely, the
   outer (top aligned) inline is aligned with the top of the line box, meaning
   the line box doesn't contain the whole line.

See: http://lists.w3.org/Archives/Member/w3c-css-wg/2002JanMar/0021.html

Since our current behaviour is technically correct, this is a minor bug.

Comment 1

[Hixie (not reading bugmail)]

Er. The first URI above was supposed to be:
   http://www.people.fas.harvard.edu/~dbaron/css/2000/01/dibm

Comment 2

[Hixie (not reading bugmail)]

Attachment: Test case: There should be no red visible.

View the source of the test case for some more explanatory comments with
diagrams, showing where the logical line boxes should fall.

Comment 3

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

*** Bug 118151 has been marked as a duplicate of this bug. ***

Comment 4

[Hixie (not reading bugmail)]

Our behaviour recently become wrong. The spec is being changed to match David's
description. This bug is therefore now valid.

Comment 5

[Greg K.]

Reconfirmed using FizzillaCFM/2002071208. Setting All/All.

Comment 6

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

*** Bug 168927 has been marked as a duplicate of this bug. ***

Comment 7

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

Taking.

Comment 8

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

*** Bug 247536 has been marked as a duplicate of this bug. ***

Comment 9

[Wayne Mery (:wsmwk)]

does bug 364961 comment 5 look to be a dupe?

Comment 10

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

Attachment: patch 1: don't duplicate PlaceTopBottomFrames to add 3 lines to it

This removes duplication of the function I need to modify for the real fix (with the addition of 3 lines of code for the VALIGN_OTHER case), and instead just does 2 separate loops.

Comment 11

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

Attachment: patch 2: vertical-align the subtree

This is the fix (now that the main part needs to be made in only one place).

I still need to write reftest, both for this bug, and for other vertical alignment behavior.

Comment 12

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

Attachment: patch 1: don't duplicate PlaceTopBottomFrames to add 3 lines to it

Comment 13

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

Attachment: patch 1: don't duplicate PlaceTopBottomFrames to add 3 lines to it

Comment 14

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

Attachment: patch 2: vertical-align the subtree

Now with a test added that covers a good bit.  Firefox before this patch fails exactly the four test items within it that are labeled with this bug number and passes the rest; with the patch the test passes.

Comment 15

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

http://hg.mozilla.org/mozilla-central/index.cgi/rev/fd7f8513fa8c
http://hg.mozilla.org/mozilla-central/index.cgi/rev/851965dfe20d
-> fixed (for 1.9.1)

Comment 16

[Jeff Walden [:Waldo]]

<http://mxr.mozilla.org/mozilla-central/source/layout/reftests/box-properties/CSS21-t100801-vertical-align.xhtml> should hopefully work whenever anyone gets around to reading this; it's "not found" right now.