Diff
Modified: trunk/Source/WebCore/ChangeLog (126944 => 126945)
--- trunk/Source/WebCore/ChangeLog 2012-08-29 00:47:37 UTC (rev 126944)
+++ trunk/Source/WebCore/ChangeLog 2012-08-29 01:05:41 UTC (rev 126945)
@@ -1,3 +1,18 @@
+2012-08-28 Leandro Gracia Gil <[email protected]>
+
+ Content detection should not disrupt the page behaviour
+ https://bugs.webkit.org/show_bug.cgi?id=94727
+
+ Reviewed by Adam Barth.
+
+ Tested by WebViewTest::DetectContentAroundPosition.
+
+ * dom/Node.cpp:
+ (WebCore::Node::willRespondToTouchEvents): checks if a node listens to touch events. Very similar to willRespondToMouseClickEvents.
+ (WebCore):
+ * dom/Node.h:
+ (Node):
+
2012-08-28 Simon Fraser <[email protected]>
Handle sticky that overflows its container
Modified: trunk/Source/WebCore/dom/Node.cpp (126944 => 126945)
--- trunk/Source/WebCore/dom/Node.cpp 2012-08-29 00:47:37 UTC (rev 126944)
+++ trunk/Source/WebCore/dom/Node.cpp 2012-08-29 01:05:41 UTC (rev 126945)
@@ -2765,6 +2765,17 @@
return isContentEditable() || hasEventListeners(eventNames().mouseupEvent) || hasEventListeners(eventNames().mousedownEvent) || hasEventListeners(eventNames().clickEvent) || hasEventListeners(eventNames().DOMActivateEvent);
}
+bool Node::willRespondToTouchEvents()
+{
+#if ENABLE(TOUCH_EVENTS)
+ if (disabled())
+ return false;
+ return hasEventListeners(eventNames().touchstartEvent) || hasEventListeners(eventNames().touchmoveEvent) || hasEventListeners(eventNames().touchcancelEvent) || hasEventListeners(eventNames().touchendEvent);
+#else
+ return false;
+#endif
+}
+
#if ENABLE(MICRODATA)
DOMSettableTokenList* Node::itemProp()
{
Modified: trunk/Source/WebCore/dom/Node.h (126944 => 126945)
--- trunk/Source/WebCore/dom/Node.h 2012-08-29 00:47:37 UTC (rev 126944)
+++ trunk/Source/WebCore/dom/Node.h 2012-08-29 01:05:41 UTC (rev 126945)
@@ -584,6 +584,7 @@
virtual bool willRespondToMouseMoveEvents();
virtual bool willRespondToMouseClickEvents();
+ virtual bool willRespondToTouchEvents();
PassRefPtr<Element> querySelector(const AtomicString& selectors, ExceptionCode&);
PassRefPtr<NodeList> querySelectorAll(const AtomicString& selectors, ExceptionCode&);
Modified: trunk/Source/WebKit/chromium/ChangeLog (126944 => 126945)
--- trunk/Source/WebKit/chromium/ChangeLog 2012-08-29 00:47:37 UTC (rev 126944)
+++ trunk/Source/WebKit/chromium/ChangeLog 2012-08-29 01:05:41 UTC (rev 126945)
@@ -1,3 +1,20 @@
+2012-08-28 Leandro Gracia Gil <[email protected]>
+
+ Content detection should not disrupt the page behaviour
+ https://bugs.webkit.org/show_bug.cgi?id=94727
+
+ Reviewed by Adam Barth.
+
+ Triggers content detection in the embedder on tap gestures and
+ add checks for the appropriate event listeners in order to prevent
+ triggering content detection when it would disrupt the page's behaviour.
+
+ * src/WebViewImpl.cpp:
+ (WebKit::WebViewImpl::handleGestureEvent):
+ (WebKit::WebViewImpl::detectContentOnTouch):
+ * tests/WebViewTest.cpp:
+ * tests/data/content_listeners.html: Added.
+
2012-08-28 Sheriff Bot <[email protected]>
Unreviewed, rolling out r126933.
Modified: trunk/Source/WebKit/chromium/src/WebViewImpl.cpp (126944 => 126945)
--- trunk/Source/WebKit/chromium/src/WebViewImpl.cpp 2012-08-29 00:47:37 UTC (rev 126944)
+++ trunk/Source/WebKit/chromium/src/WebViewImpl.cpp 2012-08-29 01:05:41 UTC (rev 126945)
@@ -687,6 +687,7 @@
{
switch (event.type) {
case WebInputEvent::GestureFlingStart: {
+ m_client->cancelScheduledContentIntents();
m_lastWheelPosition = WebPoint(event.x, event.y);
m_lastWheelGlobalPosition = WebPoint(event.globalX, event.globalY);
m_flingModifier = event.modifiers;
@@ -702,6 +703,10 @@
}
return false;
case WebInputEvent::GestureTap: {
+ m_client->cancelScheduledContentIntents();
+ if (detectContentOnTouch(WebPoint(event.x, event.y), event.type))
+ return true;
+
PlatformGestureEventBuilder platformEvent(mainFrameImpl()->frameView(), event);
RefPtr<WebCore::PopupContainer> selectPopup;
selectPopup = m_selectPopup;
@@ -722,6 +727,10 @@
if (!mainFrameImpl() || !mainFrameImpl()->frameView())
return false;
+ m_client->cancelScheduledContentIntents();
+ if (detectContentOnTouch(WebPoint(event.x, event.y), event.type))
+ return true;
+
m_page->contextMenuController()->clearContextMenu();
m_contextMenuAllowed = true;
PlatformGestureEventBuilder platformEvent(mainFrameImpl()->frameView(), event);
@@ -730,6 +739,7 @@
return handled;
}
case WebInputEvent::GestureTapDown: {
+ m_client->cancelScheduledContentIntents();
// Queue a highlight animation, then hand off to regular handler.
#if OS(LINUX)
enableTouchHighlight(IntPoint(event.x, event.y));
@@ -737,11 +747,12 @@
PlatformGestureEventBuilder platformEvent(mainFrameImpl()->frameView(), event);
return mainFrameImpl()->frame()->eventHandler()->handleGestureEvent(platformEvent);
}
+ case WebInputEvent::GestureDoubleTap:
case WebInputEvent::GestureScrollBegin:
+ case WebInputEvent::GesturePinchBegin:
+ m_client->cancelScheduledContentIntents();
case WebInputEvent::GestureScrollEnd:
case WebInputEvent::GestureScrollUpdate:
- case WebInputEvent::GestureDoubleTap:
- case WebInputEvent::GesturePinchBegin:
case WebInputEvent::GesturePinchEnd:
case WebInputEvent::GesturePinchUpdate: {
PlatformGestureEventBuilder platformEvent(mainFrameImpl()->frameView(), event);
@@ -3990,7 +4001,14 @@
if (!node || !node->isTextNode())
return false;
- // FIXME: Should we not detect content intents in nodes that have event listeners?
+ // Ignore when tapping on links or nodes listening to click events, unless the click event is on the
+ // body element, in which case it's unlikely that the original node itself was intended to be clickable.
+ for (; node && !node->hasTagName(HTMLNames::bodyTag); node = node->parentNode()) {
+ if (node->isLink() || (touchType != WebInputEvent::GestureLongPress
+ && (node->willRespondToTouchEvents() || node->willRespondToMouseClickEvents()))) {
+ return false;
+ }
+ }
WebContentDetectionResult content = m_client->detectContentAround(touchHit);
if (!content.isValid())
Modified: trunk/Source/WebKit/chromium/tests/WebViewTest.cpp (126944 => 126945)
--- trunk/Source/WebKit/chromium/tests/WebViewTest.cpp 2012-08-29 00:47:37 UTC (rev 126944)
+++ trunk/Source/WebKit/chromium/tests/WebViewTest.cpp 2012-08-29 01:05:41 UTC (rev 126945)
@@ -32,14 +32,18 @@
#include "WebView.h"
#include "Document.h"
+#include "Element.h"
#include "FrameTestHelpers.h"
#include "FrameView.h"
#include "HTMLDocument.h"
#include "URLTestHelpers.h"
+#include "WebContentDetectionResult.h"
#include "WebDocument.h"
+#include "WebElement.h"
#include "WebFrame.h"
#include "WebFrameClient.h"
#include "WebFrameImpl.h"
+#include "WebInputEvent.h"
#include "platform/WebSize.h"
#include "WebViewClient.h"
#include "WebViewImpl.h"
@@ -418,4 +422,132 @@
webView->close();
}
+class ContentDetectorClient : public WebViewClient {
+public:
+ ContentDetectorClient() { reset(); }
+
+ virtual WebContentDetectionResult detectContentAround(const WebHitTestResult& hitTest) OVERRIDE
+ {
+ m_contentDetectionRequested = true;
+ return m_contentDetectionResult;
+ }
+
+ virtual void scheduleContentIntent(const WebURL& url) OVERRIDE
+ {
+ m_scheduledIntentURL = url;
+ }
+
+ virtual void cancelScheduledContentIntents() OVERRIDE
+ {
+ m_pendingIntentsCancelled = true;
+ }
+
+ void reset()
+ {
+ m_contentDetectionRequested = false;
+ m_pendingIntentsCancelled = false;
+ m_scheduledIntentURL = WebURL();
+ m_contentDetectionResult = WebContentDetectionResult();
+ }
+
+ bool contentDetectionRequested() const { return m_contentDetectionRequested; }
+ bool pendingIntentsCancelled() const { return m_pendingIntentsCancelled; }
+ const WebURL& scheduledIntentURL() const { return m_scheduledIntentURL; }
+ void setContentDetectionResult(const WebContentDetectionResult& result) { m_contentDetectionResult = result; }
+
+private:
+ bool m_contentDetectionRequested;
+ bool m_pendingIntentsCancelled;
+ WebURL m_scheduledIntentURL;
+ WebContentDetectionResult m_contentDetectionResult;
+};
+
+static bool tapElementById(WebView* webView, WebInputEvent::Type type, const WebString& id)
+{
+ ASSERT(webView);
+ RefPtr<WebCore::Element> element = static_cast<PassRefPtr<WebCore::Element> >(webView->mainFrame()->document().getElementById(id));
+ if (!element)
+ return false;
+
+ element->scrollIntoViewIfNeeded();
+ WebCore::IntPoint center = element->screenRect().center();
+
+ WebGestureEvent event;
+ event.type = type;
+ event.x = center.x();
+ event.y = center.y();
+
+ webView->handleInputEvent(event);
+ webkit_support::RunAllPendingMessages();
+ return true;
}
+
+TEST_F(WebViewTest, DetectContentAroundPosition)
+{
+ URLTestHelpers::registerMockedURLFromBaseURL(WebString::fromUTF8(m_baseURL.c_str()), WebString::fromUTF8("content_listeners.html"));
+
+ ContentDetectorClient client;
+ WebView* webView = FrameTestHelpers::createWebViewAndLoad(m_baseURL + "content_listeners.html", true, 0, &client);
+ webView->resize(WebSize(500, 300));
+ webView->layout();
+ webkit_support::RunAllPendingMessages();
+
+ WebString clickListener = WebString::fromUTF8("clickListener");
+ WebString touchstartListener = WebString::fromUTF8("touchstartListener");
+ WebString mousedownListener = WebString::fromUTF8("mousedownListener");
+ WebString noListener = WebString::fromUTF8("noListener");
+ WebString link = WebString::fromUTF8("link");
+
+ // Ensure content detection is not requested for nodes listening to click,
+ // mouse or touch events when we do simple taps.
+ EXPECT_TRUE(tapElementById(webView, WebInputEvent::GestureTap, clickListener));
+ EXPECT_FALSE(client.contentDetectionRequested());
+ client.reset();
+
+ EXPECT_TRUE(tapElementById(webView, WebInputEvent::GestureTap, touchstartListener));
+ EXPECT_FALSE(client.contentDetectionRequested());
+ client.reset();
+
+ EXPECT_TRUE(tapElementById(webView, WebInputEvent::GestureTap, mousedownListener));
+ EXPECT_FALSE(client.contentDetectionRequested());
+ client.reset();
+
+ // Content detection should still work on click, mouse and touch event listeners for long taps
+ // as long as we're not tapping on links.
+ EXPECT_TRUE(tapElementById(webView, WebInputEvent::GestureLongPress, clickListener));
+ EXPECT_TRUE(client.contentDetectionRequested());
+ client.reset();
+
+ EXPECT_TRUE(tapElementById(webView, WebInputEvent::GestureLongPress, touchstartListener));
+ EXPECT_TRUE(client.contentDetectionRequested());
+ client.reset();
+
+ EXPECT_TRUE(tapElementById(webView, WebInputEvent::GestureLongPress, mousedownListener));
+ EXPECT_TRUE(client.contentDetectionRequested());
+ client.reset();
+
+ EXPECT_TRUE(tapElementById(webView, WebInputEvent::GestureLongPress, link));
+ EXPECT_FALSE(client.contentDetectionRequested());
+ client.reset();
+
+ // Content detection should work normally without these event listeners.
+ // The click listener in the body should be ignored as a special case.
+ EXPECT_TRUE(tapElementById(webView, WebInputEvent::GestureTap, noListener));
+ EXPECT_TRUE(client.contentDetectionRequested());
+ EXPECT_FALSE(client.scheduledIntentURL().isValid());
+
+ WebURL intentURL = toKURL(m_baseURL);
+ client.setContentDetectionResult(WebContentDetectionResult(WebRange(), WebString(), intentURL));
+ EXPECT_TRUE(tapElementById(webView, WebInputEvent::GestureTap, noListener));
+ EXPECT_TRUE(client.scheduledIntentURL() == intentURL);
+
+ // Tapping elsewhere should cancel the scheduled intent.
+ WebGestureEvent event;
+ event.type = WebInputEvent::GestureTap;
+ webView->handleInputEvent(event);
+ webkit_support::RunAllPendingMessages();
+ EXPECT_TRUE(client.pendingIntentsCancelled());
+ webView->close();
+}
+
+}
Added: trunk/Source/WebKit/chromium/tests/data/content_listeners.html (0 => 126945)
--- trunk/Source/WebKit/chromium/tests/data/content_listeners.html (rev 0)
+++ trunk/Source/WebKit/chromium/tests/data/content_listeners.html 2012-08-29 01:05:41 UTC (rev 126945)
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+span {
+ font-size: 300%;
+}
+</style>
+<script>
+function listener() {
+}
+</script>
+</head>
+<body _onclick_="listener()">
+<span id="clickListener" _onclick_="listener()">
+This has a click listener.
+</span></br>
+<span id="touchstartListener" _ontouchstart_="listener()">
+This has a touchstart listener.
+</span></br>
+<span id="mousedownListener" _onmousedown_="listener()">
+This has a mousedown listener.
+</span></br>
+<span id="noListener">
+This has no specific listener (the body listener should be ignored).
+</span></br>
+<a href="" id="link">
+This has no specific listener, but it's a link.
+</a></br>
+</body>
+</html>
