[webkit-reviews] review granted: [Bug 173854] [iOS DnD] Support dragging out of contenteditable areas without a prior selection : [Attachment 313873] Depends on bug #173832

Tue Jun 27 14:47:12 PDT 2017

Ryosuke Niwa <rniwa at webkit.org> has granted  review:
Bug 173854: [iOS DnD] Support dragging out of contenteditable areas without a
prior selection
https://bugs.webkit.org/show_bug.cgi?id=173854

Attachment 313873: Depends on bug #173832

https://bugs.webkit.org/attachment.cgi?id=313873&action=review

--- Comment #2 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 313873
  --> https://bugs.webkit.org/attachment.cgi?id=313873
Depends on bug #173832

View in context: https://bugs.webkit.org/attachment.cgi?id=313873&action=review

r=me assuming it passes tests.

> Source/WebCore/page/DragController.cpp:726
> +    auto* renderer = image.renderer();
> +    if (!is<RenderImage>(renderer))
> +	   return false;
> +
> +    auto* cachedImage = downcast<RenderImage>(*renderer).cachedImage();
> +    return cachedImage && !cachedImage->errorOccurred() &&
cachedImage->imageForRenderer(renderer);

We need to be always careful when we rely on the existence of renderer(). Are
we sure we've updated the layout / style before getting here?

> Source/WebCore/page/ios/EventHandlerIOS.mm:579
> +    IntPoint adjustedClientPosition;

We do we need to declare this upfront?

> Source/WebCore/page/ios/EventHandlerIOS.mm:584
> +    {
> +	   FloatPoint adjustedClientPositionAsFloatPoint(clientPosition);
> +	   protectedFrame->nodeRespondingToClickEvents(clientPosition,
adjustedClientPositionAsFloatPoint);
> +	   adjustedClientPosition =
roundedIntPoint(adjustedClientPositionAsFloatPoint);
> +    }

Why can't we get rid of the curly brackets and declare adjustedClientPosition
right in the third line instead?

> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/contenteditable-and-target.html:1
> +<head>

Missing DOCTYPE.