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

[bugzilla-daemon at webkit.org]

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





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

>>> 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;
> 
> As you can see in the WebCore, http://trac.webkit.org/browser/trunk/Source/WebCore/html/HTMLInputElement.cpp#L1723
> If there is not a valid value, input element works with "filesystem".
> 
> So, EWK_FILE_CHOOSER_CAPTURE_TYPE_INVALID is only used when Ewk_File_Chooser pointer or capture attribute is invalid.
> Therefore I think current implementation is right.

Ok, I understand.

>>> 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.
> 
> No problem. 
> but I wonder which one is right? "Use..." like a ewk_intest.h or "use...".
> I thought we don't use capital letter with @return.
> But, you didn't mention about that.
> 
> Thanks gyuyoung.

Please use lower case. I'm going to change uppercase in ewk_intent.h

> Source/WebKit/efl/ewk/ewk_file_chooser.h:119
> + * The capture attributes is to support HTML Media Capture.

Why don't you mention more information as chromium port ?

    // See http://www.w3.org/TR/html-media-capture/ for the semantics of the
    // capture attribute. This string will either be empty (meaning the feature
    // is disabled) or one of the following values:
    //  - filesystem (default)
    //  - camera
    //  - camcorder
    //  - microphone

> Source/WebKit/efl/ewk/ewk_file_chooser_private.h:28
> +void ewk_file_chooser_free(Ewk_File_Chooser* ewkFileChooser);

ditto.

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