Dragging image to desktop gives webloc instead of image file in WebKit2
Created attachment 85477 [details] Patch
Comment on attachment 85477 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=85477&action=review The code looks good to me. It's a very clever way of solving the problem! I'm assuming you've verified that the rest of the image dragging works as before. > Source/WebKit2/WebProcess/WebCoreSupport/mac/WebDragClientMac.mm:99 > + // FIXME: Seems this mesage should be named StartDrag, not SetDragImage. typo message. It was named after the AppKit API, but I agree that this actually starts the dragging. > Source/WebKit2/WebProcess/WebCoreSupport/mac/WebDragClientMac.mm:173 > + It is a bit hard to follow, given the amount of changes, but I don't understand why you're writing all the data to the pasteboard without checking the types required. Could you please clarify?
Attachment 85477 [details] did not build on mac: Build output: http://queues.webkit.org/results/8144005
Created attachment 85589 [details] Patch
Comment on attachment 85589 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=85589&action=review > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1519 > +#if PLATFORM(MAC) > + platformDragEnded(); > +#endif In general, I like having calls like these without the #if, and have each platform just have a stubbed out implementation.
Committed r80948: <http://trac.webkit.org/changeset/80948>
(In reply to comment #2) > (From update of attachment 85477 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=85477&action=review > > The code looks good to me. It's a very clever way of solving the problem! I'm assuming you've verified that the rest of the image dragging works as before. I did some testing. Not sure I covered everything. > > Source/WebKit2/WebProcess/WebCoreSupport/mac/WebDragClientMac.mm:99 > > + // FIXME: Seems this mesage should be named StartDrag, not SetDragImage. > > typo message. It was named after the AppKit API, but I agree that this actually starts the dragging. Oops, I missed your note and didn’t fix the typo. The AppKit API is named dragImage... not setDragImage, so I think our name is not really in the same spirit. > > Source/WebKit2/WebProcess/WebCoreSupport/mac/WebDragClientMac.mm:173 > > It is a bit hard to follow, given the amount of changes, but I don't understand why you're writing all the data to the pasteboard without checking the types required. Could you please clarify? The way the code works is that it puts the list of types in, and then checks each type. Since the code put the types in, there is no need for it to double check what it put in! Presumably the checks existed in the code we copied this from because there was some kind of separation of responsibilities of determining which types to include and putting the data on the pasteboard. But here I see no need for this checking.
*** Bug 56224 has been marked as a duplicate of this bug. ***
Oops, wrong bug.