[webkit-reviews] review granted: [Bug 24252] Assert when hit testing a 3d transform that is null : [Attachment 28108] fix for crash

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Dean Jackson
<dino at apple.com>'s request for review:
Bug 24252: Assert when hit testing a 3d transform that is null
https://bugs.webkit.org/show_bug.cgi?id=24252

Attachment 28108: fix for crash
https://bugs.webkit.org/attachment.cgi?id=28108&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
> diff --git a/LayoutTests/transforms/no_transform_hit_testing-expected.txt
b/LayoutTests/transforms/no_transform_hit_testing-expected.txt
> new file mode 100644
> index 0000000..57ff5af
> --- /dev/null
> +++ b/LayoutTests/transforms/no_transform_hit_testing-expected.txt
> @@ -0,0 +1,5 @@
> +Testing hittest on a layer with null transform
> +
> +https://bugs.webkit.org/show_bug.cgi?id=24252
> +
> +transformed elementdid not crash

Make that "Did not crash, to test PASSED"

> diff --git a/LayoutTests/transforms/no_transform_hit_testing.html
b/LayoutTests/transforms/no_transform_hit_testing.html

> +  <script type="text/javascript" charset="utf-8">
> +	if (window.layoutTestController) {
> +	  layoutTestController.dumpAsText();
> +	  layoutTestController.waitUntilDone();

I don't think this needs to wait if you run the test from onload. Also, I don't
think it
needs to be a text test.

> +	function runTest()
> +	{
> +	  if (window.layoutTestController) {
> +	    eventSender.mouseMoveTo(100, 50);
> +	    eventSender.mouseDown();
> +	    eventSender.mouseUp();
> +	    eventSender.mouseDown();
> +	    eventSender.mouseUp();

Just call document.elementFromPoint(...) so it works in Safari too.

> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index 9c59011..1a22d7a 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,16 @@
> +2009-02-27  Dean Jackson  <dino at apple.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Fix asserting when hit testing an element that
> +	   has no transform.
> +	   https://bugs.webkit.org/show_bug.cgi?id=24252

I think this needs to explain the crash. Something like:

"renderer()->hasTransform() returns true for elements with perspective, but no
transform,
 so test for transform when hit testing".

r=me wit those changes.