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

[bugzilla-daemon at webkit.org]

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





--- Comment #17 from Carlos Garcia Campos <cgarcia at igalia.com>  2013-12-24 07:09:52 PST ---
(From update of attachment 219967)
View in context: https://bugs.webkit.org/attachment.cgi?id=219967&action=review

It looks much better, I have a just a few more minor comments. I also wonder if this makes tests in fast/events/touch pass, this bug is listed in the TestExpectations file, so I would expect a patch here to also unskip touch those tests (at least for WebKit2 tests in this case)

> Source/WebCore/platform/gtk/GtkTouchContextHelper.h:30
> +#include <wtf/Vector.h>

This is no longer needed now that you are returning the HashMap

> Source/WebCore/platform/gtk/GtkTouchContextHelper.h:37
> +class GtkTouchContextHelper {
> +public:

We probably want to make this non copyable. You only need to add here (before the public:) WTF_MAKE_NONCOPYABLE(GtkTouchContextHelper);

> Source/WebCore/platform/gtk/GtkTouchContextHelper.h:38
> +    GtkTouchContextHelper() { };

I don't understand why this is needed.

> Source/WebCore/platform/gtk/GtkTouchContextHelper.h:42
> +    const HashMap<uint32_t, GdkEvent*>& touchEvents() const { return m_touchEvents; };

I would use a typedef for this, something like

typedef TouchEventsMap HashMap<uint32_t, GdkEvent*>;

> Source/WebKit2/Shared/gtk/WebEventFactory.cpp:208
> +

Nit: remove this empty line here.

> Source/WebKit2/Shared/gtk/WebEventFactory.cpp:239
> +

Ditto.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:722
> +    WebKitWebViewBase* webViewBase = WEBKIT_WEB_VIEW_BASE(widget);
> +    WebKitWebViewBasePrivate* priv = webViewBase->priv;

We are only using priv, this could be just:

WebKitWebViewBasePrivate* priv = WEBKIT_WEB_VIEW_BASE(widget)->priv;

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