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

[bugzilla-daemon at webkit.org]

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





--- Comment #11 from Martin Robinson <mrobinson at webkit.org>  2013-12-23 17:33:38 PST ---
(From update of attachment 219947)
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.

Thank you very kindly for updating the CMake build!

> Source/WebCore/platform/gtk/GtkTouchContextHelper.cpp:37
> +bool GtkTouchContextHelper::handleEvent(GdkEvent *touchEvent)

The asterisk here is on the wrong side.

> Source/WebCore/platform/gtk/GtkTouchContextHelper.cpp:64
> +Vector<GdkEvent*> GtkTouchContextHelper::touchEvents(void) const
> +{
> +    Vector<GdkEvent*> touchEvents;
> +    copyValuesToVector(m_touchEvents, touchEvents);
> +    return touchEvents;
> +}
> +

Perhaps you could just do:

HashMapValuesProxy& GtkTouchContextHelper::touchEvents() const
{
    return m_touchEvents.values();
}

That would avoid the copy. WebKit uses empty argument lists instead of void.

> 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.

> Source/WebKit2/Shared/gtk/WebEventFactory.cpp:232
> +    gdouble root_x, root_y;

Please use rootX and rootY here.

> 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?

> Source/WebKit2/UIProcess/API/gtk/PageClientImpl.cpp:346
> +    const GdkEvent* touch_event = event.nativeEvent();
> +
> +    if (!touch_event->touch.emulating_pointer)
> +        return;

touch_event -> touchEvent.

> Source/WebKit2/UIProcess/API/gtk/PageClientImpl.cpp:349
> +

pointer_event -> pointerEvent.

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