Override shortcuts only when an HTML input field has focus

Otherwise an application shortcut like Shift+Delete would no longer
work when webengine has focus (e.g. "delete mail" in KMail)

This removes unconditional calls to editorActionForKeyEvent for
ShortcutOverride event handling. We can remove those, because the key
sequences that are checked by editorActionForKeyEvent are a subset of
the key sequences checked by isCommonTextEditShortcut.

This amends commit 3902b27e.

Change-Id: I12a98368381edef36f11457c8b864d843efb871a
Reviewed-by: David Faure <david.faure@kdab.com>
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>

4 files changed, 20 insertions, 33 deletions

diff --git a/src/webengine/render_widget_host_view_qt_delegate_quick.cpp b/src/webengine/render_widget_host_view_qt_delegate_quick.cpp
index 749a2e0d8..b6069d519 100644
--- a/src/webengine/render_widget_host_view_qt_delegate_quick.cpp
+++ b/src/webengine/render_widget_host_view_qt_delegate_quick.cpp
@@ -240,14 +240,8 @@ void RenderWidgetHostViewQtDelegateQuick::inputMethodStateChanged(bool editorVis
 
 bool RenderWidgetHostViewQtDelegateQuick::event(QEvent *event)
 {
-    if (event->type() == QEvent::ShortcutOverride) {
-        QKeyEvent *keyEvent = static_cast<QKeyEvent *>(event);
-        if (m_client->handleShortcutOverrideEvent(keyEvent))
-            return true;
-        if (editorActionForKeyEvent(keyEvent) != QQuickWebEngineView::NoWebAction)
-            event->accept();
-        return true;
-    }
+    if (event->type() == QEvent::ShortcutOverride)
+        return m_client->handleShortcutOverrideEvent(static_cast<QKeyEvent *>(event));
 
     if (event->type() == QEvent::NativeGesture)
         return m_client->forwardEvent(event);
diff --git a/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp b/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp
index c608ba2aa..d02191b23 100644
--- a/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp
+++ b/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp
@@ -59,15 +59,6 @@
 
 namespace QtWebEngineCore {
 
-static bool handleShortcutOverrideEvent(RenderWidgetHostViewQtDelegateClient *client, QKeyEvent *ke)
-{
-    if (client->handleShortcutOverrideEvent(ke))
-        return true;
-    if (editorActionForKeyEvent(ke) != QWebEnginePage::NoWebAction)
-        ke->accept();
-    return true;
-}
-
 class RenderWidgetHostViewQuickItem : public QQuickItem {
 public:
     RenderWidgetHostViewQuickItem(RenderWidgetHostViewQtDelegateClient *client) : m_client(client)
@@ -79,10 +70,8 @@ public:
 protected:
     bool event(QEvent *event) override
     {
-        if (event->type() == QEvent::ShortcutOverride) {
-            handleShortcutOverrideEvent(m_client, static_cast<QKeyEvent *>(event));
-            return true;
-        }
+        if (event->type() == QEvent::ShortcutOverride)
+            return m_client->handleShortcutOverrideEvent(static_cast<QKeyEvent *>(event));
         return QQuickItem::event(event);
     }
     void focusInEvent(QFocusEvent *event) override
@@ -454,10 +443,8 @@ bool RenderWidgetHostViewQtDelegateWidget::event(QEvent *event)
         // We forward focus events later, once they have made it to the m_rootItem.
         return QQuickWidget::event(event);
     case QEvent::ShortcutOverride:
-        if (event->type() == QEvent::ShortcutOverride) {
-            handleShortcutOverrideEvent(m_client, static_cast<QKeyEvent *>(event));
+        if (m_client->handleShortcutOverrideEvent(static_cast<QKeyEvent *>(event)))
             return true;
-        }
         break;
     case QEvent::DragEnter:
     case QEvent::DragLeave:
diff --git a/tests/auto/widgets/qwebengineview/BLACKLIST b/tests/auto/widgets/qwebengineview/BLACKLIST
index 7121f7561..b3f393af4 100644
--- a/tests/auto/widgets/qwebengineview/BLACKLIST
+++ b/tests/auto/widgets/qwebengineview/BLACKLIST
@@ -3,3 +3,6 @@ windows
 
 [imeComposition]
 osx
+
+[inputFieldOverridesShortcuts]
+osx
diff --git a/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp b/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp
index c35617b3d..ad03839d6 100644
--- a/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp
+++ b/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp
@@ -1344,7 +1344,6 @@ void tst_QWebEngineView::inputFieldOverridesShortcuts()
 {
     bool actionTriggered = false;
     QAction *action = new QAction;
-    action->setShortcut(Qt::Key_X);
     connect(action, &QAction::triggered, [&actionTriggered] () { actionTriggered = true; });
 
     QWebEngineView view;
@@ -1352,7 +1351,7 @@ void tst_QWebEngineView::inputFieldOverridesShortcuts()
 
     QSignalSpy loadFinishedSpy(&view, SIGNAL(loadFinished(bool)));
     view.setHtml(QString("<html><body onload=\"input1=document.getElementById('input1')\">"
-                         "<input id=\"dummy\" type=\"text\">"
+                         "<button id=\"btn1\" type=\"button\">push it real good</button>"
                          "<input id=\"input1\" type=\"text\" value=\"x\">"
                          "</body></html>"));
     QVERIFY(loadFinishedSpy.wait());
@@ -1365,7 +1364,15 @@ void tst_QWebEngineView::inputFieldOverridesShortcuts()
                                       "input1.value").toString();
     };
 
+    // The input form is not focused. The action is triggered on pressing Shift+Delete.
+    action->setShortcut(Qt::SHIFT + Qt::Key_Delete);
+    QTest::keyClick(view.windowHandle(), Qt::Key_Delete, Qt::ShiftModifier);
+    QTRY_VERIFY(actionTriggered);
+    QCOMPARE(inputFieldValue(), QString("x"));
+
     // The input form is not focused. The action is triggered on pressing X.
+    action->setShortcut(Qt::Key_X);
+    actionTriggered = false;
     QTest::keyClick(view.windowHandle(), Qt::Key_X);
     QTRY_VERIFY(actionTriggered);
     QCOMPARE(inputFieldValue(), QString("x"));
@@ -1384,16 +1391,12 @@ void tst_QWebEngineView::inputFieldOverridesShortcuts()
     QTRY_VERIFY(actionTriggered);
     QCOMPARE(inputFieldValue(), QString("yx"));
 
-    // Remove focus from the input field. A QKeySequence::Copy action still must not be triggered.
-    evaluateJavaScriptSync(view.page(), "input1.blur();");
+    // Remove focus from the input field. A QKeySequence::Copy action must be triggerable.
+    evaluateJavaScriptSync(view.page(), "document.getElementById('btn1').focus();");
     action->setShortcut(QKeySequence::Copy);
     actionTriggered = false;
     QTest::keyClick(view.windowHandle(), Qt::Key_C, Qt::ControlModifier);
-    // Add some text in the input field to ensure that the key event went through.
-    evaluateJavaScriptSync(view.page(), "input1.focus();");
-    QTest::keyClick(view.windowHandle(), Qt::Key_U);
-    QTRY_COMPARE(inputFieldValue(), QString("yux"));
-    QVERIFY(!actionTriggered);
+    QTRY_VERIFY(actionTriggered);
 }
 
 class TestInputContext : public QPlatformInputContext