Bug 52880 - [testcase] table floating at wrong position
Summary: [testcase] table floating at wrong position
Status: RESOLVED FIXED
Alias: None
Product: konqueror
Classification: Applications
Component: khtml renderer (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Konqueror Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-01-11 15:59 UTC by lenar
Modified: 2007-01-31 08:46 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments
testcase (305 bytes, text/html)
2003-06-14 17:44 UTC, Kai Lahmann
Details

Note You need to log in before you can comment on or make changes to this bug.
Description lenar 2003-01-11 15:59:26 UTC
Version:            (using KDE KDE 3.0.99)
Installed from:    Compiled From Sources
OS:          Linux

Consider following html:

<html>
<body>
  <table>
  <tr><td>
  <table width="100" height="200" border="1" style="float: right">
    <tr><td>table floating to right</td></tr>
  </table>
  <table width="400" height="100" border="1">
    <tr><td>not so floating table</td></tr>
  </table>
  some text ...
  </td></tr>
  </table>
</body>
</html>

Table floating to right overlaps with table not floating at all. It should not.
Please fix.
Comment 1 Kai Lahmann 2003-06-14 16:44:46 UTC
not a bug. This is correct behavior. 
Comment 2 lenar 2003-06-14 17:39:50 UTC
So you want to say that mozilla does this the wrong way (and IE of 
course, but that's not a good example I suppose). In mozilla those tables 
do not overlap. And my logic says it's the correct behavior. I'm not 
saying: 'position:absolute' or anything like that, which might result in 
behavior like in konqueror now. 
 
Comment 3 Kai Lahmann 2003-06-14 17:44:18 UTC
Created attachment 1802 [details]
testcase

sorry, haven't seen the normal table running into the floated one...
Comment 4 Kai Lahmann 2003-06-14 17:46:36 UTC
and so need to confirm it 
Comment 5 lenar 2003-10-17 16:28:59 UTC
Actually it seems when the contents changes unneccessary repaints are happening.
Even if the resultcount remains the same (was www. and after keypress www.e). Seems that box is cleared and then new string added back. It should be done so that only one repaint is needed (with new image and without visible all-clearing step).

But it's still better than before. Thanks.
Comment 6 lenar 2003-10-17 16:29:45 UTC
Oh. Wrong bug report. Sorry.
Comment 7 Allan Sandfeld 2006-06-08 15:32:48 UTC
Actually it's a quirk in mozilla, they add the floating width to the default width of a table-cell if it floats right. 

They don't when it floats left.

I am not sure we should even fix it. It's inconsistent, and not standard.
Comment 8 Germain Garand 2007-01-31 08:46:35 UTC
SVN commit 628748 by ggarand:

adapt float minMaxWidth calculation change by David Hyatt <hyatt@apple.com>. 
now considers flowAroundFloats, takes better care of clearing. 
BUG: 52880



 M  +49 -26    render_block.cpp  


--- trunk/KDE/kdelibs/khtml/rendering/render_block.cpp #628747:628748
@@ -2990,9 +2990,7 @@
     bool nowrap = !style()->autoWrap();
 
     RenderObject *child = firstChild();
-    RenderObject* prevFloat = 0;
-    int floatWidths = 0;
-    int floatMaxWidth = 0;
+    int floatLeftWidth = 0, floatRightWidth = 0, floatMaxWidth = 0;
 
     while(child != 0)
     {
@@ -3002,11 +3000,16 @@
             continue;
         }
 
-        if (prevFloat && (!child->isFloating() ||
-                          ((prevFloat->style()->floating() & FLEFT) && (child->style()->clear() & CLEFT)) ||
-                          ((prevFloat->style()->floating() & FRIGHT) && (child->style()->clear() & CRIGHT)))) {
-            m_maxWidth = qMax(floatWidths, m_maxWidth);
-            floatWidths = 0;
+         if (child->isFloating() || child->flowAroundFloats()) {
+             int floatTotalWidth = floatLeftWidth + floatRightWidth;
+             if (child->style()->clear() & CLEFT) {
+                 m_maxWidth = qMax(floatTotalWidth, m_maxWidth);
+                 floatLeftWidth = 0;
+             }
+             if (child->style()->clear() & CRIGHT) {
+                 m_maxWidth = qMax(floatTotalWidth, m_maxWidth);
+                 floatRightWidth = 0;
+             }
         }
 
         Length ml = child->style()->marginLeft();
@@ -3024,35 +3027,60 @@
         // Percentage margins are computed as a percentage of the width we calculated in
         // the calcWidth call above.  In this case we use the actual cached margin values on
         // the RenderObject itself.
-        int margin = 0;
+        int margin = 0, marginLeft = 0, marginRight = 0;
         if (ml.isFixed())
-            margin += ml.value();
+            marginLeft += ml.value();
         else if (ml.isPercent())
-            margin += child->marginLeft();
+            marginLeft += child->marginLeft();
 
         if (mr.isFixed())
-            margin += mr.value();
+            marginRight += mr.value();
         else if (mr.isPercent())
-            margin += child->marginRight();
+            marginRight += child->marginRight();
 
-        if (margin < 0) margin = 0;
+        margin = marginLeft + marginRight;
 
         int w = child->minWidth() + margin;
         if(m_minWidth < w) m_minWidth = w;
+
         // IE ignores tables for calculation of nowrap. Makes some sense.
         if ( nowrap && !child->isTable() && m_maxWidth < w )
             m_maxWidth = w;
 
         w = child->maxWidth() + margin;
 
-        if(m_maxWidth < w) m_maxWidth = w;
+         if (!child->isFloating()) {
+             if (child->flowAroundFloats()) {
+                 // Determine a left and right max value based on whether or not the floats can fit in the
+                 // margins of the object.  For negative margins, we will attempt to overlap the float if the negative margin
+                 // is smaller than the float width.
+                 int maxLeft = marginLeft > 0 ? qMax(floatLeftWidth, marginLeft) : floatLeftWidth + marginLeft;
+                 int maxRight = marginRight > 0 ? qMax(floatRightWidth, marginRight) : floatRightWidth + marginRight;
+                 w = child->maxWidth() + maxLeft + maxRight;
+                 w = qMax(w, floatLeftWidth + floatRightWidth);
+             }
+             else
+                 m_maxWidth = qMax(floatLeftWidth + floatRightWidth, m_maxWidth);
+             floatLeftWidth = floatRightWidth = 0;
+         }
 
         if (child->isFloating()) {
-            if (prevFloat && (floatWidths + w > floatMaxWidth)) {
-               m_maxWidth = qMax(floatWidths, m_maxWidth);
-               floatWidths = w;
-            } else
-               floatWidths += w;
+             if (!floatMaxWidth) 
+                 floatMaxWidth = availableWidth();
+             if (style()->floating() & FLEFT) {
+                 if (floatLeftWidth + w > floatMaxWidth) {
+                     m_maxWidth = qMax(floatLeftWidth+floatRightWidth, m_maxWidth);
+                     floatLeftWidth = w;
+                 } else
+                     floatLeftWidth += w;
+                 
+             } else {
+                 if (floatRightWidth + w > floatMaxWidth) {
+                     m_maxWidth = qMax(floatLeftWidth+floatRightWidth, m_maxWidth);
+                     floatRightWidth = w;
+                 } else
+                     floatRightWidth += w;
+             }
         } else if (m_maxWidth < w)
             m_maxWidth = w;
 
@@ -3077,14 +3105,9 @@
             if (!cb->isTableCell())
                 m_maxWidth = BLOCK_MAX_WIDTH;
         }
-        if (child->isFloating()) {
-            prevFloat = child;
-            if (!floatMaxWidth)
-                floatMaxWidth = availableWidth();
-        }
         child = child->nextSibling();
     }
-    m_maxWidth = qMax(floatWidths, m_maxWidth);
+    m_maxWidth = qMax(floatLeftWidth + floatRightWidth, m_maxWidth);
 }
 
 void RenderBlock::close()