Bug 81589 - [Qt][WK2] Support multi-file upload
Summary: [Qt][WK2] Support multi-file upload
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-19 16:30 PDT by Dinu Jacob
Modified: 2012-03-26 19:30 PDT (History)
4 users (show)

See Also:


Attachments
Patch (20.14 KB, patch)
2012-03-19 16:42 PDT, Dinu Jacob
hausmann: review+
hausmann: commit-queue-
Details | Formatted Diff | Diff
Patch (20.05 KB, patch)
2012-03-26 10:32 PDT, Dinu Jacob
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (20.67 KB, patch)
2012-03-26 11:01 PDT, Dinu Jacob
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Re-based patch (21.29 KB, patch)
2012-03-26 12:23 PDT, Dinu Jacob
no flags Details | Formatted Diff | Diff
Re-based patch (20.92 KB, patch)
2012-03-26 12:27 PDT, Dinu Jacob
no flags Details | Formatted Diff | Diff
Patch (20.93 KB, patch)
2012-03-26 12:32 PDT, Dinu Jacob
no flags Details | Formatted Diff | Diff
Patch again (20.93 KB, patch)
2012-03-26 13:20 PDT, Dinu Jacob
no flags Details | Formatted Diff | Diff
Patch (20.97 KB, patch)
2012-03-26 14:00 PDT, Dinu Jacob
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dinu Jacob 2012-03-19 16:30:36 PDT
Add support for multi-file upload
Comment 1 Dinu Jacob 2012-03-19 16:42:06 PDT
Created attachment 132715 [details]
Patch
Comment 2 Simon Hausmann 2012-03-26 07:22:07 PDT
CC'ing Alex, this might require coordination due to his work on cleaning up the dialog code.
Comment 3 Simon Hausmann 2012-03-26 07:36:18 PDT
Comment on attachment 132715 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=132715&action=review

r=me, but please fix the conversion before landing

> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:384
> +    if (!dialogRunner.initForFilePicker(filePicker, q, selectedFileNames, type))

Hmm, initForFilePicker's last argument is a boolean, but type is an enum. It sounds like that conversion should be a bit more explicit, because like this it's not very readable.
Comment 4 Dinu Jacob 2012-03-26 10:32:04 PDT
Created attachment 133843 [details]
Patch
Comment 5 WebKit Review Bot 2012-03-26 10:59:08 PDT
Comment on attachment 133843 [details]
Patch

Rejecting attachment 133843 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
webkit-commit-queue/

Error: the Git diff contains a binary file without the binary data in line: "Binary files /dev/null and b/Tools/MiniBrowser/qt/icons/checkbox_checked.png differ".  Be sure to use the --binary flag when invoking "git diff" with diffs containing binary files. at /mnt/git/webkit-commit-queue/Tools/Scripts/VCSUtils.pm line 690, <ARGV> line 369.

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 9 cwd: /mnt/git/webkit-commit-queue/

Full output: http://queues.webkit.org/results/12131964
Comment 6 Dinu Jacob 2012-03-26 11:01:16 PDT
Created attachment 133847 [details]
Patch
Comment 7 Alexander Færøy 2012-03-26 11:09:18 PDT
I am fine with this change. I just got sidetracked by something else, so we can just get this in and I will rebase my patch before uploading it for review :-)
Comment 8 WebKit Review Bot 2012-03-26 11:20:08 PDT
Comment on attachment 133847 [details]
Patch

Rejecting attachment 133847 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
1 hunk FAILED -- saving rejects to file Source/WebKit2/UIProcess/qt/QtDialogRunner.h.rej
patching file Tools/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Tools/MiniBrowser/qt/MiniBrowser.qrc
patching file Tools/MiniBrowser/qt/js/MultiSelect.js
patching file Tools/MiniBrowser/qt/qml/CheckBox.qml
patching file Tools/MiniBrowser/qt/qml/FilePicker.qml

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue/

Full output: http://queues.webkit.org/results/12132900
Comment 9 Dinu Jacob 2012-03-26 12:23:43 PDT
Created attachment 133863 [details]
Re-based patch
Comment 10 Dinu Jacob 2012-03-26 12:27:29 PDT
Created attachment 133864 [details]
Re-based patch
Comment 11 Dinu Jacob 2012-03-26 12:32:36 PDT
Created attachment 133865 [details]
Patch
Comment 12 Build Bot 2012-03-26 12:54:24 PDT
Comment on attachment 133865 [details]
Patch

Attachment 133865 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12133969
Comment 13 Dinu Jacob 2012-03-26 13:20:07 PDT
Created attachment 133876 [details]
Patch again
Comment 14 Alexander Færøy 2012-03-26 13:49:09 PDT
Comment on attachment 133876 [details]
Patch again

View in context: https://bugs.webkit.org/attachment.cgi?id=133876&action=review

> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:371
> +    if (!dialogRunner.initForFilePicker(filePicker, q, selectedFileNames, (bool)type))

I don't think that was what Simon meant by being more explicit. I suggest you change it to something like:

if (!dialogRunner.initForFilePicker(filePicker, q, selectedFileNames, type == QtWebPageUIClient::Whatever))
Comment 15 Dinu Jacob 2012-03-26 13:51:15 PDT
(In reply to comment #14)
> (From update of attachment 133876 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=133876&action=review
> 
> > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:371
> > +    if (!dialogRunner.initForFilePicker(filePicker, q, selectedFileNames, (bool)type))
> 
> I don't think that was what Simon meant by being more explicit. I suggest you change it to something like:
> 
> if (!dialogRunner.initForFilePicker(filePicker, q, selectedFileNames, type == QtWebPageUIClient::Whatever))

Sure I can make it an explicit boolean value like that.
Comment 16 Dinu Jacob 2012-03-26 14:00:14 PDT
Created attachment 133888 [details]
Patch
Comment 17 WebKit Review Bot 2012-03-26 19:29:58 PDT
Comment on attachment 133888 [details]
Patch

Clearing flags on attachment: 133888

Committed r112195: <http://trac.webkit.org/changeset/112195>
Comment 18 WebKit Review Bot 2012-03-26 19:30:03 PDT
All reviewed patches have been landed.  Closing bug.