[Webkit-unassigned] [Bug 91957] [EFL] Add File Chooser API

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=91957


Gyuyoung Kim <gyuyoung.kim at samsung.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #154220|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #24 from Gyuyoung Kim <gyuyoung.kim at samsung.com>  2012-07-24 19:26:46 PST ---
(From update of attachment 154220)
View in context: https://bugs.webkit.org/attachment.cgi?id=154220&action=review

When you submit modified patch, please take a look this patch further. In addition, please run layout test as well. I don't like too long bug review thread.

> Source/WebKit/efl/ewk/ewk_file_chooser.cpp:90
> +    if (capture == "camera")

If possible, follow enum type definition order.

> Source/WebKit/efl/ewk/ewk_file_chooser.cpp:99
> +    return EWK_FILE_CHOOSER_CAPTURE_TYPE_FILESYSTEM;

I wonder if this patch can guarantee that capture only has camera, camcorder, microphone and file system.

If not so, you have to modify this as below,

if (capture == "filesystem")
    return EWK_FILE_CHOOSER_CAPTURE_TYPE_FILESYSTEM;

return EWK_FILE_CHOOSER_CAPTURE_TYPE_INVALID;

> Source/WebKit/efl/ewk/ewk_file_chooser.cpp:112
> +void ewk_file_chooser_free(Ewk_File_Chooser* ewkFileChooser)

Other functions are using just *chooser*. Keep to be consistent with other functions.

> Source/WebKit/efl/ewk/ewk_file_chooser.h:112
> + *         (use eina_stringshare_del() to free the items)

I think you don't need to use ( ). See also below comment.

http://trac.webkit.org/browser/trunk/Source/WebKit/efl/ewk/ewk_intent.h#L80

I will not use (..) in my next patch.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.