Bug 89095 – [EFL][WK2] Add title support to Ewk_View

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Chris Dumez]

We need to add support for title getting and title change notification to Ewk_View.

This can then be used in MiniBrowser to set / update the window title.

[Chris Dumez]

Created attachment 147575 [details]
Patch

[Ryuan Choi]

Looks good to me.

[Chris Dumez]

I need this patch also for Ewk integration of Web Intents because it has the WKPageLoaderClient. I will need to define a callback for dispatchIntent() and emit a signal with the Ewk_Intent.

[Eunmi Lee]

> Source/WebKit2/UIProcess/API/efl/ewk_loader_client.cpp:29
> +#include "ewk_loader_client_private.h"
I think "ewk_loader_client_private.h"'s proper location is #27 line.

> Source/WebKit2/UIProcess/API/efl/ewk_loader_client.cpp:30
> +#include "ewk_view.h"
I think "ewk_view.h" is not needed in this file.

> Source/WebKit2/UIProcess/API/efl/ewk_loader_client.cpp:45
> +void ewk_loader_client_attach(WKPageRef pageRef, Evas_Object* ewkView)
How about changing the function name to "ewk_view_loader_client_attach(Evas_Object* ewkView, WKPageRef pageRef)" and file name to "ewk_view_loader_client.cpp"?
I think it it better to use "ewk_view" prefix because loader client will be set for ewk_view.

> Source/WebKit2/UIProcess/API/efl/ewk_loader_client_private.h: 32
> +void ewk_loader_client_attach(WKPageRef pageRef, Evas_Object *o);
Would you change "Evas_Object *o" to "Evas_Object* o"? This file has to follow WebKit style because it is private header.

> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:549
> +evas_object_smart_callback_call(ewkView, "title,changed", (void*)title);
Please use static_cast<void*>() instead of (void*).

[Chris Dumez]

Comment on attachment 147575 [details]
Patch

Clear flags until I update the patch. Thanks for the feedback.

[Chris Dumez]

(In reply to comment #4)
> > Source/WebKit2/UIProcess/API/efl/ewk_loader_client.cpp:29
> > +#include "ewk_loader_client_private.h"
> I think "ewk_loader_client_private.h"'s proper location is #27 line.

What you're suggesting would be right if it was a public header (i.e. "ewk_loader_client.h"). However, I believe the current location is correct considering it is a private header. Also, if I move the line, the style check script starts complaining.

[Eunmi Lee]

(In reply to comment #6)
> (In reply to comment #4)
> > > Source/WebKit2/UIProcess/API/efl/ewk_loader_client.cpp:29
> > > +#include "ewk_loader_client_private.h"
> > I think "ewk_loader_client_private.h"'s proper location is #27 line.
> 
> What you're suggesting would be right if it was a public header (i.e. "ewk_loader_client.h"). However, I believe the current location is correct considering it is a private header. Also, if I move the line, the style check script starts complaining.

Oh, I see. you are right :)

[Chris Dumez]

Created attachment 147783 [details]
Patch

Take feedback into consideration.

[Chris Dumez]

Comment on attachment 147783 [details]
Patch

Needs rebasing due to other patch that just landed.

[Chris Dumez]

Created attachment 147791 [details]
Patch for landing

Rebasing on master. Could someone please cq+ ?

[WebKit Review Bot]

Comment on attachment 147791 [details]
Patch for landing

Clearing flags on attachment: 147791

Committed r120442: <http://trac.webkit.org/changeset/120442>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.