From 1e89fa4226be870d97d3b21c6c7087886c81bbc5 Mon Sep 17 00:00:00 2001
From: Peter Varga <pvarga@inf.u-szeged.hu>
Date: Tue, 11 Sep 2018 15:48:43 +0200
Subject: [PATCH] Clean up WebEngineAction API

- Rename iconText to iconName
- Remove unused QQuickWebEngineAction::toggled signal
- Remove argument of QQuickWebEngineAction::enabledChanged signal

Change-Id: I37172c096003eea58e567753265abd91679dacf1
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
 src/webengine/api/qquickwebengineaction.cpp   | 19 ++--
 src/webengine/api/qquickwebengineaction_p.h   |  9 +-
 src/webengine/api/qquickwebengineaction_p_p.h |  4 +-
 src/webengine/api/qquickwebengineview.cpp     | 14 +--
 src/webengine/plugin/plugins.qmltypes         |  7 +-
 src/webengine/ui_delegates_manager.cpp        |  2 +-
 tests/auto/quick/publicapi/tst_publicapi.cpp  |  5 +-
 tests/auto/quick/qmltests/data/tst_action.qml | 88 +++++++++----------
 8 files changed, 71 insertions(+), 77 deletions(-)

diff --git a/src/webengine/api/qquickwebengineaction.cpp b/src/webengine/api/qquickwebengineaction.cpp
index a0be20b54..16eef04d3 100644
--- a/src/webengine/api/qquickwebengineaction.cpp
+++ b/src/webengine/api/qquickwebengineaction.cpp
@@ -68,10 +68,10 @@ QT_BEGIN_NAMESPACE
     \endcode
 */
 
-QQuickWebEngineActionPrivate::QQuickWebEngineActionPrivate(const QVariant &data, const QString &text, const QString &iconText, bool enabled)
+QQuickWebEngineActionPrivate::QQuickWebEngineActionPrivate(const QVariant &data, const QString &text, const QString &iconName, bool enabled)
     : m_data(data)
     , m_text(text)
-    , m_iconText(iconText)
+    , m_iconName(iconName)
     , m_enabled(enabled)
 {
 }
@@ -86,7 +86,7 @@ void QQuickWebEngineActionPrivate::setEnabled(bool enabled)
     if (m_enabled == enabled)
         return;
     m_enabled = enabled;
-    emit q->enabledChanged(enabled);
+    emit q->enabledChanged();
 }
 
 QVariant QQuickWebEngineActionPrivate::data() const
@@ -102,9 +102,9 @@ void QQuickWebEngineActionPrivate::trigger()
     }
 }
 
-QQuickWebEngineAction::QQuickWebEngineAction(const QVariant &data, const QString &text, const QString &iconText, bool enabled, QObject *parent)
+QQuickWebEngineAction::QQuickWebEngineAction(const QVariant &data, const QString &text, const QString &iconName, bool enabled, QObject *parent)
     : QObject(parent)
-    , d_ptr(new QQuickWebEngineActionPrivate(data, text, iconText, enabled))
+    , d_ptr(new QQuickWebEngineActionPrivate(data, text, iconName, enabled))
 {
     d_ptr->q_ptr = this;
 }
@@ -132,14 +132,15 @@ QString QQuickWebEngineAction::text() const
 }
 
 /*!
-    \qmlproperty string WebEngineAction::iconText
+    \qmlproperty string WebEngineAction::iconName
 
-    This property holds the action's descriptive icon text.
+    This property holds the name of the icon for the action. This name
+    can be used to pick the icon from a theme.
 */
