[webkit-reviews] review denied: [Bug 38711] DragData::asURL() shouldn't do file validity checks : [Attachment 55903] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu May 13 10:58:15 PDT 2010


Jian Li <jianli at chromium.org> has denied Daniel Cheng <dcheng at chromium.org>'s
request for review:
Bug 38711: DragData::asURL() shouldn't do file validity checks
https://bugs.webkit.org/show_bug.cgi?id=38711

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

------- Additional Comments from Jian Li <jianli at chromium.org>
Please also ask someone who is familiar with WebNSPasteboardExtras.mm to take a
look.

LayoutTests/ChangeLog:3
 +	    DragData::asURL() shouldn't do file validity checks
Since you also touch WebNSPasteboardExtras.mm, could you please update the bug
title and all ChangeLog to describe the problem in generic way?

LayoutTests/editing/pasteboard/file-input-files-access.html: 
 +  <script src="../../fast/js/resources/js-test-post.js"></script>
Why do we need to remove this?

LayoutTests/editing/pasteboard/script-tests/file-input-files-access.js:112
 +	// It'd be easiest to use try-finally, but it doesn't work.
This comment is somewhat confusing. Can you explain?

LayoutTests/editing/pasteboard/script-tests/file-input-files-access.js:115
 +	    // file input here.
Don't need to fold into 2 lines.

WebKit/mac/Misc/WebNSPasteboardExtras.mm:150
 +		    isDirectory)
Don't need to fold into 2 lines.

WebKit/mac/ChangeLog:11
 +	    handling, since the Mac loader doesn't work correctly with them. 
I think there is a risk condition here. If the directory does not exist at
first, [NSPasteboard _web_bestURL] will return a file URL. Then the directory
is created.


More information about the webkit-reviews mailing list