[FLOAT] problems with HRs around floats

VERIFIED FIXED in M18

Status

()

Core
Layout
P2
major
VERIFIED FIXED
19 years ago
17 years ago

People

(Reporter: Neil Marshall, Assigned: buster)

Tracking

({testcase})

Trunk
testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta3+][PDTP2] WG)

Attachments

(2 attachments)

(Reporter)

Description

19 years ago
I have an image (The first thing inside of the body tag) setup like this:
<img src="myimage.gif" width="185" height="591" align="left">

Then I have numerous <p></p>'s and a horizontal rule.  Now it comes up fine, but
when I resize the image, most of the text flows around it, but there are some
lines of text which do not.

For example, this line flows around it perfectly
<p><font face="Arial, Helvetica, sans-serif"><b>November 11, 1999</b></font></p>
while this one
<p><font face="Arial, Helvetica, sans-serif"><b>October 30, 1999</b></font></p>
places itself right on top of the image.  I personally don't see much of a
difference in the 2

<hr align="center">
The horizontal rule, when I resize the page, also ignores the image and centers
itself on the page, as opposed to in the room that it has been given.

Build: 1999111208

Updated

19 years ago
Assignee: troy → kipp

Comment 1

18 years ago
The text is rendered fine for me (build 1999-12-12-08 on Win98), but the problem
with the HR tag remains. I'll attach a simplified test case for this.

Comment 2

18 years ago
Created attachment 3436 [details]
Test case

Comment 3

18 years ago
*** Bug 18559 has been marked as a duplicate of this bug. ***

Comment 4

18 years ago
*** Bug 20466 has been marked as a duplicate of this bug. ***

Comment 5

18 years ago
*** Bug 21499 has been marked as a duplicate of this bug. ***

Updated

18 years ago
Whiteboard: [TESTCASE]

Updated

18 years ago
Summary: Text doesn't flow around an image all the time → HR flow around an image
Summary: HR flow around an image → problems with HRs around floats
Bug 18559 has references to historical bugs.

Comment 7

