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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jul 24 01:18:44 PDT 2012


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





--- Comment #18 from Kihong Kwon <kihong.kwon at samsung.com>  2012-07-24 01:18:47 PST ---
(In reply to comment #15)
> (From update of attachment 153943 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=153943&action=review
> 
> Stupid question: How is the client support to use this API? Both the constructor and destructor are private and the patch does not introduce any where to retrieve the Ewk_File_Choose_Settings from somewhere else. In itself, the patch seems to add code but no real functionality. Am I missing something?

Client can access this by run_open_panel to get attribute of panel.

> > Source/WebKit/efl/ewk/ewk_file_chooser_settings.cpp:65
> > +    Vector<WTF::String>::const_iterator it = settings->fileChooserSettings.acceptMIMETypes.begin();
> 
> As per coding style, you should avoid using iterators and use indexes instead.
> 

I didn't know about that, could you show me the url of guide?

> > Source/WebKit/efl/ewk/ewk_file_chooser_settings.cpp:67
> > +        mimetypes = eina_list_append(mimetypes, strdup((*it).utf8().data()));
> 
> I thought we prefered to use eina_stringshare in EFL port.
> 

I agree. but we need to change strdup to eina_string_share in the ewk_intent like gyuyoung said.

> > Source/WebKit/efl/ewk/ewk_file_chooser_settings.h:107
> > + * @return one of these string "camera", "camcorder", "microphone" 
> 
> If the values are predetermined, why not use an enumeration instead? That would be much better for the clients.

I agree.

Thanks :)

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



More information about the webkit-unassigned mailing list