Relative positioned float disregarded as containing block

RESOLVED FIXED in mozilla1.5alpha

Status

()

Core
Layout: R & A Pos
P2
normal
RESOLVED FIXED
14 years ago
12 years ago

People

(Reporter: Olaf Zander, Assigned: dbaron)

Tracking

({testcase})

Trunk
mozilla1.5alpha
testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [patch], URL)

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

14 years ago
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

14 years ago
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 ?
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.
(Assignee)

Comment 3

14 years ago
->Evang (unless someone gives us a testcase showing it's a layout bug, but I
doubt that)
Assignee: other → german
Component: Layout → German
Product: Browser → Tech Evangelism
QA Contact: ian → german
Version: Trunk → unspecified
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 5

14 years ago
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.
Assignee: german → position
Component: German → Layout: R & A Pos
Keywords: testcase
OS: Windows XP → All
Product: Tech Evangelism → Browser
QA Contact: german → ian
Summary: several texts written through each other and images placed on top of text --> unreadable → Relative positioned float disregarded as containing block
Version: unspecified → 1.0 Branch

Comment 6

14 years ago
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.
(Assignee)

Comment 7

14 years ago
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.
(Assignee)

Comment 8

14 years ago
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...)
Attachment #126649 - Attachment is obsolete: true
(Assignee)

Updated

14 years ago
Attachment #126653 - Flags: superreview?(roc+moz)
Attachment #126653 - Flags: review?(roc+moz)
(Assignee)

Comment 9

14 years ago
Taking.
Assignee: position → dbaron
Priority: -- → P2
Hardware: PC → All
Whiteboard: [patch]
Target Milestone: --- → mozilla1.5alpha
Version: 1.0 Branch → Trunk
>   // 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.
This is actually a dupe of bug 67543, the testcase there renders fine with
the attached patch.
(Assignee)

Comment 12

14 years ago
*** Bug 67543 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 13

14 years ago
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.
Attachment #126653 - Attachment is obsolete: true
(Assignee)

Updated

14 years ago
Attachment #126718 - Flags: superreview?(bzbarsky)
Attachment #126718 - Flags: review?(bzbarsky)
(Assignee)

Updated

14 years ago
Attachment #126653 - Flags: superreview?(roc+moz)
Attachment #126653 - Flags: review?(roc+moz)
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
Attachment #126718 - Flags: superreview?(bzbarsky)
Attachment #126718 - Flags: superreview+
Attachment #126718 - Flags: review?(bzbarsky)
Attachment #126718 - Flags: review+
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))) {
(Assignee)

Comment 16

14 years ago
Fix checked in to trunk, 2003-06-30 14:48/55 -0700.
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
(Assignee)

Updated

14 years ago
Blocks: 208684
(Assignee)

Updated

12 years ago
Blocks: 175590
You need to log in before you can comment on or make changes to this bug.