Small test case for bug in renderer floats

Wed Aug 7 21:24:31 BST 2002

Hi,

Ok, what goes wrong at Google, after I applied the patch below (which is
in CVS now) is this test case; a floating table and a div.

  <html><body style="border:1px solid gray" ;bgcolor=#ffffff>
    <p>
    <table border=0 cellpadding=0 cellspacing=4 align=right width=20% bgcolor=#d
dffdd>
      <tr><td><a href=""><b>Bold</b>anchor</a>
          <br><br>Some text in floating table.
      </td></tr></table>
    <div>
      Some text in div. Some text in div. Some text in div.
      Some text in div. Some text in div.
      <br>
      Some text in div. Some text in div. Some text in div.
      Some text in div. Some text in div.
      <br>
      Some text in div. Some text in div. Some text in div.
      Some text in div. Some text in div.
    </div>
  </body>
</html>

The floating table can go over the text in the div. Also it behaves
erratically, if the mouse hovers the link.
This seems to work correct without my patch, however if you resize khtml
so that the table becomes overhanging, the original non-aligned-right-
border bug shows it self.
Damn, is this stuff touchy :(.
Anyway, there is a third option :) Call RenderFlow::addOverHangingFloats
not with '-child->xPos()' but with '-child->xPos() + child->marginLeft()'
as xoffset param at line 455.

Dirk, I also thought this second option make sense but it breaks google. I
attached the diff for option number three.

Vadim, as you may understand, I'm not going to backport it yet.

Regards,

Koos

 On Thu, 1 Aug 2002, Koos Vriezen wrote:

> On Mon, 29 Jul 2002, Koos Vriezen wrote:
>
> > > |  > On Sunday 28 July 2002 5:15 pm, Koos Vriezen wrote:
> > > |  > |  This is rendered wrong
> > > |  > |    <html><body>
> > > |  > |     <div style="float:left;" border=1>div text<br>div text</div>
> > > |  > |     some text
> > > |  > |    </body></html>
> > > |  > |
> > > |  > |  Seems the float gets an extra left offset if the height is greater
> > > |  > | than height of surrounding text.
>
> > Exactly and I think I found the cause. The test in
> > RenderFlow::layoutBlockChildren of RenderHtml
> > 'if ( child->hasOverhangingFloats() )', where
> > hasOverhangingFloats() is defined as 'floatBottom() > m_height', succeeds
> > in this case. The child being the body.
> > Next it calls RenderFlow::addOverHangingFloats with
> > child, -child->xPos(), -child->yPos() and true for flow, xoff, offset and
> > child resp.
> > A new special is created with 'special->left' is set to 'r->left - xoff'
> > (becomes +10). The test 'if (flow != parent())' succeeds and
> > 'special->left += flow->marginLeft()' is executed (becomes +20).
> > The next test 'if ( !child )' fails, but has no effect, since the if
> > clause 'special->left -= marginLeft()' does nothing for a RenderHtml.
> >
> > Not sure what goes wrong here, but this makes the float be indented by the
> > body margin twice. First guess would be call
> > RenderFlow::addOverHangingFloats with child, 0, -child->yPos() and true.
> > Also helps for http://www.linuxplanet.com/linuxplanet/reviews/4315/1
> > reported by Hetz Ben Hamo on 18 July to this list.
>
> Hmmm, this breaks http://msdn.microsoft.com/. Could also be that
> rendering/render_flow.cpp:1000
>                 if (flow != parent())
>                     special->left += flow->marginLeft();
> should be
>                 if (flow == parent())
>                     special->left += flow->marginLeft();
> Works for the above and msdn.
>
> Koos
>
>
-------------- next part --------------
Index: rendering/render_flow.cpp
===================================================================
RCS file: /home/kde/kdelibs/khtml/rendering/render_flow.cpp,v
retrieving revision 1.279
diff -u -3 -p -r1.279 render_flow.cpp

--- rendering/render_flow.cpp	2002/08/04 14:23:23	1.279
+++ rendering/render_flow.cpp	2002/08/07 20:21:10
@@ -452,7 +452,7 @@ void RenderFlow::layoutBlockChildren( bo
 
 	if ( child->hasOverhangingFloats() ) {
 	    // need to add the float to our special objects
-	    addOverHangingFloats( static_cast<RenderFlow *>(child), -child->xPos(), -child->yPos(), true );
+	    addOverHangingFloats( static_cast<RenderFlow *>(child), -child->xPos() + child->marginLeft(), -child->yPos(), true );
 	}
 
         child = child->nextSibling();
@@ -997,7 +997,7 @@ void RenderFlow::addOverHangingFloats( R
 		special->startY = r->startY - offset;
 		special->endY = r->endY - offset;
 		special->left = r->left - xoff;
-		if (flow == parent())
+		if (flow != parent())
 		    special->left += flow->marginLeft();
 		if ( !child ) {
 		    special->left -= marginLeft();
