Bug 100818

Summary:

[EFL][WK2] Add Ewk_File_Chooser_Request API

Product:

WebKit

Reporter:

Chris Dumez <cdumez>

Component:

WebKit EFL

Assignee:

Chris Dumez <cdumez>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

gyuyoung.kim, kenneth, laszlo.gombos, lucas.de.marchi, rakuco, tonikitoo, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

61838, 100822

Attachments:

Description	Flags
Patch	none
Patch	kenneth: review+, webkit.review.bot: commit-queue-
Patch for landing	none

Chris Dumez

Reported 2012-10-30 23:35:17 PDT

When the user click on an input button of type "file", we should show a file chooser dialog. However, we don't currently provide an API allowing the browser to do so.

Attachments
Patch (29.65 KB, patch) 2012-10-31 05:37 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (30.00 KB, patch) 2012-10-31 06:55 PDT, Chris Dumez	kenneth: review+ webkit.review.bot: commit-queue-	Details Formatted Diff Diff
Patch for landing (30.02 KB, patch) 2012-10-31 09:50 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Chris Dumez

Comment 1 2012-10-31 05:37:23 PDT

Created attachment 171625 [details] Patch

Mikhail Pozdnyakov

Comment 2 2012-10-31 06:15:17 PDT

Comment on attachment 171625 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171625&action=review > Source/WebKit2/UIProcess/API/efl/ewk_file_chooser_request.cpp:72 > +void Ewk_File_Chooser_Request::chooseFiles(Vector< RefPtr<APIObject> >& fileURLs) does not look like you're modifying fileURLs. Why not const ref? > Source/WebKit2/UIProcess/API/efl/ewk_file_chooser_request.cpp:143 > + EINA_LIST_FOREACH(chosenFiles, l, data) { any check that data is not null, assertion? > Source/WebKit2/UIProcess/API/efl/ewk_file_chooser_request_private.h:52 > + inline bool wasHandled() const { return m_wasRequestHandled; } is inline needed? > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_file_chooser_request.cpp:38 > +static void onFileChooserRequest(void* userData, Evas_Object*, void* eventInfo) shouldn't input parameters been checked? > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_file_chooser_request.cpp:43 > +static int compareStrings(const void* string1, const void* string2) same here

Chris Dumez

Comment 3 2012-10-31 06:30:56 PDT

Comment on attachment 171625 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171625&action=review >> Source/WebKit2/UIProcess/API/efl/ewk_file_chooser_request.cpp:72 >> +void Ewk_File_Chooser_Request::chooseFiles(Vector< RefPtr<APIObject> >& fileURLs) > > does not look like you're modifying fileURLs. Why not const ref? Needed by ImmutableArray::adopt() >> Source/WebKit2/UIProcess/API/efl/ewk_file_chooser_request.cpp:143 >> + EINA_LIST_FOREACH(chosenFiles, l, data) { > > any check that data is not null, assertion? Yeah, I'll add a EINA_SAFETY_ON_NULL_RETURN_VAL(). >> Source/WebKit2/UIProcess/API/efl/ewk_file_chooser_request_private.h:52 >> + inline bool wasHandled() const { return m_wasRequestHandled; } > > is inline needed? It does not hurt. >> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_file_chooser_request.cpp:38 >> +static void onFileChooserRequest(void* userData, Evas_Object*, void* eventInfo) > > shouldn't input parameters been checked? Ok >> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_file_chooser_request.cpp:43 >> +static int compareStrings(const void* string1, const void* string2) > > same here ok

Chris Dumez

Comment 4 2012-10-31 06:55:33 PDT

Created attachment 171642 [details] Patch Take Mikhail's feedback into consideration.

WebKit Review Bot

Comment 5 2012-10-31 09:37:52 PDT

Comment on attachment 171642 [details] Patch Rejecting attachment 171642 [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: t/git/webkit-commit-queue/Source/WebKit/chromium/v8 --revision 12787 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' 50>At revision 12787. ________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Full output: http://queues.webkit.org/results/14561278

Chris Dumez

Comment 6 2012-10-31 09:50:09 PDT

Created attachment 171675 [details] Patch for landing Rebased on master and remove tabs. Could someone please cq+ ?

WebKit Review Bot

Comment 7 2012-10-31 12:17:36 PDT

Comment on attachment 171675 [details] Patch for landing Clearing flags on attachment: 171675 Committed r133057: <http://trac.webkit.org/changeset/133057>

WebKit Review Bot

Comment 8 2012-10-31 12:17:41 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.