[webkit-reviews] review denied: [Bug 47514] CSS transforms should affect scrolling : [Attachment 70986] Patch

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied Beth Dakin <bdakin at apple.com>'s request
for review:
Bug 47514: CSS transforms should affect scrolling
https://bugs.webkit.org/show_bug.cgi?id=47514

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

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=70986&action=review

LayoutTests/platform/mac/fast/borders/border-image-rotate-transform-expected.pn
g

The results for this one look odd to me.  Why are there scrollbars?

Other comments inline.

> WebCore/rendering/RenderBlock.cpp:3524
> +    if (layer() && layer()->hasTransform()) {
> +	   IntRect transformRect = applyLayerTransformToRect(IntRect(x(), y(),
width(), bottom));
> +	   return transformRect.height();
> +    }

Ok, this check needs to take into account the other positions.	You can't just
use x, y and width.  You have to use the *untransformed* leftmostPosition,
topmostPosition, rightmostPosition and lowestPosition to make the rect.  Then
you transform that rect.  The other checks have the same issue.

> WebCore/rendering/RenderMedia.cpp:608
> +    return min(top, 
m_controlsShadowRoot->renderBox()->transformedFrameRect().y() +
m_controlsShadowRoot->renderBox()->topmostPosition(includeOverflowInterior,
includeSelf));

Can't the RenderMedia be transformed?  It seems like you have to account for a
transform here.  The base class call gives you the transformed rectangle's top.
 You can't just add the controls root position to that, since it was also
affected by the transform.

This needs to be a bit more like RenderBlock.  You have to apply the transform
at the end (and just as with RenderBlock, overflow out all four sides has to be
considered).