630925 - GetClientRect() inaccuracy

Status: RESOLVED INVALID

(Core :: General, defect)

Description

[Terje Rosenlund]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; nb-NO; rv:1.9.2.13) Gecko/20101203 Firefox/3.6.13
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; nb-NO; rv:1.9.2.13) Gecko/20101203 Firefox/3.6.13

It seems that GetClientRect() are inaccurate in some cases
A google-search for 'firefox 1px off' gives 701 000 matches and indicates that there is something wrong
Problems are reported for many elements and may be caused by a common denominator (GetClientRect()?)

I have tried to create a sample but fails to reproduce the inaccuracy when isolating and moving affected html to a test-case

The problem shows up eg. in the following case:

 - Table inside a scrolling div where first row is a header-row (scrolls)
 - Above this table another table inside a non-scrolling div (static)
 - Both divs and tables are on same level in an outher table-cell and have same layout
 - direction: ltr;
 - border-collapse: separate;
 
My intention is to set all cells in the static header to the same width as cells in the scrolling header before I hide the scrolling header

Normaly this works fine but in some rare cases the width is off by 1px after resizing 
Checking the actual width after resizing for differing cells (using getComputedStyle()) reports them as equal

This problem has been there for a very long time and I have tried to fix it many times 
Finally it seems I have succeeded with this odd function:

function resize_headers(table)
{
	if (!table.records)		{  return null;  }	
	var i;
		var res = [];	// Debug
	var mcell = table.scroll_header.cells;	  // Change width to (measure)
	var scell = table.static_header.cells;		// Change width on (set)
	var width = accum = 0;
	table.scroll_header.style.display = '';
		table.container.style.textAlign = 'left'; // Debug
	for (i=0; i<scell.length; i++)
	{
		width = Pos(mcell.item(i)).getInside().width;
		if (i==scell.length-1)  {  width += scrollbars_width(table.scroll);  }
		trx.set.width(scell.item(i), width + accum + 0.5);
			// Debug only
			var dim = { accum: accum, preWidth: width, setWidth: width + accum + 0.5, afterWidth: Pos(scell.item(i)).getInside().width };
			res.push(dim);
		accum = (width+accum) - Math.round(width+accum);
			if (!confirm('Index: ' + i + '\naccum: ' + dim.accum + '\npreWidth: ' + dim.preWidth + '\nsetWidth: ' + dim.setWidth + '\nafter setWidth: ' + dim. afterWidth + '\nnew accum: ' + accum))  {  return res;  }
	}
		table.container.style.textAlign = ''; // Debug
//	table.scroll_header.style.display = 'none'; // Uncomment after debug!!
	return res;
}

Removing the odd 'accum' and/or 0.5 will cause the bug to show up again

The code has 2 effects:
 - Adding 0.5 makes ff act as if Math.round() was used instead of parseInt()
 - Fraction leftover from one cell is added to returned GetClientRect().width for next cell

I presume that getComputedStyle() calculates values for table-elements according to computed styles 
- taking into account margins, borders, padding, border-colapse and direction (or simply from left(?))

Style for body in my default style-sheet has border-collapse: separate; and direction: ltr;	(both default and inherited)
No possition or dimmention has been set (by style) to fractual values on any element
Affected elements includes table-elements where element and all predecessors have style.position: static
Values calculated by GetClientRect() should therefore (as far as I can se) never return fractual values for any position or dimmension

My sample has 3 cells per row and a dump from GetClientRect() shows:

	Obj.left = 9
	Obj.top = 121
	Obj.width = 308.20001220703125  <<= No - actual width is 308
	Obj.height = 16
	
	Obj.left = 328.20001220703125		<<= No - inherited offset from previous (+10px padding)
	Obj.top = 121
	Obj.width = 69.35000610351562   <<= No - actual width is 70
	Obj.height = 16
	
	Obj.left = 408.54998779296875   <<= No - inherited offset from ALL previous (+10px padding)
	Obj.top = 121
	Obj.width = 89.45001220703125   <<= No - + 17 (scrollbar) = 106.45001220703125, actual width is 106
	Obj.height = 16

Dump from resize_headers()

	Obj.0.accum = 0
	Obj.0.preWidth = 308.20001220703125
	Obj.0.setWidth = 308.70001220703125
	Obj.0.afterWidth = 308
	
	Obj.1.accum = 0.20001220703125
	Obj.1.preWidth = 69.35000610351562  <<= This one is wrong - loosing 1px
	Obj.1.setWidth = 70.05001831054688  <<= Saved by adding accum + 0.5 before set
	Obj.1.afterWidth = 70
	
	Obj.2.accum = -0.449981689453125
	Obj.2.preWidth = 106.45001220703125
	Obj.2.setWidth = 106.50003051757812
	Obj.2.afterWidth = 106

