Bug 176721 – [iOS DnD] Support DataTransfer.setDragImage when starting a drag on iOS

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Wenson Hsieh]

<rdar://problem/34373660>

[Wenson Hsieh]

Created attachment 320517 [details]
Patch

[Tim Horton]

Comment on attachment 320517 [details]
Patch

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

> Source/WebCore/page/DragController.cpp:1248
> +                if (auto* page = frame.page())
> +                    dragPreviewSize.scale(1 / page->deviceScaleFactor());

This seems a little odd. Did you test with page scale too?

> Source/WebCore/page/DragController.cpp:1252
> +            // We can simply position the preview simply using the bounds of the drag source element.

Too many simplys

[Wenson Hsieh]

(In reply to Tim Horton from comment #2)
> Comment on attachment 320517 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=320517&action=review
> 
> > Source/WebCore/page/DragController.cpp:1248
> > +                if (auto* page = frame.page())
> > +                    dragPreviewSize.scale(1 / page->deviceScaleFactor());
> 
> This seems a little odd. Did you test with page scale too?

Yes -- what'll happen is that the image will also appear scaled up, to match what the rest of the page looks like. I thought this would be better than having a detached drag image be a constant size, regardless of page scale, which is why I didn't also scale by 1 / pageScaleFactor() here, though I'm definitely open to that option as well!

> 
> > Source/WebCore/page/DragController.cpp:1252
> > +            // We can simply position the preview simply using the bounds of the drag source element.
> 
> Too many simplys

Good catch! I should stop spamming this word 🙃

[Tim Horton]

(In reply to Wenson Hsieh from comment #3)
> (In reply to Tim Horton from comment #2)
> > Comment on attachment 320517 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=320517&action=review
> > 
> > > Source/WebCore/page/DragController.cpp:1248
> > > +                if (auto* page = frame.page())
> > > +                    dragPreviewSize.scale(1 / page->deviceScaleFactor());
> > 
> > This seems a little odd. Did you test with page scale too?
> 
> Yes -- what'll happen is that the image will also appear scaled up, to match
> what the rest of the page looks like. I thought this would be better than
> having a detached drag image be a constant size, regardless of page scale,
> which is why I didn't also scale by 1 / pageScaleFactor() here, though I'm
> definitely open to that option as well!

Ah, got it. I think your plan makes sense.

[Wenson Hsieh]

Created attachment 320520 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 320520 [details]
Patch for landing

Clearing flags on attachment: 320520

Committed r221907: <http://trac.webkit.org/changeset/221907>