[webkit-reviews] review granted: [Bug 28754] Positioned, compositing child jumps around as opacity transition runs : [Attachment 38767] Patch, testcases, changelogs

[bugzilla-daemon at webkit.org]

mitz at webkit.org has granted Simon Fraser (smfr) <simon.fraser at apple.com>'s
request for review:
Bug 28754: Positioned, compositing child jumps around as opacity transition
runs
https://bugs.webkit.org/show_bug.cgi?id=28754

Attachment 38767: Patch, testcases, changelogs
https://bugs.webkit.org/attachment.cgi?id=38767&action=review

------- Additional Comments from mitz at webkit.org
> +    for ( ; curr && !isPositionedContainer(curr); curr = curr->parent())
> +    { }

Is this the WebKit style convention for empty loop bodies? It seems strange.

>      for ( ; curr && !curr->renderer()->isRenderView() && !curr->transform();
curr = curr->parent())
> -	   { }
> +    { }

Ditto.

> +	       RenderLayer* positionedAncestor = enclosingPositionedAncestor();


I think you can call parentLayer->enclosingPositionedAncestor() instead.

> +	       int thisX = 0, thisY = 0;

Please define each variable on a separate line.

> +	       convertToLayerCoords(positionedAncestor, thisX, thisY);
> +	       
> +	       int ancestorX = 0, ancestorY = 0;

Separate lines, please.