210873 - Relative positioned float disregarded as containing block

Status: RESOLVED FIXED

(Core :: Layout: Positioned, defect, P2)

Description

[Olaf Zander]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4) Gecko/20030624
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4) Gecko/20030624

When opening the URL http://www.traffiq.de, several texts are written through
each other and therefore unreadable. In addition images are placed on top of
text, hiding the text.

Reproducible: Always

Steps to Reproduce:
browse to the URL http://www.traffiq.de




Expected Results:  
Text layed out correctly as displayed in MS-IE6.

Comment 1

[Frederic Bezies]

Well, it must be a web page design problem... If you look at code, you can see
it was created using MS-Word 10 aka Word XP... And reading source code is an
headache giver :-}

Maybe a problem related to css inside code page. W3C validator don't want to
check this page, saying : " Sorry, I am unable to validate this document because
on line 6  it contained one or more bytes that I cannot interpret as utf-8 (in
other words, the bytes found are not valid values in the specified Character
Encoding). Please check both the content of the file and the character encoding
indication."

Maybe a tech evangelism bug ?

Comment 2

[Shark Daddy (Jonathan L.)]

I think we can agree it's definitely not a layout bug. The CSS and proprietary
instructions are indeed headache-giving, to Opera as well (which gives the same
layout as Mozilla). I'm inclined to invalidate this, but I suppose Tech
Evangelism is appropriate.

Comment 3

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

->Evang (unless someone gives us a testcase showing it's a layout bug, but I
doubt that)

Comment 4

[Mathieu Pillard [:mat]]

Yes, evang. a quick test in the dom inspector tells me that the problem comes
from the line-height everywhere (set to 11pt, but the font-size is not specified...)

removing those line-height should fix it, but I dont have the time to test right
now.

Comment 5

[Mats Palmgren (inactive)]

AFAICT, this is a violation of CSS2.1, chap 10.1, which does not exclude relative
positioned floats from being the containing block for its abs. pos. children.

Comment 6

[Mats Palmgren (inactive)]

Attachment: Testcase #1

Testcase #1, distilled from http://www.traffiq.de in about 5 minutes without
giving me any headache at all.

Comment 7

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

Attachment: patch

This fixes the bug, but I need to investigate whether the two things in the XXX
comments are improvements or not.

Comment 8

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

Attachment: patch

This adds an argument to ConstructBlock for passing to CreateViewForFrame, and
removes ProcessBlockChildren, which I've long thought was a vestige of the old
way of handling blocks-within-inlines.	(There are some differences relating to
anonymous table objects, but I don't think there should be any problems, and
alpha is as good a time as any...)

Comment 9

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

Taking.

Comment 10

[Mats Palmgren (inactive)]

>   // The frame is also a block if it's an inline frame that's floated or
>   // absolutely positioned
>+  // XXXldb Doesn't nsRuleNode take care of this?

Yes it does, using |EnsureBlockDisplay|:
http://lxr.mozilla.org/seamonkey/source/content/base/src/nsRuleNode.cpp#2716

You can safely remove the following block:
   if ((NS_STYLE_DISPLAY_INLINE == aDisplay->mDisplay) &&
       (isFloating || aDisplay->IsAbsolutelyPositioned())) {
     isBlock = PR_TRUE;
   }

Just to be sure I added a printf in there and ran (most of) the top100 list -
no problem, it didn't trigger.

You can also remove this block, and the variable |isFloating|:
   if (NS_STYLE_FLOAT_NONE != aDisplay->mFloats) {
     isFloating = PR_TRUE;
   }

and use |aDisplay->IsFloating()| instead, it's only used in three places
and most likely only one of those paths will be taken.

Comment 11

[Mats Palmgren (inactive)]

This is actually a dupe of bug 67543, the testcase there renders fine with
the attached patch.

Comment 12

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

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

Comment 13

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

Attachment: patch

This should address Mats's comments, and also fixes EnsureBlockDisplay to do
the right thing for inner table display types.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 126718 [details] [diff] [review]
patch

>Index: content/base/src/nsRuleNode.cpp

> static void EnsureBlockDisplay(PRUint8& display)

Add a comment in here that basically says that fixup only needs to happen if
the display is not NONE and not one of the types listed in
nsStyleDisplay::IsBlockLevel.

>Index: layout/html/style/src/nsCSSFrameConstructor.cpp

>   // See if the frame is floated, and it's a block or inline frame
>-  else if (isFloating &&
>+  else if (aDisplay->IsFloating() &&
>            ((NS_STYLE_DISPLAY_BLOCK == aDisplay->mDisplay) ||
>             (NS_STYLE_DISPLAY_INLINE == aDisplay->mDisplay) ||
>             (NS_STYLE_DISPLAY_LIST_ITEM == aDisplay->mDisplay))) {

Didn't we just ensure that aDisplay->mDisplay != NS_STYLE_DISPLAY_INLINE?  That
check (and that part of the comment) can go, no?

>   // XXXbz should we be passing in a non-null aContentParentFrame?
>   nsHTMLContainerFrame::CreateViewForFrame(aPresContext, aNewFrame,
>-                                           aStyleContext, nsnull, PR_FALSE);
>+                                           aStyleContext, aContentParentFrame,
>+                                           PR_FALSE);

That XXX comment can go now... ;)

With those nits picked, r+sr=bzbarsky

Comment 15

[Mats Palmgren (inactive)]

You can remove INLINE here too:

  // See if the frame is absolute or fixed positioned
  else if (aDisplay->IsAbsolutelyPositioned() &&
           ((NS_STYLE_DISPLAY_BLOCK == aDisplay->mDisplay) ||
            (NS_STYLE_DISPLAY_INLINE == aDisplay->mDisplay) ||
            (NS_STYLE_DISPLAY_LIST_ITEM == aDisplay->mDisplay))) {

Comment 16

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

Fix checked in to trunk, 2003-06-30 14:48/55 -0700.