Bug 89095

Summary: [EFL][WK2] Add title support to Ewk_View
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebKit2Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: enmi.lee, kenneth, rakuco, ryuan.choi, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 61838    
Attachments:
Description Flags
Patch
none
Patch
kenneth: review+, cdumez: commit-queue-
Patch for landing none

Chris Dumez
Reported 2012-06-14 06:42:26 PDT
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.
Attachments
Patch (11.72 KB, patch)
2012-06-14 06:50 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (11.66 KB, patch)
2012-06-15 03:28 PDT, Chris Dumez 		kenneth: review+
cdumez: commit-queue-
DetailsFormatted DiffDiff
Patch for landing (11.70 KB, patch)
2012-06-15 04:03 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2012-06-14 06:50:00 PDT
Created attachment 147575 [details] Patch
Ryuan Choi
Comment 2 2012-06-14 16:17:53 PDT
Looks good to me.
Chris Dumez
Comment 3 2012-06-15 01:10:03 PDT
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
Comment 4 2012-06-15 01:12:35 PDT
> 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 5 2012-06-15 01:26:14 PDT
Comment on attachment 147575 [details] Patch Clear flags until I update the patch. Thanks for the feedback.
Chris Dumez
Comment 6 2012-06-15 01:31:36 PDT
(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
Comment 7 2012-06-15 03:26:21 PDT
(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
Comment 8 2012-06-15 03:28:51 PDT
Created attachment 147783 [details] Patch Take feedback into consideration.
Chris Dumez
Comment 9 2012-06-15 04:00:51 PDT
Comment on attachment 147783 [details] Patch Needs rebasing due to other patch that just landed.
Chris Dumez
Comment 10 2012-06-15 04:03:49 PDT
Created attachment 147791 [details] Patch for landing Rebasing on master. Could someone please cq+ ?
WebKit Review Bot
Comment 11 2012-06-15 05:17:12 PDT
Comment on attachment 147791 [details] Patch for landing Clearing flags on attachment: 147791 Committed r120442: <http://trac.webkit.org/changeset/120442>
WebKit Review Bot
Comment 12 2012-06-15 05:17:17 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.