Closed Bug 18754 Opened 21 years ago Closed 20 years ago

[FLOAT] problems with HRs around floats

Categories

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

Product:
Core
Component:
Layout
Type:
defect

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: marshall, Assigned: buster)

References

Details

(Keywords: testcase, Whiteboard: [nsbeta3+][PDTP2] WG)

Attachments

(2 files)

Test case
104 bytes, text/html
Details
proposed fix (still needs some extra commenting and minor cleanup)
7.42 KB, patch
Details | Diff | Splinter Review 
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
Assignee: troy → kipp
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.
Attached file Test case — 
*** Bug 18559 has been marked as a duplicate of this bug. ***
*** Bug 20466 has been marked as a duplicate of this bug. ***
*** Bug 21499 has been marked as a duplicate of this bug. ***
Whiteboard: [TESTCASE]
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.
Updating to default Layout Assignee...kipp no longer with us :-(
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.
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.
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
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.
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.
moving all buster m15 bugs to m16.
Target Milestone: M15 → M16
*** Bug 34921 has been marked as a duplicate of this bug. ***
*** Bug 35788 has been marked as a duplicate of this bug. ***
Blocks: 34892
mass-moving all remaining M16 bugs to M17
Target Milestone: M16 → M17
Adding nsbeta2 keyword since there are a lot of duplicates.
Assignee: buster → beppe
Keywords: nsbeta2
Correcting mistake and reassigning back to buster.
Assignee: beppe → buster
[nsbeta2-]
Whiteboard: [TESTCASE] → [nsbeta2-] [TESTCASE]
Priority: P3 → P1
This bad <HR> behavior appears to be gone in the M16 build for Linux.

*** Bug 34892 has been marked as a duplicate of this bug. ***
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-]
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
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
mine
Severity: normal → major
Status: NEW → ASSIGNED
Hopefully, can be fixed soon. Moving to P2.  Adding [PDTP2]
Priority: P1 → P2
Whiteboard: [nsbeta2-][nsbeta3+] → [nsbeta2-][nsbeta3+][PDTP2]
I am soooooo close on this one!
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.)
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.
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
Blocks: 51961
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
Closed: 20 years ago
Resolution: --- → FIXED
Whiteboard: [nsbeta3+][PDTP2][fix in hand] WG → [nsbeta3+][PDTP2] WG
Blocks: 20582
Marking verified in the Sept 12th build.
Status: RESOLVED → VERIFIED
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;
>             }
>         }
>     }
> }


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.