Bug 100818 – [EFL][WK2] Add Ewk_File_Chooser_Request API

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Chris Dumez]

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.

[Chris Dumez]

Created attachment 171625 [details]
Patch

[Mikhail Pozdnyakov]

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 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]

Created attachment 171642 [details]
Patch

Take Mikhail's feedback into consideration.

[WebKit Review Bot]

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]

Created attachment 171675 [details]
Patch for landing

Rebased on master and remove tabs.

Could someone please cq+ ?

[WebKit Review Bot]

Comment on attachment 171675 [details]
Patch for landing

Clearing flags on attachment: 171675

Committed r133057: <http://trac.webkit.org/changeset/133057>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.