Last Comment Bug 210873 - Relative positioned float disregarded as containing block
: Relative positioned float disregarded as containing block
Status: RESOLVED FIXED
[patch]
: testcase
Product: Core
Classification: Components
Component: Layout: R & A Pos (show other bugs)
: Trunk
: All All
: P2 normal (vote)
: mozilla1.5alpha
Assigned To: David Baron :dbaron: ⌚️UTC-8
: Hixie (not reading bugmail)
: Jet Villegas (:jet)
Mentors:
http://www.traffiq.de
: 67543 (view as bug list)
Depends on:
Blocks: 175590 208684
  Show dependency treegraph
 
Reported: 2003-06-27 03:47 PDT by Olaf Zander
Modified: 2006-03-12 17:12 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Testcase #1 (987 bytes, text/html)
2003-06-27 14:42 PDT, Mats Palmgren (:mats)
no flags Details
patch (5.52 KB, patch)
2003-06-27 15:28 PDT, David Baron :dbaron: ⌚️UTC-8
no flags Details | Diff | Splinter Review
patch (16.51 KB, patch)
2003-06-27 15:51 PDT, David Baron :dbaron: ⌚️UTC-8
no flags Details | Diff | Splinter Review
patch (23.97 KB, patch)
2003-06-29 14:32 PDT, David Baron :dbaron: ⌚️UTC-8
bzbarsky: review+
bzbarsky: superreview+
Details | Diff | Splinter Review

Description Olaf Zander 2003-06-27 03:47:52 PDT
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 2003-06-27 04:58:50 PDT
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.) 2003-06-27 09:51:20 PDT
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-8 2003-06-27 11:02:56 PDT
->Evang (unless someone gives us a testcase showing it's a layout bug, but I
doubt that)
Comment 4 Mathieu Pillard [:mat] 2003-06-27 11:35:20 PDT
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 (:mats) 2003-06-27 14:39:43 PDT
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 (:mats) 2003-06-27 14:42:45 PDT
Created attachment 126645 [details]
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-8 2003-06-27 15:28:58 PDT
Created attachment 126649 [details] [diff] [review]
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-8 2003-06-27 15:51:33 PDT
Created attachment 126653 [details] [diff] [review]
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-8 2003-06-27 15:56:20 PDT
Taking.
Comment 10 Mats Palmgren (:mats) 2003-06-27 18:22:46 PDT
>   // 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 (:mats) 2003-06-27 22:18:30 PDT
This is actually a dupe of bug 67543, the testcase there renders fine with
the attached patch.
Comment 12 David Baron :dbaron: ⌚️UTC-8 2003-06-28 12:29:31 PDT
*** Bug 67543 has been marked as a duplicate of this bug. ***
Comment 13 David Baron :dbaron: ⌚️UTC-8 2003-06-29 14:32:26 PDT
Created attachment 126718 [details] [diff] [review]
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 [:bz] (still a bit busy) 2003-06-29 21:23:06 PDT
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 (:mats) 2003-06-29 23:11:52 PDT
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-8 2003-06-30 14:56:07 PDT
Fix checked in to trunk, 2003-06-30 14:48/55 -0700.

Note You need to log in before you can comment on or make changes to this bug.