18 years ago
Updating to default Layout Assignee...kipp no longer with us :-(

Comment 8

18 years ago
Why are you re-reassing layout bugs? Do NOT touch layout bugs.

The bugs are assigned to Kipp so they can stay neatly organized until we have a
new owner for the block/inline code.
(Assignee)

Comment 9

18 years ago
mass moving all Kipp's pre-beta bugs to M15.  Nisheeth and I will
prioritize these and selectively move high-priority bugs into M13 and M14.
(Assignee)

Updated

18 years ago
Summary: problems with HRs around floats → [FLOAT] problems with HRs around floats
Bulk moving [testcase] code to new testcase keyword. Sorry for the spam!
Keywords: testcase

Comment 11

18 years ago
This behavior is apparant with any <foo align=foo><hr> elements. See
http://www.dzm.com/ to see the <hr> overlap a table that is align=right.
(Assignee)

Comment 12

18 years ago
mine! mine mine mine!  all mine!  whoo-hoo!
Assignee: kipp → buster
http://www.w3.org/Style/CSS/Test/current/sec414.htm shows some interesting
behavior of HR's around floats.

Comment 14

18 years ago
moving all buster m15 bugs to m16.
Target Milestone: M15 → M16
(Assignee)

Comment 15

18 years ago
*** Bug 34921 has been marked as a duplicate of this bug. ***

Comment 16

18 years ago
*** Bug 35788 has been marked as a duplicate of this bug. ***

Updated

18 years ago
Blocks: 34892
(Assignee)

Comment 17

18 years ago
mass-moving all remaining M16 bugs to M17
Target Milestone: M16 → M17

Comment 18

18 years ago
Adding nsbeta2 keyword since there are a lot of duplicates.
Assignee: buster → beppe
Keywords: nsbeta2

Comment 19

18 years ago
Correcting mistake and reassigning back to buster.
Assignee: beppe → buster

Comment 20

18 years ago
[nsbeta2-]
Whiteboard: [TESTCASE] → [nsbeta2-] [TESTCASE]
(Assignee)

Updated

18 years ago
Priority: P3 → P1

Comment 21

18 years ago
This bad <HR> behavior appears to be gone in the M16 build for Linux.
(Assignee)

Comment 22

18 years ago
*** Bug 34892 has been marked as a duplicate of this bug. ***

Comment 23

18 years ago
The code attached by Silvester Erdeg -
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=3436
is in M16 still incorrect.

So is the code I submitted as #20466

M16 for Linux, Build ID: 2000061311

Mozilla/5.0 (X11; U; Linux 2.2.17pre9 i686; en-US; m16) Gecko/20000613
I think we should get this sorted out for beta3.  It's got a lot of dups.
Keywords: nsbeta3
*** Bug 22563 has been marked as a duplicate of this bug. ***
*** Bug 25839 has been marked as a duplicate of this bug. ***
*** Bug 46625 has been marked as a duplicate of this bug. ***
Keywords: correctness
Whiteboard: [nsbeta2-] [TESTCASE] → [nsbeta2-]

Comment 28

18 years ago
OS field should be changed to 'All' I think..

Another page with a lot of floating HR occurances:
http://kontek.net/pp/
OS: Windows 98 → All
Hardware: PC → All

Comment 29

18 years ago
Approving for beta3 based on number of dups and the fact that HR use is so 
common.
Whiteboard: [nsbeta2-] → [nsbeta2-][nsbeta3+]
Target Milestone: M17 → M18
(Assignee)

Comment 30

18 years ago
mine
Severity: normal → major
Status: NEW → ASSIGNED

Comment 31

18 years ago
Hopefully, can be fixed soon. Moving to P2.  Adding [PDTP2]
Priority: P1 → P2
Whiteboard: [nsbeta2-][nsbeta3+] → [nsbeta2-][nsbeta3+][PDTP2]
(Assignee)

Comment 32

18 years ago
I am soooooo close on this one!
(Assignee)

Comment 33

18 years ago
This is a mess.  HR is a block element. In Nav4 and IE5, however, it acts like 
an inline element when it is positioned next to a float.  That is, it is 
sized and positioned taking full account of the floater geometry.  This is in 
direct contradition to the CSS2 spec, which says block elements are laid out as 
if floaters did not exist. (someone could quote the spec here for me!)

But, a ton of pages make use of this behavior.

So, I suggest a quirks mode fix.  See if you can shoot any holes in this idea.  
It's truly unsavory, but it's pretty low risk and preliminary testing looks 
hopeful.

The basic idea is to make HR inline, rather than block, in quirks mode.  Use 
generated content to insert newlines before and after the HR, giving it the 
behavior of a block element.  Finally, add some magic to make it align like a 
block even though it's an inline.

The fix has 3 parts:
1) Implement generated content on HR's (in fact, on all leaf nodes.)  Generated 
content is currently impleneted on containers, but not leaf nodes.  This is 
contrary to the CSS2 spec, and I can't find a good reason for the omission.  
It's a separate issue about whether we want to advertise support for this 
feature in Netscape 6, I'll submit a separate bug on that. 

2) In quirks.css, make HR display:inline.  Add HR:before and HR:after rules with 
content("/A") and whitespace:pre.  Also make HR box-sizing:border-box; since the 
edge of an HR is drawn as a border, though really it's the content isn't it?  
Finally, make HR a "replaced" element (I don't see anything in the CSS spec that 
says HR's can't be considered "replaced" elements!  dbaron?)

3) Add code to nsLineLayout::HorizontalAlignFrames() that detects the situation 
of an HR on a line (we know it must be the only thing on the line), and tinker 
with the alignment attribute to get left, right, and center based on the style 
context for the HR element.

Feedback welcome.  Patches to be attached soon (maybe not until tomorrow am.)
(Assignee)

Comment 34

