[webkit-reviews] review denied: [Bug 61338] Element not fully repainted after application and removal of transform : [Attachment 121086] patch for "ement not fully repainted after application and removal of transform " with testcase, expected results with no svn properties on added files

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Vamshi Krishna N
<vnampally at innominds.com>'s request for review:
Bug 61338: Element not fully repainted after application and removal of
transform
https://bugs.webkit.org/show_bug.cgi?id=61338

Attachment 121086: patch for "ement not fully repainted after application and
removal of transform " with testcase,expected results with no svn properties on
added files
https://bugs.webkit.org/attachment.cgi?id=121086&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=121086&action=review


> Source/WebCore/ChangeLog:12
> +	   Adjusting the diff in adjustStyleDifference for the changed
Transform property
> +	   taking into account the StyleDifferenceEqual returned after setting
up of the 
> +	   ContextSensitivePropertyTransform from the diff function.

I can't really understand from this what the change is about. It's better to
state what the original problem is, and how you fixed it. I'm not really
convinced that the change is correct.

> LayoutTests/fast/repaint/transform-rotate-remove.html:7
> +    .Rotate {

Class names should be adjectives, and lowercase, so 'rotated'.

> LayoutTests/fast/repaint/transform-rotate-remove.html:18
> +	     rotate();	   

Odd indentation here.

> LayoutTests/fast/repaint/transform-rotate-remove.html:26
> +	   if (window.layoutTestController)

And here.

> LayoutTests/fast/repaint/transform-rotate-remove.html:35
> +	   document.body.offsetTop;
> +	   layoutTestController.display();

Why the offsetTop? display() should force a layout.

Is there a reason this test isn't using the standard repaint test helper
script?