[webkit-reviews] review denied: [Bug 24887] Wrong filename when dragging an image to the Desktop. : [Attachment 34290] patch, responding to Darin's review

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Jens Alfke <snej at chromium.org>'s
request for review:
Bug 24887: Wrong filename when dragging an image to the Desktop.
https://bugs.webkit.org/show_bug.cgi?id=24887

Attachment 34290: patch, responding to Darin's review
https://bugs.webkit.org/attachment.cgi?id=34290&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
Isn't the plumbing already there?  You do a drag of an image, with eventSender
and then look at what the URLs are on the clipboard.  This should all be
possible from DRT today, no?

There are lots of examples of starting drags in DRT using
eventSender.mouseDown(), eventSender.leapForward(), eventSender.mouseMove(),
and there are also examples of inspecting the clipboard (event.dataTransfer),
which you can do on a drop event.

What I don't understand is why this is such a huge amount of code.  Is this
more than one fix?  It's very difficult to understand what's going on in a
patch this large, making it difficult to review.  Is a bunch of this
refactoring which coudl be done first before the actual code change?

Also, welcome (back to?) WebKit Jens. :)

r- for lack of test (or at least in need of more explaination as to why the
above proposed testing strategy would not work.)

The code i'm proposing:

var dragStart = document.createElement('img');
dragStart.src = "imageName.png";
document.body.appendChild(dragStart);

var dragEnd = document.createElement('div');
dragEnd.style.cssText = "width: 100px; height: 100px;"
document.body.appendChild(dragEnd);

// get the x/y middle of dragStart and dragEnd (see examples in the layout
tests).

dragEnd.ondrop = function() {
    event.dataTransfer.getData("URLS");
    // do whatever you need to do here.
}

eventSender.mouseMove(startX, startY);
eventSender.mouseDown();
eventSender.leapForward(1000); // to bypass the mac drag delay
eventSender.mouseMove(endX, endY);
eventSender.mouseUp();


Ideally the test would be dumpAsText with a bunch of nice pass/fail messages,
which you would get for free if you use the fast/js testing framework.	Search
for TEMPLATE.html in the LayoutTests for examples of these types of tests.  You
write a .js file and then make-js-test-wrappers generates a wrapper for you
using the TEMPLATE.html file found in the same directory.