[webkit-reviews] review denied: [Bug 99842] Touch operation corrupts screen when specifying other than overflow:visible in css : [Attachment 172286] The patch to fix this problem (WinCairo port)

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Hideki <yoshida-hxa at necst.nec.co.jp>'s
request for review:
Bug 99842: Touch operation corrupts screen when specifying other than
overflow:visible in css
https://bugs.webkit.org/show_bug.cgi?id=99842

Attachment 172286: The patch to fix this problem (WinCairo port)
https://bugs.webkit.org/attachment.cgi?id=172286&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=172286&action=review


Change is probably OK, but bug fixes like this need a ChangeLog before they can
be reviewed and landed.

> Source/WebCore/platform/graphics/win/GraphicsContextCairoWin.cpp:1665
>	   // We negate here since panning up moves the content up, but moves
the scrollbar down.

Not the fault of this patch, but this code is in the complete wrong place. It’s
inappropriate for panning code that makes use of nodes, renderers, and such to
be in the platform layer, which is a separate subsystem that is lower level.
GraphicsContextCairoWin.cpp is a crazy place for this code, and this code must
be deleted or moved ASAP.

Longer term we may even move the platform code to a separate library, and code
like this would not even be something you could compile.

> Source/WebCore/platform/graphics/win/GraphicsContextCairoWin.cpp:1666
> +	  
m_gestureTargetNode->renderer()->enclosingLayer()->scrollByRecursively(IntSize(
-deltaX, -deltaY),
WebCore::RenderLayer::ScrollOffsetClamping::ScrollOffsetClamped);

Should not need the WebCore:: prefix for code inside WebCore.