[Webkit-unassigned] [Bug 83865] [EFL] Add Web Inspector to WebKit-EFL

[bugzilla-daemon at webkit.org]

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





--- Comment #12 from Seokju Kwon <seokju.kwon at samsung.com>  2012-05-31 01:58:58 PST ---
(In reply to comment #11)
> (From update of attachment 143527 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=143527&action=review
> 
> > Source/WebKit/efl/WebCoreSupport/InspectorClientEfl.cpp:27
> > +#include "ewk_private.h"
> 
> Remove unneeded include.
I will remove it.

> > Source/WebKit/efl/WebCoreSupport/InspectorClientEfl.cpp:56
> > +InspectorClientEfl::InspectorClientEfl(Evas_Object* view)
> 
> I wonder whether view is webview. 157 line have used inspectorView for parameter name. Don't you need more clear name?
You're right. webView is better than view. I will fix it.

> > Source/WebKit/efl/WebCoreSupport/InspectorClientEfl.cpp:129
> > +#if ENABLE(INSPECTOR)
> 
> By the way, should we use ENABLE(INSPECTOR) macro in each function? I think we can do InspectorClientEfl.cpp isn't included in CMake file When ENABLE_INSPECTOR feature is disabled. If so, we don't need to use ENABLE(INSPECTOR) macro per a function.
InspectorClientEfl can be created regardless of ENABLE_INSPECTOR feature in _ewk_view_priv_new function. I can remove some macro only to prevent build break minimally, (for example WEB_INSPECTOR_DIR, InspectorController) if you want it. 

> > Source/WebKit/efl/WebCoreSupport/InspectorClientEfl.cpp:146
> > +    String inspectorFilesPath = "file://"WEB_INSPECTOR_INSTALL_DIR"/inspector.html";
> 
> Use string utility function to combine multiple strings. For example, makeString().
Oh thanks, I will use it.

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