-QString QQuickWebEngineAction::iconText() const
+QString QQuickWebEngineAction::iconName() const
 {
     Q_D(const QQuickWebEngineAction);
-    return d->m_iconText;
+    return d->m_iconName;
 }
 
 /*!
diff --git a/src/webengine/api/qquickwebengineaction_p.h b/src/webengine/api/qquickwebengineaction_p.h
index 5296f9dd6..8f5f3386c 100644
--- a/src/webengine/api/qquickwebengineaction_p.h
+++ b/src/webengine/api/qquickwebengineaction_p.h
@@ -69,25 +69,24 @@ class Q_WEBENGINE_EXPORT QQuickWebEngineAction : public QObject
 {
     Q_OBJECT
     Q_PROPERTY(QString text READ text CONSTANT FINAL)
-    Q_PROPERTY(QString iconText READ iconText CONSTANT FINAL)
+    Q_PROPERTY(QString iconName READ iconName CONSTANT FINAL)
     Q_PROPERTY(bool enabled READ isEnabled NOTIFY enabledChanged FINAL)
 
 public:
-    QQuickWebEngineAction(const QVariant &data, const QString &text, const QString &iconText, bool enabled, QObject *parent);
+    QQuickWebEngineAction(const QVariant &data, const QString &text, const QString &iconName, bool enabled, QObject *parent);
     QQuickWebEngineAction(QObject *parent);
     ~QQuickWebEngineAction();
 
     QString text() const;
-    QString iconText() const;
+    QString iconName() const;
     bool isEnabled() const;
 
 public Q_SLOTS:
     Q_INVOKABLE void trigger();
 
 Q_SIGNALS:
-    void toggled();
     void triggered();
-    void enabledChanged(const bool enabled);
+    void enabledChanged();
 
 private:
     Q_DECLARE_PRIVATE(QQuickWebEngineAction)
diff --git a/src/webengine/api/qquickwebengineaction_p_p.h b/src/webengine/api/qquickwebengineaction_p_p.h
index cb1817e55..4320f73e4 100644
--- a/src/webengine/api/qquickwebengineaction_p_p.h
+++ b/src/webengine/api/qquickwebengineaction_p_p.h
@@ -63,7 +63,7 @@ class QQuickWebEngineActionPrivate
 {
 public:
     Q_DECLARE_PUBLIC(QQuickWebEngineAction)
-    QQuickWebEngineActionPrivate(const QVariant &data, const QString &text, const QString &iconText, bool enabled);
+    QQuickWebEngineActionPrivate(const QVariant &data, const QString &text, const QString &iconName, bool enabled);
     ~QQuickWebEngineActionPrivate();
 
     void setEnabled(bool enabled);
@@ -77,7 +77,7 @@ private:
 
     QVariant m_data;
     QString m_text;
-    QString m_iconText;
+    QString m_iconName;
     bool m_enabled;
 };
 
diff --git a/src/webengine/api/qquickwebengineview.cpp b/src/webengine/api/qquickwebengineview.cpp
index 6bf23ea7b..116f49c3c 100644
--- a/src/webengine/api/qquickwebengineview.cpp
+++ b/src/webengine/api/qquickwebengineview.cpp
@@ -1768,30 +1768,30 @@ QQuickWebEngineAction *QQuickWebEngineView::action(WebAction action)
     }
 
     QString text;
-    QString iconText;
+    QString iconName;
 
     switch (action) {
     case Back:
         text = RenderViewContextMenuQt::getMenuItemName(RenderViewContextMenuQt::ContextMenuItem::Back);
-        iconText = QStringLiteral("go-previous");
+        iconName = QStringLiteral("go-previous");
         break;
     case Forward:
         text = RenderViewContextMenuQt::getMenuItemName(RenderViewContextMenuQt::ContextMenuItem::Forward);
-        iconText = QStringLiteral("go-next");
+        iconName = QStringLiteral("go-next");
         break;
     case Stop:
         text = tr("Stop");
         break;
     case Reload:
         text = RenderViewContextMenuQt::getMenuItemName(RenderViewContextMenuQt::ContextMenuItem::Reload);
-        iconText = QStringLiteral("view-refresh");
+        iconName = QStringLiteral("view-refresh");
         break;
     case ReloadAndBypassCache:
         text = tr("Reload and Bypass Cache");
         break;
     case Cut:
         text = RenderViewContextMenuQt::getMenuItemName(RenderViewContextMenuQt::ContextMenuItem::Cut);
-        iconText = QStringLiteral("Cut");
+        iconName = QStringLiteral("Cut");
         break;
     case Copy:
         text = RenderViewContextMenuQt::getMenuItemName(RenderViewContextMenuQt::ContextMenuItem::Copy);
@@ -1870,7 +1870,7 @@ QQuickWebEngineAction *QQuickWebEngineView::action(WebAction action)
         break;
     case ViewSource:
         text = RenderViewContextMenuQt::getMenuItemName(RenderViewContextMenuQt::ContextMenuItem::ViewSource);
-        iconText = QStringLiteral("view-source");
+        iconName = QStringLiteral("view-source");
         break;
     case ToggleBold:
         text = tr("&Bold");
@@ -1914,7 +1914,7 @@ QQuickWebEngineAction *QQuickWebEngineView::action(WebAction action)
         break;
     }
 
-    QQuickWebEngineAction *retVal = new QQuickWebEngineAction(action, text, iconText, false, this);
+    QQuickWebEngineAction *retVal = new QQuickWebEngineAction(action, text, iconName, false, this);
 
     d->actions[action] = retVal;
     d->updateAction(action);
diff --git a/src/webengine/plugin/plugins.qmltypes b/src/webengine/plugin/plugins.qmltypes
index a17bcfbab..1f295ac57 100644
--- a/src/webengine/plugin/plugins.qmltypes
+++ b/src/webengine/plugin/plugins.qmltypes
@@ -15,14 +15,9 @@ Module {
         isCreatable: false
         exportMetaObjectRevisions: [0]
         Property { name: "text"; type: "string"; isReadonly: true }
-        Property { name: "iconText"; type: "string"; isReadonly: true }
+        Property { name: "iconName"; type: "string"; isReadonly: true }
         Property { name: "enabled"; type: "bool"; isReadonly: true }
-        Signal { name: "toggled" }
         Signal { name: "triggered" }
-        Signal {
-            name: "enabledChanged"
-            Parameter { name: "enabled"; type: "bool" }
-        }
         Method { name: "trigger" }
     }
     Component {
diff --git a/src/webengine/ui_delegates_manager.cpp b/src/webengine/ui_delegates_manager.cpp
index c35d26017..252bdc9b6 100644
--- a/src/webengine/ui_delegates_manager.cpp
+++ b/src/webengine/ui_delegates_manager.cpp
@@ -215,7 +215,7 @@ void UIDelegatesManager::addMenuItem(QQuickWebEngineAction *action, QObject *men
     QObject *it = menuItemComponent->beginCreate(qmlContext(m_view));
 
     QQmlProperty(it, QStringLiteral("text")).write(action->text());
-    QQmlProperty(it, QStringLiteral("iconName")).write(action->iconText());
+    QQmlProperty(it, QStringLiteral("iconName")).write(action->iconName());
     QQmlProperty(it, QStringLiteral("enabled")).write(action->isEnabled());
     QQmlProperty(it, QStringLiteral("checkable")).write(checkable);
     QQmlProperty(it, QStringLiteral("checked")).write(checked);
diff --git a/tests/auto/quick/publicapi/tst_publicapi.cpp b/tests/auto/quick/publicapi/tst_publicapi.cpp
index d8c1bd80c..0e48e280d 100644
--- a/tests/auto/quick/publicapi/tst_publicapi.cpp
+++ b/tests/auto/quick/publicapi/tst_publicapi.cpp
@@ -96,11 +96,10 @@ static const QStringList hardcodedTypes = QStringList()
 
 static const QStringList expectedAPI = QStringList()
     << "QQuickWebEngineAction.text --> QString"
-    << "QQuickWebEngineAction.iconText --> QString"
+    << "QQuickWebEngineAction.iconName --> QString"
     << "QQuickWebEngineAction.enabled --> bool"
-    << "QQuickWebEngineAction.toggled() --> void"
     << "QQuickWebEngineAction.triggered() --> void"
-    << "QQuickWebEngineAction.enabledChanged(bool) --> void"
+    << "QQuickWebEngineAction.enabledChanged() --> void"
     << "QQuickWebEngineAction.trigger() --> void"
     << "QQuickWebEngineAuthenticationDialogRequest.AuthenticationTypeHTTP --> AuthenticationType"
     << "QQuickWebEngineAuthenticationDialogRequest.AuthenticationTypeProxy --> AuthenticationType"
diff --git a/tests/auto/quick/qmltests/data/tst_action.qml b/tests/auto/quick/qmltests/data/tst_action.qml
index b27e7d821..f6d8669fe 100644
--- a/tests/auto/quick/qmltests/data/tst_action.qml
+++ b/tests/auto/quick/qmltests/data/tst_action.qml
@@ -47,50 +47,50 @@ TestWebEngineView {
 
         function test_actions_data() {
             return [
-                   { webAction: WebEngineView.Back, text: "Back", iconText: "go-previous", enabled: false },
-                   { webAction: WebEngineView.Forward, text: "Forward", iconText: "go-next", enabled: false },
-                   { webAction: WebEngineView.Stop, text: "Stop", iconText: "", enabled: false },
-                   { webAction: WebEngineView.Reload, text: "Reload", iconText: "view-refresh", enabled: true },
-                   { webAction: WebEngineView.Cut, text: "Cut", iconText: "Cut", enabled: true },
-                   { webAction: WebEngineView.Copy, text: "Copy", iconText: "", enabled: true },
-                   { webAction: WebEngineView.Paste, text: "Paste", iconText: "", enabled: true },
-                   { webAction: WebEngineView.Undo, text: "Undo", iconText: "", enabled: true },
-                   { webAction: WebEngineView.Redo, text: "Redo", iconText: "", enabled: true },
-                   { webAction: WebEngineView.SelectAll, text: "Select all", iconText: "", enabled: true },
-                   { webAction: WebEngineView.ReloadAndBypassCache, text: "Reload and Bypass Cache", iconText: "", enabled: true },
-                   { webAction: WebEngineView.PasteAndMatchStyle, text: "Paste and match style", iconText: "", enabled: true },
-                   { webAction: WebEngineView.OpenLinkInThisWindow, text: "Open link in this window", iconText: "", enabled: true },
-                   { webAction: WebEngineView.OpenLinkInNewWindow, text: "Open link in new window", iconText: "", enabled: true },
-                   { webAction: WebEngineView.OpenLinkInNewTab, text: "Open link in new tab", iconText: "", enabled: true },
-                   { webAction: WebEngineView.CopyLinkToClipboard, text: "Copy link address", iconText: "", enabled: true },
-                   { webAction: WebEngineView.DownloadLinkToDisk, text: "Save link", iconText: "", enabled: true },
-                   { webAction: WebEngineView.CopyImageToClipboard, text: "Copy image", iconText: "", enabled: true },
-                   { webAction: WebEngineView.CopyImageUrlToClipboard, text: "Copy image address", iconText: "", enabled: true },
-                   { webAction: WebEngineView.DownloadImageToDisk, text: "Save image", iconText: "", enabled: true },
-                   { webAction: WebEngineView.CopyMediaUrlToClipboard, text: "Copy media address", iconText: "", enabled: true },
-                   { webAction: WebEngineView.ToggleMediaControls, text: "Show controls", iconText: "", enabled: true },
-                   { webAction: WebEngineView.ToggleMediaLoop, text: "Loop", iconText: "", enabled: true },
-                   { webAction: WebEngineView.ToggleMediaPlayPause, text: "Toggle Play/Pause", iconText: "", enabled: true },
-                   { webAction: WebEngineView.ToggleMediaMute, text: "Toggle Mute", iconText: "", enabled: true },
-                   { webAction: WebEngineView.DownloadMediaToDisk, text: "Save media", iconText: "", enabled: true },
-                   { webAction: WebEngineView.InspectElement, text: "Inspect", iconText: "", enabled: true },
-                   { webAction: WebEngineView.ExitFullScreen, text: "Exit full screen", iconText: "", enabled: true },
-                   { webAction: WebEngineView.RequestClose, text: "Close Page", iconText: "", enabled: true },
-                   { webAction: WebEngineView.Unselect, text: "Unselect", iconText: "", enabled: true },
-                   { webAction: WebEngineView.SavePage, text: "Save page", iconText: "", enabled: true },
-                   { webAction: WebEngineView.ViewSource, text: "View page source", iconText: "view-source", enabled: true },
-                   { webAction: WebEngineView.ToggleBold, text: "&Bold", iconText: "", enabled: true },
-                   { webAction: WebEngineView.ToggleItalic, text: "&Italic", iconText: "", enabled: true },
-                   { webAction: WebEngineView.ToggleUnderline, text: "&Underline", iconText: "", enabled: true },
-                   { webAction: WebEngineView.ToggleStrikethrough, text: "&Strikethrough", iconText: "", enabled: true },
-                   { webAction: WebEngineView.AlignLeft, text: "Align &Left", iconText: "", enabled: true },
-                   { webAction: WebEngineView.AlignCenter, text: "Align &Center", iconText: "", enabled: true },
-                   { webAction: WebEngineView.AlignRight, text: "Align &Right", iconText: "", enabled: true },
-                   { webAction: WebEngineView.AlignJustified, text: "Align &Justified", iconText: "", enabled: true },
-                   { webAction: WebEngineView.Indent, text: "&Indent", iconText: "", enabled: true },
-                   { webAction: WebEngineView.Outdent, text: "&Outdent", iconText: "", enabled: true },
-                   { webAction: WebEngineView.InsertOrderedList, text: "Insert &Ordered List", iconText: "", enabled: true },
-                   { webAction: WebEngineView.InsertUnorderedList, text: "Insert &Unordered List", iconText: "", enabled: true }
+                   { webAction: WebEngineView.Back, text: "Back", iconName: "go-previous", enabled: false },
+                   { webAction: WebEngineView.Forward, text: "Forward", iconName: "go-next", enabled: false },
+                   { webAction: WebEngineView.Stop, text: "Stop", iconName: "", enabled: false },
+                   { webAction: WebEngineView.Reload, text: "Reload", iconName: "view-refresh", enabled: true },
+                   { webAction: WebEngineView.Cut, text: "Cut", iconName: "Cut", enabled: true },
+                   { webAction: WebEngineView.Copy, text: "Copy", iconName: "", enabled: true },
+                   { webAction: WebEngineView.Paste, text: "Paste", iconName: "", enabled: true },
+                   { webAction: WebEngineView.Undo, text: "Undo", iconName: "", enabled: true },
+                   { webAction: WebEngineView.Redo, text: "Redo", iconName: "", enabled: true },
+                   { webAction: WebEngineView.SelectAll, text: "Select all", iconName: "", enabled: true },
+                   { webAction: WebEngineView.ReloadAndBypassCache, text: "Reload and Bypass Cache", iconName: "", enabled: true },
+                   { webAction: WebEngineView.PasteAndMatchStyle, text: "Paste and match style", iconName: "", enabled: true },
+                   { webAction: WebEngineView.OpenLinkInThisWindow, text: "Open link in this window", iconName: "", enabled: true },
+                   { webAction: WebEngineView.OpenLinkInNewWindow, text: "Open link in new window", iconName: "", enabled: true },
+                   { webAction: WebEngineView.OpenLinkInNewTab, text: "Open link in new tab", iconName: "", enabled: true },
+                   { webAction: WebEngineView.CopyLinkToClipboard, text: "Copy link address", iconName: "", enabled: true },
+                   { webAction: WebEngineView.DownloadLinkToDisk, text: "Save link", iconName: "", enabled: true },
+                   { webAction: WebEngineView.CopyImageToClipboard, text: "Copy image", iconName: "", enabled: true },
+                   { webAction: WebEngineView.CopyImageUrlToClipboard, text: "Copy image address", iconName: "", enabled: true },
+                   { webAction: WebEngineView.DownloadImageToDisk, text: "Save image", iconName: "", enabled: true },
+                   { webAction: WebEngineView.CopyMediaUrlToClipboard, text: "Copy media address", iconName: "", enabled: true },
+                   { webAction: WebEngineView.ToggleMediaControls, text: "Show controls", iconName: "", enabled: true },
+                   { webAction: WebEngineView.ToggleMediaLoop, text: "Loop", iconName: "", enabled: true },
+                   { webAction: WebEngineView.ToggleMediaPlayPause, text: "Toggle Play/Pause", iconName: "", enabled: true },
+                   { webAction: WebEngineView.ToggleMediaMute, text: "Toggle Mute", iconName: "", enabled: true },
+                   { webAction: WebEngineView.DownloadMediaToDisk, text: "Save media", iconName: "", enabled: true },
+                   { webAction: WebEngineView.InspectElement, text: "Inspect", iconName: "", enabled: true },
+                   { webAction: WebEngineView.ExitFullScreen, text: "Exit full screen", iconName: "", enabled: true },
+                   { webAction: WebEngineView.RequestClose, text: "Close Page", iconName: "", enabled: true },
+                   { webAction: WebEngineView.Unselect, text: "Unselect", iconName: "", enabled: true },
+                   { webAction: WebEngineView.SavePage, text: "Save page", iconName: "", enabled: true },
+                   { webAction: WebEngineView.ViewSource, text: "View page source", iconName: "view-source", enabled: true },
+                   { webAction: WebEngineView.ToggleBold, text: "&Bold", iconName: "", enabled: true },
+                   { webAction: WebEngineView.ToggleItalic, text: "&Italic", iconName: "", enabled: true },
+                   { webAction: WebEngineView.ToggleUnderline, text: "&Underline", iconName: "", enabled: true },
+                   { webAction: WebEngineView.ToggleStrikethrough, text: "&Strikethrough", iconName: "", enabled: true },
+                   { webAction: WebEngineView.AlignLeft, text: "Align &Left", iconName: "", enabled: true },
+                   { webAction: WebEngineView.AlignCenter, text: "Align &Center", iconName: "", enabled: true },
+                   { webAction: WebEngineView.AlignRight, text: "Align &Right", iconName: "", enabled: true },
+                   { webAction: WebEngineView.AlignJustified, text: "Align &Justified", iconName: "", enabled: true },
+                   { webAction: WebEngineView.Indent, text: "&Indent", iconName: "", enabled: true },
+                   { webAction: WebEngineView.Outdent, text: "&Outdent", iconName: "", enabled: true },
+                   { webAction: WebEngineView.InsertOrderedList, text: "Insert &Ordered List", iconName: "", enabled: true },
+                   { webAction: WebEngineView.InsertUnorderedList, text: "Insert &Unordered List", iconName: "", enabled: true }
             ];
         }
 
-- 
GitLab