[webkit-reviews] review denied: [Bug 24887] [chromium] Use correct file name for dragging out images. : [Attachment 95510] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Daniel Cheng <dcheng at chromium.org>'s
request for review:
Bug 24887: [chromium] Use correct file name for dragging out images.
https://bugs.webkit.org/show_bug.cgi?id=24887

Attachment 95510: Patch
https://bugs.webkit.org/attachment.cgi?id=95510&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=95510&action=review

Seems OK.  Mostly small nits.

> LayoutTests/fast/events/drag-image-filename.html:50
> +<p>This test requires DumpRenderTree.
> +
> +<div id="target" ondragenter="dragEnterOrOver(event)"
ondragover="dragEnterOrOver(event)" ondrop="drop(event)"></div>
> +<img id="source" src="resources/onload-image.png" alt="Does it work?">

Please include instructions for manually testing (e.g., drag the image to the
desktop.  The filename should be onload-image.png).

> LayoutTests/platform/gtk/Skipped:1147
> +# EventSender::dumpFilenameBeingDragged not implemented.
> +fast/events/drag-image-filename.html

Please file bugs for each platform that fails and include the bug link in the
Skipped files before landing.

> Source/WebCore/ChangeLog:6
> +	   [chromium] Use correct file name for dragging out images.
> +	   https://bugs.webkit.org/show_bug.cgi?id=24887

Please be more specific here.  Like, "Prefer the last component of the URL for
the filename over alt text when dragging out images."

> Tools/DumpRenderTree/chromium/EventSender.cpp:348
> +void EventSender::dumpFilenameBeingDragged(const CppArgumentList& arguments,
CppVariant* result)

Nit: I think webkit style is to not name params that are unused.