[Webkit-unassigned] [Bug 95672] [EFL][WK2] Add javascript popup api.

[bugzilla-daemon at webkit.org]

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





--- Comment #4 from Byungwoo Lee <bw80.lee at samsung.com>  2012-09-04 06:26:23 PST ---
(From update of attachment 161938)
View in context: https://bugs.webkit.org/attachment.cgi?id=161938&action=review

Thank you for your comments.

>> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:106
>> +    OwnPtr<CString> javascriptPromptResult;
> 
> Why does does this need to be a pointer? What prevents you from using simply a CString?

I used pointer to release string data after returning ewk_view_run_javascript_prompt() function.
Otherwise, Ewk_View_Private_Data should keep the prompt result data.

>> Source/WebKit2/UIProcess/API/efl/ewk_view.h:239
>> + */
> 
> You should document that the strings are stringshared.

I think this structure have no need to use the stringshared.
There is no use case to return some string with this structure from application side to webkit side.
This structure type just passes the message or default value string through the callback function.
So user can just read strings and use it, and have no need to delete or change this.

>>> Source/WebKit2/UIProcess/API/efl/ewk_view_private.h:104
>>> +PassOwnPtr<CString> ewk_view_run_javascript_prompt(Evas_Object* ewkView, char* message, char* default_value);
>> 
>> s/defalut_value/defaultValue/g
> 
> Why does this function return a PassOwnPtr<CString> instead of a simple CString?

As replied above, to release prompt result string after the popup is finished.

>> Source/WebKit2/UIProcess/API/efl/ewk_view_ui_client.cpp:77
>> +    return WKStringCreateWithUTF8CString(result ? result->data() : "");
> 
> I think this one is better. 
> 
> result ? adoptWK(WKStringCreateWithUTF8CString(result->data())) : 0;

Ok I'll apply it. I found a bug about returning null with applying this comment and added it to the test case also.

>> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:276
>> +    destination->alert.message = strdup(source->alert.message);
> 
> Should use eina_stringshare_ref() instead of strdup().

Should this structure be changed to using eina stringshare because of this test case code?
Actually test case just use this structure to keep the strings for checking,
and can use some other way or define other structure for keeping the data.

If strdup in test case is a problem, how about changing the test case code?

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