[webkit-reviews] review granted: [Bug 111000] Selection gaps don't repaint correctly with transforms : [Attachment 193218] Patch

[bugzilla-daemon at webkit.org]

Julien Chaffraix <jchaffraix at webkit.org> has granted Shezan Baig
<shezbaig.wk at gmail.com>'s request for review:
Bug 111000: Selection gaps don't repaint correctly with transforms
https://bugs.webkit.org/show_bug.cgi?id=111000

Attachment 193218: Patch
https://bugs.webkit.org/attachment.cgi?id=193218&action=review

------- Additional Comments from Julien Chaffraix <jchaffraix at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=193218&action=review


> Source/WebCore/rendering/RenderBlock.cpp:3367
> +    mapLocalToContainer(repaintContainer, transformState, ApplyContainerFlip
| UseTransforms);

I would be supportive of testing the ApplyContainerFlip as part of this change
if it's not covered. Your change is correct from this perspective.

> LayoutTests/fast/repaint/selection-gap-transform.html:4
> +    <script src="resources/text-based-repaint.js" type="text/javascript"
charset="utf-8"></script>
> +    <script type="text/javascript" charset="utf-8">

Please remove the unneeded attributes (type, charset).

> LayoutTests/fast/repaint/selection-gap-transform.html:21
> +    <div style="-webkit-transform: translate(50px, 50px);">
> +	   <div id="target" style="background-color: red; width: 100px; height:
100px; position: absolute;"><br/></div><br/>

I would add:
* a fixed positioned case
* a container is transformed and fixed positioned case
(in different tests though)

as boxes with transforms act as containing block for fixed elements too.