Bug 90711

Summary:	KHTML Padding issue with Tables that embedd other Tables using CSS2
Product:	[Applications] konqueror	Reporter:	Ali Akcaagac <aliakc>
Component:	khtml renderer	Assignee:	Konqueror Bugs <konqueror-bugs-null>
Status:	RESOLVED FIXED
Severity:	normal
Priority:	NOR
Version First Reported In:	unspecified
Target Milestone:	---
Platform:	Compiled Sources
OS:	Linux
Latest Commit:		Version Fixed/Implemented In:
Sentry Crash Report:
Attachments:	Firefox PNG Konqueror PNG

Description Ali Akcaagac 2004-10-03 17:49:44 UTC

Version:            (using KDE Devel)
Installed from:    Compiled sources
Compiler:          gcc 3.4.1 
OS:                Linux

As you can see on below Screenshots you see Konqueror and Firefox rendering my personal Web site. Both result in different rendering and layout which looks somehow strange. I also verified this with Internet Explorer from Microsoft and it more or less is going conform with what Firefox renders.

The Web site is using CSS2 heavily and the problems are inside Tables that embedd other Tables and padding.

Please notice 750 pixel size Table. The left part shows the menu which is a Table on it's own and the right part (with the context) is a Table as well.

So the main Table embedds 2 Tables one for the Menu left and one for the Context right. The pane weight is 20% for the left Menu and 80% for the right Context.

Furthermore the Menu Table itself embedds another Table which itself contains the Links (this was done on purpose). KHTML has some issues here rendering the padding correctly as you can see in the Stylesheet file: (http://www.akcaagac.com/index.css). Also the weight itself 20% and 80% isn't really accurate and I believe this to be the reason because of ignoring the padding. Left, Right, Top and Bottom padding.

I would like to ask whether this can be fixed.

Comment 1 Ali Akcaagac 2004-10-03 17:50:30 UTC

Created attachment 7771 [details]
Firefox PNG

Comment 2 Ali Akcaagac 2004-10-03 17:51:11 UTC

Created attachment 7772 [details]
Konqueror PNG

Comment 3 Allan Sandfeld 2005-03-08 12:23:18 UTC

CVS commit by carewolf: 

Respect padding in tables. Merged from WebCore
BUG: 50235, 90711


  M +9 -1      ChangeLog   1.397
  M +6 -11     html/html_tableimpl.cpp   1.190
  M +11 -9     rendering/render_table.cpp   1.277
  M +6 -5      rendering/render_table.h   1.113
  M +4 -4      rendering/table_layout.cpp   1.41


--- kdelibs/khtml/html/html_tableimpl.cpp  #1.189:1.190
@@ -455,17 +455,12 @@ void HTMLTableElementImpl::parseAttribut
         break;
     case ATTR_CELLPADDING:
-        if (!attr->value().isEmpty()) {
-            addCSSLength(CSS_PROP_PADDING_TOP, attr->value(), true);
-            addCSSLength(CSS_PROP_PADDING_LEFT, attr->value(), true);
-            addCSSLength(CSS_PROP_PADDING_BOTTOM, attr->value(), true);
-            addCSSLength(CSS_PROP_PADDING_RIGHT, attr->value(), true);
+        if (!attr->value().isEmpty())
             padding = kMax( 0, attr->value().toInt() );
-        }
-        else {
-            removeCSSProperty(CSS_PROP_PADDING_TOP);
-            removeCSSProperty(CSS_PROP_PADDING_LEFT);
-            removeCSSProperty(CSS_PROP_PADDING_BOTTOM);
-            removeCSSProperty(CSS_PROP_PADDING_RIGHT);
+        else
             padding = 1;
+        if (m_render && m_render->isTable()) {
+            static_cast<RenderTable *>(m_render)->setCellPadding(padding);
+            if (!m_render->needsLayout())
+                m_render->setNeedsLayout(true);
         }
         break;

--- kdelibs/khtml/ChangeLog  #1.396:1.397
@@ -1,2 +1,10 @@
+2005-03-04  Allan Sandfeld Jensen <kde@carewolf.com>
+
+        Merge table padding from WebCore
+
+        * html/html_tableimpl.cpp: CELLPADDING should not set normal padding
+        * rendering/render_table.cpp: Respect padding
+        * rendering/render_table.h: bordersAndSpacing() -> bordersPaddingAndSpacing()
+
 2005-03-04  Germain Garand  <germain@ebooksfrance.org>
 
@@ -15,5 +23,5 @@
         * ecma/kjs_dom.cpp (getValueProperty/putValueProperty):
         Mozilla/IE strict compatibility: document.documentElement.scroll{Top,Left}
-        concern the canvas, not the root block. 
+        concern the canvas, not the root block.
         Cf. http://www.quirksmode.org/viewport/compatibility.html
 

--- kdelibs/khtml/rendering/render_table.cpp  #1.276:1.277
@@ -296,6 +296,6 @@ void RenderTable::layout()
     }
 
-    int bpTop = borderTop();
-    int bpBottom = borderBottom();
+    int bpTop = borderTop() + (collapseBorders() ? 0 : paddingTop());
+    int bpBottom = borderBottom() + (collapseBorders() ? 0 : paddingBottom());
 
     m_height += bpTop;
@@ -327,4 +327,6 @@ void RenderTable::layout()
     }
     int bl = borderLeft();
+    if (!collapseBorders())
+        bl += paddingLeft();
 
     // position the table sections

--- kdelibs/khtml/rendering/render_table.h  #1.112:1.113
@@ -161,6 +161,7 @@ public:
     }
 
-    int bordersAndSpacing() const {
-        return borderLeft() + borderRight() + (numEffCols()+1) * borderHSpacing();
+    int bordersPaddingAndSpacing() const {
+        return borderLeft() + borderRight() +
+               (collapseBorders() ? 0 : (paddingLeft() + paddingRight() + (numEffCols()+1) * borderHSpacing()));
     }
 

--- kdelibs/khtml/rendering/table_layout.cpp  #1.40:1.41
@@ -233,5 +233,5 @@ void FixedTableLayout::calcMinMaxWidth()
     // unlimited.
 
-    int bs = table->bordersAndSpacing();
+    int bs = table->bordersPaddingAndSpacing();
     int tableWidth = table->style()->width().isFixed() ? table->style()->width().value() - bs : 0;
 
@@ -258,5 +258,5 @@ void FixedTableLayout::calcMinMaxWidth()
 void FixedTableLayout::layout()
 {
-    int tableWidth = table->width() - table->bordersAndSpacing();
+    int tableWidth = table->width() - table->bordersPaddingAndSpacing();
     int available = tableWidth;
     int nEffCols = table->numEffCols();
@@ -597,5 +597,5 @@ void AutoTableLayout::calcMinMaxWidth()
     maxWidth = kMax( maxWidth, spanMaxWidth );
 
-    int bs = table->bordersAndSpacing();
+    int bs = table->bordersPaddingAndSpacing();
     minWidth += bs;
     maxWidth += bs;
@@ -850,5 +850,5 @@ void AutoTableLayout::layout()
 {
     // table layout based on the values collected in the layout structure.
-    int tableWidth = table->width() - table->bordersAndSpacing();
+    int tableWidth = table->width() - table->bordersPaddingAndSpacing();
     int available = tableWidth;
     int nEffCols = table->numEffCols();