[Webkit-unassigned] [Bug 98931] [GTK] Add touch support

[bugzilla-daemon at webkit.org]

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





--- Comment #12 from Carlos Garnacho <carlosg at gnome.org>  2013-12-23 18:14:39 PST ---
Thanks for the review Martin, only a few comments below so far:

(In reply to comment #11)
> (From update of attachment 219947 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=219947&action=review
> 
> Thanks for updating the patch. I have a few, mostly stylistic, nits.
> 
> > Source/WebCore/PlatformGTK.cmake:631
> > +    if (ENABLE_TOUCH_EVENTS)
> > +        list(APPEND GObjectDOMBindings_IDL_FILES
> > +            dom/Touch.idl
> > +        )
> > +    endif ()
> > +
> 
> We are enabling touch events unconditionally for GTK+ and this conditional isn't mirrored in the autotools version of the build, so I think it makes sense to unconditionally add the file to the list here as well.

hmm, Isn't that done on the change to Source/autotools/SetupWebKitFeatures.m4? or should I be doing that somewhere else?

> 
> Thank you very kindly for updating the CMake build!

:), that was the part that I did somewhat more blindly tbh

> 
> > Source/WebKit2/Shared/gtk/WebEventFactory.cpp:207
> > +#ifndef GTK_API_VERSION_2
> 
> No need for this check here, since WebKit2 is only available for GTK+ 3 builds.

Isn't that code used on the webprocess? I got errors when compiling otherwise, from what I checked it was linking to gtk+2 at that time.

> > Source/WebKit2/Shared/gtk/WebEventFactory.cpp:272
> > +    return WebTouchEvent(type, touchPointList, modifiersForEvent(event), (double) gdk_event_get_time(event));
> 
> Please use a static_cast here instead of the C style cast. Does an implicit cast work?
> 

Right, saw other similar pieces of code casting implicitly, will update that

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