273946 - spacing improperly rendered on spreadfirefox.com - top margin ignored

Status: RESOLVED FIXED

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

Description

[Ivan Ičin]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a6) Gecko/20041209 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a6) Gecko/20041209 Firefox/1.0+

The spacing between top box and log-in box does not exist in trunk build,
although it exists in 1.0 (and other browsers).

Reproducible: Always
Steps to Reproduce:
1. go to spreadfirefox.com
2. see the spacing between these two boxes
3.




I think I saw this on some more pages with less complicated CSS, but I didn't
write them down. I'll try to report on them later.

Comment 1

[Martijn Wargers (dead)]

Attachment: Testcase 1 - Quirk

Comment 2

[Martijn Wargers (dead)]

I don't see the bug in:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a5) Gecko/20041125 7:07 am
I can see the bug in:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a5) Gecko/20041125
Firefox/0.9.1+ 10:19am

This must be a regression from the fix of bug 209694.

Comment 3

[Martijn Wargers (dead)]

Attachment: Testcase 2 - Quirk

Slightly different case, here is the margin-top ignored from the body element.

Comment 4

[Adam Hauner]

Simple workaround is to set height or min-height for element with margins.

Comment 5

[Mathieu Pellerin]

wasn't that a reason why everybody bitched about IE, the lack of a good css
rendering engine. If we start thinking of not patching and creating workarounds
for a CSS property as basic as margin-top that is not working right, won't we
get as bad as IE ?? 

Comment 6

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

I'm not sure whether the behavior is incorrect according to CSS.  The
interaction of margin collapsing and floats is very complicated, and I'd rather
have Hixie or roc evaluate it.

Comment 7

[Mathieu Pellerin]

We can enter in the complexity of words and papers, or we can try to deal with
the obvious

a) The gecko engine never ignored the margin-top value in that kind of scenario
before. We are speaking of all the 0.x and 1.x releases, up to 1.8alpha;

b) Opera, Safari, internet explorer (might not be the best browser to refer to,
but it goes along the point I want to make) and other browsers aren't ignoring
this margin-top value.

just switching behavior on 'words interpretation' when on the other hand you
shipped so many versions of gecko before with the old behavior and that many
other browsers act the same way seems to me a bit ... odd.

I'm all for standards and sticking to official specs etc.etc. but not really on
breaking, for an unclear specification in the guideline or various stretched
interpretations, something that worked for so long in your own rendering engine
+ other 'respectable' browser (that statement now excludes IE ;) )

That being said, I never read the whole CSS 2.1 reference document; the behavior
might be crystal clear and worth applying in the gecko engine.

Comment 8

[Hixie (not reading bugmail)]

Both testcases are in quirks mode. I'll need a standards-mode testcase before I
can make a definite assessment. I'll have a moment in a few days to work on this.

Comment 9

[Adam Hauner]

Attachment: Testcase 1 - Std. mode

Comment 10

[Adam Hauner]

Attachment: Testcase 2 - Std. mode

Comment 11

[Hixie (not reading bugmail)]

Yes, those testcases IMHO show bugs.

Comment 12

[Asa Dotzler [:asa]]

too late for 1.8a6. blocking-

Comment 13

[Jasper]

This bug is causing many many websites on internet (including spreadfirefox,
apple and more) to look very different from what they used to look.

I tracked down the date on which it started to occur and that december 3 2004.
On  december 2 everything was fine.

it looks like december 2 was a bad day. This is the second major/blocking bug I
found that started on builds the day after. What happened, sabotage?

Comment 14

[Bill Gianopoulos [:WG9s]]

re: comment #13.  Perhaps it has been lost amongst the other comments on this
bug, but it has already been determined that this is a regression caused by the
checkin for bug 209694, so don't waste a lot of time trying to isolate a
regression window.

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The relevant language is:
> If the top and bottom margins of a box are adjoining, then it is possible for
> margins to collapse through it. In this case, the position of the element
> depends on its relationship with the other elements whose margins are being
> collapsed.
>
>     * If the element's margins are collapsed with its parent's top margin, the
> top border edge of the box is defined to be the same as the parent's.
>     * Otherwise, either the element's parent is not taking part in the margin
> collapsing, or only the parent's bottom margin is involved. The position of
> the element's top border edge is the same as it would have been if the element
> had a non-zero top border.

I agree that the first testcase shows a bug; 'columns' is not collapsing its top
margin with its parent's top margin, so we should treat 'columns' as if it had a
non-zero top border and not collapse through it.

However, I believe our rendering of the second testcase is correct. In this
testcase, BODY's top and bottom margins are adjoining so we invoke the above
paragraph. BODY's margins collapse with its parent's top margin, so we place the
top border-edge of BODY at the top border-edge of HTML, i.e., the top of the
viewport. The float is then also placed at the top of the viewport. Ian, correct
me if I'm wrong :-).

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Oh, never mind. Our rendering of the second testcase is indeed incorrect, because:

> # Margins of the root element's box do not collapse.

That will actually make this easier to fix :-).

Comment 17

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

It turns out that we don't really have to reset the y-coordinate of empty
blocks. If the empty block's top margin is collapsing with its parent's top
margin, then it will already be positioned in the right place.

We do have to fix nsBlockFrame::ShouldApplyTopMargin so that empty blocks with
clearance stop subsequent lines' top margins from collapsing with the parent.
We also want to collapse top and bottom margins for an empty block even if it
has clearance.

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in.

Comment 19

[Mats Palmgren (inactive)]

Attachment: Testcase #3

Robert, could you take a look at this testcase please.

The computed height of the outermost DIV is 200px - shouldn't it be 100px
and the float overflowing its lower border?
(the testcase comes from bug 274249 if you'll rather handle it there)