Bug 102536 - [test case] Incorrect margin for tables
Summary: [test case] Incorrect margin for tables
Status: RESOLVED FIXED
Alias: None
Product: konqueror
Classification: Applications
Component: khtml renderer (show other bugs)
Version: unspecified
Platform: unspecified Linux
: NOR normal with 20 votes (vote)
Target Milestone: ---
Assignee: Konqueror Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-03-26 15:41 UTC by fleona
Modified: 2006-10-14 16:32 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments
test case (297 bytes, text/html)
2005-04-12 10:55 UTC, Allan Sandfeld
Details
Corrected test case (299 bytes, text/html)
2005-04-12 10:56 UTC, Allan Sandfeld
Details

Note You need to log in before you can comment on or make changes to this bug.
Description fleona 2005-03-26 15:41:47 UTC
Version:            (using KDE KDE 3.4.0)
Installed from:    Unlisted Binary Package
Compiler:          gcc 3.4.3 
OS:                Linux

Hey guys. I have a small site that shows the static pages of a project of mine.

http://mipagina.cantv.net/fleona/html

Mozilla and Safari render all those tables centered correctly, respecting the margins i set with CSS. Somehow, konqueror displays the tables left-aligned, as if it wasn't respecting the right margin

Here is some css code of a table:
ttp://mipagina.cantv.net/fleona/html/css/style.css

table#header {
	background: #345487;
	width:90%;
	text-align: center;
	border-collapse: collapse;
	margin-left:5%;
	margin-right:5%;
}

margin-left and margin-right are 5%, set to center the table. 

On konqueror the right margin seems to be triple (15%). I don't think i will need to provide a screenshot, but if somehow you can't reproduce this, tell me to attach a comparison between mozilla and konqueror. Like i said, safari also renders correctly.
Comment 1 lexual 2005-04-12 10:17:05 UTC
The alignment looks correct to me and the same as firefox.

The right margin is too big though.
Comment 2 Allan Sandfeld 2005-04-12 10:55:26 UTC
Created attachment 10589 [details]
test case

A simple test case. It appears it doesn't happen if you replace the table with
a div.
Comment 3 Allan Sandfeld 2005-04-12 10:56:54 UTC
Created attachment 10590 [details]
Corrected test case

Sorry, the first test case was buggy
Comment 4 Allan Sandfeld 2006-10-14 16:32:07 UTC
SVN commit 595493 by carewolf:

Don't subtract margin from available-width + fix various width bugs that 
revealed themselves up after fixing that.
BUG: 102536


 M  +15 -9     render_table.cpp  
 M  +4 -0      render_table.h  
 M  +6 -2      table_layout.cpp  


--- branches/KDE/3.5/kdelibs/khtml/rendering/render_table.cpp #595492:595493
@@ -233,27 +233,33 @@
     RenderBlock *cb = containingBlock();
     int availableWidth = cb->contentWidth();
 
-    // Subtract minimum margins
-    availableWidth -= style()->marginLeft().minWidth(cb->contentWidth());
-    availableWidth -= style()->marginRight().minWidth(cb->contentWidth());
-
     LengthType widthType = style()->width().type();
     if(widthType > Relative && style()->width().value() > 0) {
 	// Percent or fixed table
-        m_width = style()->width().minWidth( availableWidth );
+        m_width = calcBoxWidth(style()->width().minWidth( availableWidth ));
         if(m_minWidth > m_width) m_width = m_minWidth;
     } else {
-        m_width = KMIN(short( availableWidth ), short(m_maxWidth));
+        // Subtract out any fixed margins from our available width for auto width tables.
+        int marginTotal = 0;
+        if (!style()->marginLeft().isVariable())
+            marginTotal += style()->marginLeft().width(availableWidth);
+        if (!style()->marginRight().isVariable())
+            marginTotal += style()->marginRight().width(availableWidth);
+
+        // Subtract out our margins to get the available content width.
+        int availContentWidth = kMax(0, availableWidth - marginTotal);
+
+        // Ensure we aren't bigger than our max width or smaller than our min width.
+        m_width = kMin(availContentWidth, m_maxWidth);
     }
 
     // restrict width to what we really have
     // EXCEPT percent tables, which are still calculated as above
-
     availableWidth = cb->lineWidth( m_y );
     if ( widthType != Percent )
-        m_width = KMIN( short( availableWidth ), m_width );
+        m_width = kMin( short( availableWidth ), m_width );
 
-    m_width = KMAX (m_width, m_minWidth);
+    m_width = kMax (m_width, m_minWidth);
 
     // Finally, with our true width determined, compute our margins for real.
     m_marginRight=0;
--- branches/KDE/3.5/kdelibs/khtml/rendering/render_table.h #595492:595493
@@ -73,6 +73,10 @@
     int borderRight() const;
     int borderTop() const;
     int borderBottom() const;
+    int paddingLeft() const { return collapseBorders() ? 0 : RenderBlock::paddingLeft(); }
+    int paddingRight() const { return collapseBorders() ? 0 : RenderBlock::paddingRight(); }
+    int paddingTop() const { return collapseBorders() ? 0 : RenderBlock::paddingTop(); }
+    int paddingBottom() const { return collapseBorders() ? 0 : RenderBlock::paddingBottom(); }
 
     const QColor &bgColor() const { return style()->backgroundColor(); }
 
--- branches/KDE/3.5/kdelibs/khtml/rendering/table_layout.cpp #595492:595493
@@ -233,7 +233,10 @@
     // unlimited.
 
     int bs = table->bordersPaddingAndSpacing();
-    int tableWidth = table->style()->width().isFixed() ? table->style()->width().value() - bs : 0;
+    int tableWidth = 0;
+    if (table->style()->width().isFixed()) {
+        tableWidth = table->calcBoxWidth(table->style()->width().value());
+    }
 
     int mw = calcWidthArray() + bs;
     table->m_minWidth = kMin( kMax( mw, tableWidth ), 0x7fff );
@@ -599,7 +602,8 @@
 
     Length tw = table->style()->width();
     if ( tw.isFixed() && tw.value() > 0 ) {
-	minWidth = kMax( minWidth, int( tw.value() ) );
+        int width = table->calcBoxWidth(tw.value());
+        minWidth = kMax( minWidth, width );
 	maxWidth = minWidth;
     }