WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
62363
[EFL][WK2] Add PageClientImpl and WebPageProxyEfl for efl port
https://bugs.webkit.org/show_bug.cgi?id=62363
Summary
[EFL][WK2] Add PageClientImpl and WebPageProxyEfl for efl port
Eunmi Lee
Reported
2011-06-09 02:08:02 PDT
I've added PageClientImpl and WebPageProxyEfl for efl port.
Attachments
Patch
(17.92 KB, patch)
2011-06-09 02:12 PDT
,
Eunmi Lee
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Eunmi Lee
Comment 1
2011-06-09 02:12:51 PDT
Created
attachment 96565
[details]
Patch
Kenneth Rohde Christiansen
Comment 2
2011-06-09 02:47:01 PDT
Comment on
attachment 96565
[details]
Patch LGTM as an initial commit
WebKit Review Bot
Comment 3
2011-06-09 03:27:01 PDT
Comment on
attachment 96565
[details]
Patch Clearing flags on attachment: 96565 Committed
r88440
: <
http://trac.webkit.org/changeset/88440
>
WebKit Review Bot
Comment 4
2011-06-09 03:27:06 PDT
All reviewed patches have been landed. Closing bug.
Leandro Pereira
Comment 5
2011-06-09 07:03:32 PDT
Comment on
attachment 96565
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=96565&action=review
As with other WebKit2 patches, I'm mostly reviewing for style/EFL usage issues.
> Source/WebKit2/UIProcess/API/efl/PageClientImpl.cpp:73 > +#define USE_EWK_TOUCH 1 > +#if USE_EWK_TOUCH > + evas_object_move(m_viewObject, 0, 0); > +#endif
Why this #define/#if?
> Source/WebKit2/UIProcess/API/efl/PageClientImpl.cpp:229 > +void PageClientImpl::findStringInCustomRepresentation(const String&, FindOptions, unsigned maxMatchCount)
maxMatchCount isn't used. Omit its name. There are other unused parameters that should have its name omitted as well, please review them.
> Source/WebKit2/UIProcess/API/efl/PageClientImpl.h:53 > + virtual void scrollView(const WebCore::IntRect& scrollRect, const WebCore::IntSize& scrollOffset);
No need to add parameter names on prototypes.
> Source/WebKit2/UIProcess/WebPageProxy.h:668 > -#if PLATFORM(GTK) > +#if PLATFORM(GTK) || PLATFORM(EFL) > void getEditorCommandsForKeyEvent(Vector<String>&); > #endif
This function is just a stub below; do we really need the stub and this change here?
> Source/WebKit2/UIProcess/efl/WebPageProxyEfl.cpp:51 > +#if PLATFORM(X11) > + platform = "X11"; > +#else > + platform = "Unknown"; > +#endif
Since Evas/Ecore allows you to use other backends without recompiling your program, this will fail. If the result of WebPageProxy::standardUserAgent() is cached somewhere, it would be nicer if the Evas backend could be queried in runtime. If not, you can leave it like this, as this is unlikely to impact anything anyway.
Eunmi Lee
Comment 6
2011-06-09 22:55:30 PDT
(In reply to
comment #5
)
> (From update of
attachment 96565
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=96565&action=review
> > As with other WebKit2 patches, I'm mostly reviewing for style/EFL usage issues. > > > Source/WebKit2/UIProcess/API/efl/PageClientImpl.cpp:73 > > +#define USE_EWK_TOUCH 1 > > +#if USE_EWK_TOUCH > > + evas_object_move(m_viewObject, 0, 0); > > +#endif > > Why this #define/#if?
Done. I'm sorry it is my mistake and it is unnecessary codes. I removed above codes.
> > > Source/WebKit2/UIProcess/API/efl/PageClientImpl.cpp:229 > > +void PageClientImpl::findStringInCustomRepresentation(const String&, FindOptions, unsigned maxMatchCount) > > maxMatchCount isn't used. Omit its name. There are other unused parameters that should have its name omitted as well, please review them.
Done. I removed all unused name from PageClientImpl.cpp.
> > > Source/WebKit2/UIProcess/API/efl/PageClientImpl.h:53 > > + virtual void scrollView(const WebCore::IntRect& scrollRect, const WebCore::IntSize& scrollOffset); > > No need to add parameter names on prototypes.
Done. I removed all parameter names from PageClientImpl.h
> > > Source/WebKit2/UIProcess/WebPageProxy.h:668 > > -#if PLATFORM(GTK) > > +#if PLATFORM(GTK) || PLATFORM(EFL) > > void getEditorCommandsForKeyEvent(Vector<String>&); > > #endif > > This function is just a stub below; do we really need the stub and this change here?
Done. It is not used now, so I removed that function.
> > > Source/WebKit2/UIProcess/efl/WebPageProxyEfl.cpp:51 > > +#if PLATFORM(X11) > > + platform = "X11"; > > +#else > > + platform = "Unknown"; > > +#endif > > Since Evas/Ecore allows you to use other backends without recompiling your program, this will fail. If the result of WebPageProxy::standardUserAgent() is cached somewhere, it would be nicer if the Evas backend could be queried in runtime. If not, you can leave it like this, as this is unlikely to impact anything anyway.
Yes, you are right. And we can check the Evas/Ecore backends using ecore_evas_engine_name_get(). However, we can not get the Ecore_Evas because standardUserAgent() is static function. So, I think I have to leave that codes. You can see the patch in the below url:
https://bugs.webkit.org/show_bug.cgi?id=62429
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug