[webkit-reviews] review denied: [Bug 38826] Don't convert filenames to URLs in edit drags. : [Attachment 58205] Patch

[bugzilla-daemon at webkit.org]

Jian Li <jianli at chromium.org> has denied Daniel Cheng <dcheng at chromium.org>'s
request for review:
Bug 38826: Don't convert filenames to URLs in edit drags.
https://bugs.webkit.org/show_bug.cgi?id=38826

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

------- Additional Comments from Jian Li <jianli at chromium.org>
> > LayoutTests/ChangeLog:14
> >  +		(moveMouseToCenterOfElement):
> > No need to show these new methods in js.
> > 
> 
> This is the output from prepare-ChangeLog. I felt best to leave it that way.
Yes, this is output from prepare-ChangeLog. But many people choose not to add
new methods from the new file.

LayoutTests/editing/pasteboard/script-tests/file-drag-to-editable.js:18
 +  function dragFilesOntoEditableArea(files) {
Please move '{' to the beginning of next line.

WebCore/platform/android/DragDataAndroid.cpp:71
 +  bool DragData::containsURL(FilenameConversionPolicy filenamePolicy) const
It is better to omit the argument name "filenamePolicy" since otherwise it
might break other build for "unused parameter".

WebCore/platform/android/DragDataAndroid.cpp:76
 +  String DragData::asURL(FilenameConversionPolicy filenamePolicy, String*)
const
ditto.

WebCore/platform/win/ClipboardUtilitiesWin.cpp:366
 +	    // FIXME: originally, this logic was so dropping files on WebKit
would insert the filename.
The comment "this logic was so dropping files on WebKit" seems to be hard to
understand. Could you please rephrase it?

WebCore/platform/win/ClipboardUtilitiesWin.h:64
 +  String getURL(IDataObject*, DragData::FilenameConversionPolicy
filenamePolicy, bool& success, String* title = 0);
The argument name filenamePolicy can be omitted per coding style guideline.