18 years ago
I have the fix as described below.  The question for dbaron, ian, and others 
interested in CSS2 compliance is:  should the be quirks mode only, or should it 
behave this way in strict mode as well?
Whiteboard: [nsbeta2-][nsbeta3+][PDTP2] → [nsbeta3+][PDTP2][fix in hand]
*** Bug 37589 has been marked as a duplicate of this bug. ***
I guess it's going to have to be strict mode too, although I really don't like
saying that.

Can you make a new value for the 'display' property which triggers this magic
code? -moz-hr or something? I really would hate to know what will happen if 
someone says 
   HR { display: block; }
...or
   HR:before { content: ' / '; } 
   HR:after { content: ''; }
   HR { display: inline; border; none; margin: 0; padding: 0; }
...or whatever.
(Assignee)

Comment 37

18 years ago
Created attachment 13841 [details] [diff] [review]
proposed fix (still needs some extra commenting and minor cleanup)
(Assignee)

Comment 38

18 years ago
yeah, I know what you mean.  that's why I'd argue for it to be quirks mode only.
in standard mode, the user can get what they're looking for by other means using 
standard (non-icky) css boxes.  For example, you could stack left-floating 
boxes...
<style>
div {
  float:left; height:100px;
}
<div style="width:100px; border: 1px solid black;">left div</div>
<div>auto-width div<hr width=100%></div>

there must be a better way....
How possible would it be to use a new value on 'display'?
Whiteboard: [nsbeta3+][PDTP2][fix in hand] → [nsbeta3+][PDTP2][fix in hand] WG
(Assignee)

Updated

18 years ago
Blocks: 51961
(Assignee)

Comment 40

18 years ago
fix checked in 9/11/00
r=karnaze
Ian: a new value of display would be possible, given more time.  But that's time 
I don't have this week.  If you feel strongly, a separate bug should be opened 
on that solution.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
Whiteboard: [nsbeta3+][PDTP2][fix in hand] WG → [nsbeta3+][PDTP2] WG
(Assignee)

Updated

18 years ago
Blocks: 20582

Comment 41

18 years ago
Marking verified in the Sept 12th build.
Status: RESOLVED → VERIFIED

Comment 42

17 years ago
I'm looking into bug 81776, which appears to have been caused by the fixes for
this bug. Why were these changes to nsLineLayout required?

> PRUint32 textAlign = mTextAlign;
> // here is where we do special adjustments for HR's 
> // see bug 18754
> if (!InStrictMode()) {
>   if (psd->mFirstFrame && psd->mFirstFrame->mFrame)
>     {
>       nsCOMPtr<nsIAtom> frameType;
>       psd->mFirstFrame->mFrame->GetFrameType(getter_AddRefs(frameType));
>       if (nsLayoutAtoms::hrFrame == frameType.get()) {
>         // get the alignment from the HR frame
>         {
>           const nsStyleSpacing* spacing;
>           psd->mFirstFrame->mFrame->GetStyleData(eStyleStruct_Spacing,
>                                          (const nsStyleStruct*&)spacing);
>           textAlign = NS_STYLE_TEXT_ALIGN_CENTER;
>           nsStyleCoord zero(nscoord(0));
>           nsStyleCoord temp;
>           if ((eStyleUnit_Coord==spacing->mMargin.GetLeftUnit()) &&
>               (zero==spacing->mMargin.GetLeft(temp)))
>             {
>               textAlign = NS_STYLE_TEXT_ALIGN_LEFT;
>             }
>           else if ((eStyleUnit_Coord==spacing->mMargin.GetRightUnit()) &&
>                    (zero==spacing->mMargin.GetRight(temp))) {
>               textAlign = NS_STYLE_TEXT_ALIGN_RIGHT;
>             }
>         }
>     }
> }

Comment 43

17 years ago
I believe there was a time when neither of these bugs existed because I have a 
site affected by them, and it was okay for a while.. I don't think bug 81776 
came from the patch for this one.
You need to log in before you can comment on or make changes to this bug.