[webkit-reviews] review denied: [Bug 32473] [Chromium] Add 2 parameters to WebViewClient::runFileChooser() : [Attachment 46542] Proposed patch (rev.3)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jan 13 23:43:12 PST 2010


Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied TAMURA, Kent
<tkent at chromium.org>'s request for review:
Bug 32473: [Chromium] Add 2 parameters to WebViewClient::runFileChooser()
https://bugs.webkit.org/show_bug.cgi?id=32473

Attachment 46542: Proposed patch (rev.3)
https://bugs.webkit.org/attachment.cgi?id=46542&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
> +++ b/WebKit/chromium/public/WebFileChooserParams.h
...
> +struct WebFileChooserParams {
...
> +    WebFileChooserCompletion* chooserCompletion;

I'm sorry for not mentioning this earlier, but I think that this
member should not be part of this structure since it is not a
parameter that effects the way the dialog is rendered.	It is
conceptually separate from those parameters since it is just
a mechanism for indicating completion of the runFileChooser call.

Sorry to add another round to this patch, but I think this would
be a good change to make.


> +++ b/WebKit/chromium/src/ChromeClientImpl.cpp
...
> +    WebFileChooserParams params;
> +    params.multiSelect = fileChooser->allowsMultipleFiles();
> +    params.acceptTypes = fileChooser->acceptTypes();
> +    const Vector<String>& selectedFiles = fileChooser->filenames();
> +    params.selectedFiles = WebVector<WebString>(selectedFiles);

you should be able to write:

       params.selectedFiles = fileChooser->filenames();

does that not work for you?


More information about the webkit-reviews mailing list