My conclusion is that there must be something wrong in how GetClientRect() handles round-off and accumulated values

Reproducible: Always

Comment 1

[Boris Zbarsky [:bzbarsky]]

getClientRect returns unrounded values; if your objects have non-integer positions it will return them.  It's a really simple function, with not much room for error in it: it just reports what the layout is.

That said, can you please attach a complete HTML page (doesn't have to be a small testcase or anything like that) that shows the problem to this bug?

Comment 2

[Terje Rosenlund]

Attachment: Show resulting non-integer width

Comment 3

[Terje Rosenlund]

My objects gets non-integer values eg. when setting width (integer) for a table having cells with style.width = 'auto'. Attached sample shows this. Fetching width-values for theese cells sometimes gives an 1px offset as shown above.

> it just reports what the layout is

May the common denominator be in the layout-engine then?

Comment 4

[Terje Rosenlund]

Observe the calculated 'Rest:' value in my attachment

Should not this value be be zero for all table-widths to ensure correct layout?

Comment 5

[Terje Rosenlund]

Attachment: Bug in previous test-case (using parseInt in calculation, changed to parseFloat)

Bug in previous test-case (using parseInt in calculation, changed to parseFloat)

Comment 6

[Terje Rosenlund]

Attachment: Compare parseInt, parseFloat and Math.round()

Comment 7

[Terje Rosenlund]

New test-case compares inaccuracy when using different methodes on values returned from getClientRect()

Deviations can be observed by pressing 'Increase/Decrease table width' manually or by pressing 'Find max deviation' to find max deviation for table-widths between 310px and 1366px (may trigger 'script runnig slow') (press F5 between runs to avoid accumulating test-results - forgot to reset div-innerHTML)

The only test showing acceptable deviations is when the values are used as is

This is not possible in the real world since ff are using parseInt() on values before assignment to eg. style.top/left/width and height

The only way to affect this is to run Math.round() on values before assigning them to style. Effectly this will give the smallest deviation possible as shown in 'Max Math.round abs. deviation:'

Conclusion: 1px deviation is not possible to avoid in ff

Comment 8

[Boris Zbarsky [:bzbarsky]]

> My objects gets non-integer values eg. when setting width (integer) for a
> table having cells with style.width = 'auto'.

Yes, this is expected behavior.  Auto width cells get extra width distributed to them in proportion to their intrinsic widths.  This can easily lead to non-integer widths for the cells.  This isn't a problem per se.

> Fetching width-values for theese cells sometimes gives an 1px offset as shown
> above.

Gives a 1px offset where?

> Should not this value be be zero for all table-widths to ensure correct
> layout?

You're seeing a nonzero value because JavaScript's number type is IEEE double, which can't actually express all fractional values.  Not only that, but the APIs involved before (in particular, computed style) use IEEE floats.  IEEE float only gives you 6 digits of decimal precision.

Widths in Gecko are stored in units of 1/60 of a pixel.  So for example, when I set the table width to 344 px I see table cell widths of 78.0833, 104.15, and 161.767 reported by the JS.  But the actual values are 78 + 5/60, 104 + 9/60, and 161 + 46/60.  These are the values Gecko is using internally, and they add up to exactly 344.

The problem is that if you try to write (78 + 5/60) with only 6 decimal digits (which is what your JS code ends up doing) you get 78.0833 (whereas the correct value is 78.08333333333 (With the 3 repeating).  So you get an error of about 3*10^{-5}.  You also get an error for the 104 + 9/60 case, of about 2*10^{-6}.  And for the 161+46/60 case you get an error of about -3.3*10^{-4}.  Adding those up, you get an error of about -3*10^{-4}, which is what your "Rest" value is reporting in this case for me: "Rest: -0.00029999999998153726".  But all of this is an artifact of the JS API you're using; the actual values in the layout engine add up to exactly the right number.

Looking at you last testcase, the floating-point max deviation you get is about .001 on my machine.  Of course if you use rounding to integers you can get bigger deviations; in general up to 0.5*(number of things you're adding up).  All that tells you is that you lose information when rounding!  Don't round.

> This is not possible in the real world since ff are using parseInt() on
> values before assignment to eg. style.top/left/width and height

Uh... no.  We don't.  If you set width to "1.24353px" we will use that as-is (rounding to the nearest 1/60 of a pixel, but certainly not to integer pixels!).

> Conclusion: 1px deviation is not possible to avoid in ff

Sure it is.  Don't round stuff.