Bug 90540 – [WK2][EFL] Ewk_View needs API to load HTML data

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

Now that the Ewk_View reports load errors, we need to handle them in MiniBrowser to display an error page.

[Chris Dumez]

Created attachment 150764 [details]
Patch

[Ryuan Choi]

Comment on attachment 150764 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=150764&action=review

> Source/WebKit2/ChangeLog:4
> +        [WK2][EFL] MiniBrowser needs to handle load errors
> +        https://bugs.webkit.org/show_bug.cgi?id=90540

IMO, more important change of this patch looks adding new API, right?

> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:692
> +Eina_Bool ewk_view_html_load(Evas_Object* ewkView, const char* html, const char* baseUrl, const char* unreachableUrl)

In WK1/Efl, we used ewk_frame_contents_set and ewk_frame_contents_alternate_set for same feature.

http://trac.webkit.org/browser/trunk/Source/WebKit/efl/ewk/ewk_frame.cpp#L422

[Chris Dumez]

Comment on attachment 150764 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=150764&action=review

>> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:692
>> +Eina_Bool ewk_view_html_load(Evas_Object* ewkView, const char* html, const char* baseUrl, const char* unreachableUrl)
> 
> In WK1/Efl, we used ewk_frame_contents_set and ewk_frame_contents_alternate_set for same feature.
> 
> http://trac.webkit.org/browser/trunk/Source/WebKit/efl/ewk/ewk_frame.cpp#L422

In WK2, we don't have frame-level APIs anymore so we are breaking compatibility with WK1 API anyway. Since we break compatibility, I decided to simplify the API a bit and propose something similar to what is in Qt port.
The method I'm proposing can achieve the same behavior as the 2 methods you mention. We don't loose any functionality.

[Ryuan Choi]

(In reply to comment #3)
> (From update of attachment 150764 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=150764&action=review
> 
> >> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:692
> >> +Eina_Bool ewk_view_html_load(Evas_Object* ewkView, const char* html, const char* baseUrl, const char* unreachableUrl)
> > 
> > In WK1/Efl, we used ewk_frame_contents_set and ewk_frame_contents_alternate_set for same feature.
> > 
> > http://trac.webkit.org/browser/trunk/Source/WebKit/efl/ewk/ewk_frame.cpp#L422
> 
> In WK2, we don't have frame-level APIs anymore so we are breaking compatibility with WK1 API anyway. Since we break compatibility, I decided to simplify the API a bit and propose something similar to what is in Qt port.
> The method I'm proposing can achieve the same behavior as the 2 methods you mention. We don't loose any functionality.

Although we don't break compatibility, is it better to use similar name for application developers to easily understand ?

I think that 1 method looks enough.

[Chris Dumez]

(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 150764 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=150764&action=review
> > 
> > >> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:692
> > >> +Eina_Bool ewk_view_html_load(Evas_Object* ewkView, const char* html, const char* baseUrl, const char* unreachableUrl)
> > > 
> > > In WK1/Efl, we used ewk_frame_contents_set and ewk_frame_contents_alternate_set for same feature.
> > > 
> > > http://trac.webkit.org/browser/trunk/Source/WebKit/efl/ewk/ewk_frame.cpp#L422
> > 
> > In WK2, we don't have frame-level APIs anymore so we are breaking compatibility with WK1 API anyway. Since we break compatibility, I decided to simplify the API a bit and propose something similar to what is in Qt port.
> > The method I'm proposing can achieve the same behavior as the 2 methods you mention. We don't loose any functionality.
> 
> Although we don't break compatibility, is it better to use similar name for application developers to easily understand ?
> 
> I think that 1 method looks enough.

Yes, I usually try to keep compatibility or at least stay close to the WK1 API. But in this case, I think it is a good opportunity to simplify the API. Also, I don't really like the WK1 function names for this and I don't find them clear enough. Unless there is strong opinion on Samsung side against it, I would like to land this patch as it is.

[Chris Dumez]

Created attachment 150783 [details]
Patch

Update bug title in Changelogs as Ryuan suggested.

[Kenneth Rohde Christiansen]

Comment on attachment 150783 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=150783&action=review

> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:692
> +Eina_Bool ewk_view_html_load(Evas_Object* ewkView, const char* html, const char* baseUrl, const char* unreachableUrl)

html_data_load ? btw, wouldnt it load svg as well?

[Chris Dumez]

Comment on attachment 150783 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=150783&action=review

>> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:692
>> +Eina_Bool ewk_view_html_load(Evas_Object* ewkView, const char* html, const char* baseUrl, const char* unreachableUrl)
> 
> html_data_load ? btw, wouldnt it load svg as well?

html_data_load(), why not if you prefer. I would even go for html_string_load() then. if you look at the methods I'm calling internally: loadHTMLString(), this would map quite closely. If it does load SVG, then the page API is confusing :)

[Kenneth Rohde Christiansen]

I am all for string_load :-)

[Chris Dumez]

Created attachment 150881 [details]
Patch

Take Kenneth's feedback into consideration.

[WebKit Review Bot]

Comment on attachment 150881 [details]
Patch

Clearing flags on attachment: 150881

Committed r121890: <http://trac.webkit.org/changeset/121890>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Hyerim Bae]

*** Bug 91352 has been marked as a duplicate of this bug. ***