Ticket #315 (closed defect: fixed)

Opened 5 months ago

Last modified 3 months ago

getElementDimensions is may be incorrect (fix included)

Reported by: h.belder {{{@}}} student.tudelft.nl Assigned to: somebody
Priority: normal Milestone: MochiKit 1.4
Component: component1 Version: 1.0
Severity: normal Keywords:
Cc: h.belder@student.tudelft.nl, cito@online.de

Description (Last modified by cito@online.de)

When using getElementDimensions to detect the content-box dimensions of a table or cell, the reported dimensions are incorrect when the table has border-collapse:collapse.

The reason is that MochiKit calculates the content box by subtracting it's border and padding from the offsetWidth/Height. The CSS spec states that in a border-collapsed table the only half the cell border width 'belongs' to a table / table cell, so the 'border width' that is subtracted is actually two times too large.

I don't have write access to the repositories, so I'm submitting a patch file here.

style.diff 

Index: Style.js
===================================================================
--- Style.js	(revision 1391)
+++ Style.js	(working copy)
@@ -363,17 +363,23 @@
             originalHeight = elem.offsetHeight || 0;
         }
         if (contentSize) {
+            // In border-collapsed table, the table / table element's border is shared with it's child or neighbour
+            // In IE, <td>, <tr> etc. elements have display: block
+            var divisor = ((self.getStyle(elem, "display").match(/^table/i) != null 
+                || elem.tagName.match(/^(table|thead|tbody|tfoot|col|colgroup|tr|td|td)$/i) != null) 
+                && self.getStyle(elem, "borderCollapse") == "collapse") ? 2 : 1;
+
             originalWidth -= Math.round(
                 (parseFloat(self.getStyle(elem, 'paddingLeft')) || 0)
               + (parseFloat(self.getStyle(elem, 'paddingRight')) || 0)
-              + (parseFloat(self.getStyle(elem, 'borderLeftWidth')) || 0)
-              + (parseFloat(self.getStyle(elem, 'borderRightWidth')) || 0)
+              + (parseFloat(self.getStyle(elem, 'borderLeftWidth')) || 0) / divisor
+              + (parseFloat(self.getStyle(elem, 'borderRightWidth')) || 0) / divisor
             );
             originalHeight -= Math.round(
                 (parseFloat(self.getStyle(elem, 'paddingTop')) || 0)
               + (parseFloat(self.getStyle(elem, 'paddingBottom')) || 0)
-              + (parseFloat(self.getStyle(elem, 'borderTopWidth')) || 0)
-              + (parseFloat(self.getStyle(elem, 'borderBottomWidth')) || 0)
+              + (parseFloat(self.getStyle(elem, 'borderTopWidth')) || 0) / divisor
+              + (parseFloat(self.getStyle(elem, 'borderBottomWidth')) || 0) / divisor
             );
         }
         return new self.Dimensions(originalWidth, originalHeight);

Change History

07/18/08 06:39:56 changed by cito@online.de

  • cc changed from h.belder@student.tudelft.nl to h.belder@student.tudelft.nl, cito@online.de.

You're right, in case borderCollapse is "collapse", browsers return the size including only half of the borders. However, this is only true if the cell is in the middle of a table. If the cell is on the edge, then the full border is reported, and your patch would return an incorrect value.

So it seems in the case when borderCollapse is "collapse" we will also have to inspect the DOM using the next/previousSibling and parentNode methods.

Also, do you think checking the display and tagName attributes with regular expressions is really necessary? I think it suffices to check the borderCollapse attribute. If it exists and is set to "collapse", we can assume it's a table cell.

Instead of the divisor, I would apply a multiplier, and only once, to the sum of the two border attributes.

07/21/08 13:53:28 changed by cito@online.de

Turned out the problem with edge cells mentioned above exists only for MSIE. The patch would work fine for all other browsers.

The following html page shows the problem: 

<html><body>

<table border="0" style="border-collapse:collapse"><tr align="center">
<td style="width: 90px; padding:0; border:10px solid blue">1</td>
<td style="width: 90px; padding:0; border:10px solid blue">2</td>
<td style="width: 90px; padding:0; border:10px solid blue">3</td>
</tr></table>

<p><script type="text/javascript">
td = document.getElementsByTagName('td');
for (var i=0; i<3; ++i) document.writeln(td[i].offsetWidth);
</script></p>

</body></html>

All browsers except MSIE report 90+0.5*(10+10) = 100 as the width of all three table cells, but in MSIE, the outer border of the first and last cell is added in full, so their width is reported as 90+0.5*10+10 = 105.

08/29/08 09:20:32 changed by cito@online.de

  • status changed from new to closed.
  • resolution set to fixed.
  • description changed.

After analyzing this a bit more, I found the following:

  • Collapsed borders are only relevant for table cell elements.
  • Not the border-collapse of the element itself, but of the *parent* is relevant.
  • If border-collapse = 'collapse', then the border is calculated in half.
  • MSIE counts the left border of the first cell in a row as not collapsed, same for the right border of the last cell in a row.
  • Firefox and Opera always counts the top and bottom border of a table cell as collapsed.
  • MSIE never counts the top and bottom border of a table cell as collapsed.
  • Safari treats the top and bottom border of a table cell according to border-collapse.

I have checked in a patch based on these observations and added some tests in r1393.