[Webkit-unassigned] [Bug 48110] getBoundingClientRect: Do not truncate the coordinates to integers

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=48110


Nikolas Zimmermann <zimmermann at kde.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #74442|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #24 from Nikolas Zimmermann <zimmermann at kde.org>  2010-11-19 19:32:50 PST ---
(From update of attachment 74442)
View in context: https://bugs.webkit.org/attachment.cgi?id=74442&action=review

Patch is still perfect, I"m tempted to r+ it, but there are still some minor issues with the LayoutTests:

> LayoutTests/css3/zoom-coords.xhtml:-87
> -shouldBe('text1.top >= 75', 'true');
> +shouldBe('text1.top >= 250', 'true');
>  shouldBe('text1.width > 0', 'true');
>  shouldBe('text1.height > 0', 'true');
>  shouldBe('text1.right > 100', 'true');
> -shouldBe('text1.bottom > 75', 'true');

There's no way you could check for exact coordinate matches here??

> LayoutTests/css3/zoom-coords.xhtml:136
> -shouldBe('text2.left == 800', 'true');
> -shouldBe('text2.top >= 400', 'true');
> +shouldBe('text2.left == 175', 'true');
> +shouldBe('text2.top >= 100', 'true');

Ditto.

> LayoutTests/css3/zoom-coords.xhtml:141
>  shouldBe('text2.width > 0', 'true');
>  shouldBe('text2.height > 0', 'true');
> -shouldBe('text2.right > 800', 'true');
> -shouldBe('text2.bottom > 400', 'true');
> +shouldBe('text2.right > 175', 'true');
> +shouldBe('text2.bottom > 100', 'true');
> +debug("");

Ditto.

> LayoutTests/css3/zoom-coords.xhtml:170
> +shouldBe('text3.left == 1100', 'true');
> +shouldBe('text3.top >= 500', 'true');
> +shouldBe('text3.width > 0', 'true');
> +shouldBe('text3.height > 0', 'true');
> +shouldBe('text3.right > 1100', 'true');
> +shouldBe('text3.bottom > 500', 'true');

Ditto.

> LayoutTests/fast/dom/Element/getBoundingClientRect-expected.txt:8
> +PASS rect.left.toFixed(3) is "8.000"
> +PASS rect.top.toFixed(3) is "40.000"
> +PASS rect.width.toFixed(3) is "300.000"
> +PASS rect.height.toFixed(3) is "100.000"
> +PASS rect.right is rect.left + rect.width
> +PASS rect.bottom is rect.top + rect.height
> +

Excellent!

> LayoutTests/svg/zoom/page/zoom-getBoundingClientRect-expected.txt:6
> +PASS parseFloat(r1.left.toFixed(2)) is 20

You could have used:
shouldBeEqualToString("r1.left.toFixed(2)", "20.00")

This is cleaner, then using parseFloat on a toFixed returned string.

> LayoutTests/svg/zoom/page/zoom-getBoundingClientRect.xhtml:11
> +</svg><svg xmlns="http://www.w3.org/2000/svg" version="1.1"

Can you add a newline between </svg> and <svg>.

> LayoutTests/svg/zoom/page/zoom-getBoundingClientRect.xhtml:15
> +</svg><svg xmlns="http://www.w3.org/2000/svg" version="1.1"

Ditto.

> LayoutTests/svg/zoom/page/zoom-getBoundingClientRect.xhtml:34
> +    shouldBe('parseFloat(r1.left.toFixed(2))', '20');
> +    shouldBe('parseFloat(r1.top.toFixed(2))', '30');

As stated above, shouldBeEqualToString should be sufficient.

> LayoutTests/svg/zoom/page/zoom-zoom-coords-expected.txt:86
> +PASS parseFloat(text2.top.toFixed(2)) >= 100 is true
> +PASS parseFloat(text2.width.toFixed(2)) > 0 is true
> +PASS parseFloat(text2.height.toFixed(2)) > 0 is true
> +PASS parseFloat(text2.right.toFixed(2)) > 175 is true
> +PASS parseFloat(text2.bottom.toFixed(2)) > 100 is true
> +

No way to use equality here?

> LayoutTests/svg/zoom/page/zoom-zoom-coords-expected.txt:110
> +PASS parseFloat(text3.top.toFixed(2)) >= 500 is true
> +PASS parseFloat(text3.width.toFixed(2)) > 0 is true
> +PASS parseFloat(text3.height.toFixed(2)) > 0 is true
> +PASS parseFloat(text3.right.toFixed(2)) > 1100 is true
> +PASS parseFloat(text3.bottom.toFixed(2)) > 500 is true

Ditto.

> LayoutTests/svg/zoom/page/zoom-zoom-coords.xhtml:22
> +</svg><svg id="svg2" xmlns="http://www.w3.org/2000/svg"

Please add a newline between </svg> and <svg>.

> LayoutTests/svg/zoom/page/zoom-zoom-coords.xhtml:28
> +</svg><svg id="svg3" xmlns="http://www.w3.org/2000/svg"

Ditto.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.