Text selected with double-tap should not cause vibration

third_party/WebKit/Source/web/ContextMenuClientImpl.cpp

Index: third_party/WebKit/Source/web/ContextMenuClientImpl.cpp
diff --git a/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp b/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp
index ccdf9a29b18e06de6d153f65ae55c93abeedda75..7a93740c23fa3daebf54993ec5c371f09caa4835 100644
--- a/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp
+++ b/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp
@@ -131,7 +131,7 @@ static String selectMisspellingAsync(LocalFrame* selectedFrame, String& descript
     return markerRange->text();
 }
 
-void ContextMenuClientImpl::showContextMenu(const ContextMenu* defaultMenu)
+bool ContextMenuClientImpl::showContextMenu(const ContextMenu* defaultMenu)
 {
     // Displaying the context menu in this function is a big hack as we don't
     // have context, i.e. whether this is being invoked via a script or in
@@ -139,7 +139,7 @@ void ContextMenuClientImpl::showContextMenu(const ContextMenu* defaultMenu)
     // Keyboard events KeyVK_APPS, Shift+F10). Check if this is being invoked
     // in response to the above input events before popping up the context menu.
     if (!ContextMenuAllowedScope::isContextMenuAllowed())
-        return;
+        return true;

[aelias_OOO_until_Jul13, 2016/05/18 01:22:32]

As discussed offline, it should be fine for this to be false.

[amaralp, 2016/05/18 22:16:18]

Done.

 
     HitTestResult r = m_webView->page()->contextMenuController().hitTestResult();
 
@@ -362,6 +362,15 @@ void ContextMenuClientImpl::showContextMenu(const ContextMenu* defaultMenu)
     selectedWebFrame->setContextMenuNode(r.innerNodeOrImageMapImage());
     if (selectedWebFrame->client())
         selectedWebFrame->client()->showContextMenu(data);
+
+#if OS(ANDROID)

[aelias_OOO_until_Jul13, 2016/05/18 01:22:33]

Instead of #if OS(ANDROID), let's make this logic conditional on whether the
event type is a LongPress or a Click.  That event information isn't available in
this scope, so I'd suggest adding an additional argument to this method passing
it in (probably just the event type enum).

[amaralp, 2016/05/18 22:16:18]

Done.

+    if (!data.linkURL.isEmpty()
+        || data.mediaType == WebContextMenuData::MediaTypeImage
+        || data.mediaType == WebContextMenuData::MediaTypeVideo)
+        return true;

[aelias_OOO_until_Jul13, 2016/05/18 01:22:33]

I think this should be an early-return above the showContextMenu() call, to
substitute for the Java shouldShowContextMenu() logic you deleted.  How about
putting this early-return at the beginning next to
ContextMenuAllowedScope::isContextMenuAllowed()?

[amaralp, 2016/05/18 22:16:18]

I rely on the data variable being populated so I can't do this at the very top
like ContextMenuAllowedScope::isContextMenuAllowed(), but I moved it up a little
bit.

+    return false;
+#endif
+    return true;
 }
 
 void ContextMenuClientImpl::clearContextMenu()