Cleanup QAction::visible property

Gives it its own changed signal, and simplifies setting from group,
while fixing an inconsistency in propagation.

Change-Id: I22b243210260a8878144fa4b60204df46f847f37
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

5 files changed, 30 insertions, 17 deletions

diff --git a/src/gui/kernel/qaction.cpp b/src/gui/kernel/qaction.cpp
index 30100c3fcd..4b64c77012 100644
--- a/src/gui/kernel/qaction.cpp
+++ b/src/gui/kernel/qaction.cpp
@@ -1050,18 +1050,30 @@ bool QAction::isEnabled() const
 void QAction::setVisible(bool b)
 {
     Q_D(QAction);
-    if (b == d->visible && b != d->forceInvisible)
+    if (b != d->forceInvisible)
         return;
-    QAPP_CHECK("setVisible");
     d->forceInvisible = !b;
-    d->visible = b;
-    bool enabled = d->visible;
-    if (enabled && d->explicitEnabled)
-        enabled = d->explicitEnabledValue;
-    if (!d->setEnabled(enabled, false))
-        d->sendDataChanged();
+    if (b && d->group && !d->group->isVisible())
+        return;
+    d->setVisible(b);
 }
 
+void QActionPrivate::setVisible(bool b)
+{
+    Q_Q(QAction);
+    if (b == visible)
+        return;
+    QAPP_CHECK("setVisible");
+    visible = b;
+    bool enable = visible;
+    if (enable && explicitEnabled)
+        enable = explicitEnabledValue;
+    QPointer guard(q);
+    if (!setEnabled(enable, false))
+        sendDataChanged();
+    if (guard)
+        emit q->visibleChanged();
+}
 
 bool QAction::isVisible() const
 {
diff --git a/src/gui/kernel/qaction.h b/src/gui/kernel/qaction.h
index 8466bc41f7..865e5c5eb0 100644
--- a/src/gui/kernel/qaction.h
+++ b/src/gui/kernel/qaction.h
@@ -81,7 +81,7 @@ class Q_GUI_EXPORT QAction : public QObject
     Q_PROPERTY(Qt::ShortcutContext shortcutContext READ shortcutContext WRITE setShortcutContext NOTIFY changed)
     Q_PROPERTY(bool autoRepeat READ autoRepeat WRITE setAutoRepeat NOTIFY changed)
 #endif // QT_CONFIG(shortcut)
-    Q_PROPERTY(bool visible READ isVisible WRITE setVisible NOTIFY changed)
+    Q_PROPERTY(bool visible READ isVisible WRITE setVisible NOTIFY visibleChanged FINAL)
     Q_PROPERTY(MenuRole menuRole READ menuRole WRITE setMenuRole NOTIFY changed)
     Q_PROPERTY(bool iconVisibleInMenu READ isIconVisibleInMenu WRITE setIconVisibleInMenu NOTIFY changed)
     Q_PROPERTY(bool shortcutVisibleInContextMenu READ isShortcutVisibleInContextMenu WRITE setShortcutVisibleInContextMenu NOTIFY changed)
@@ -250,6 +250,7 @@ Q_SIGNALS:
     void changed();
     void enabledChanged(bool enabled);
     void checkableChanged(bool checkable);
+    void visibleChanged();
     void triggered(bool checked = false);
     void hovered();
     void toggled(bool);
diff --git a/src/gui/kernel/qaction_p.h b/src/gui/kernel/qaction_p.h
index 3e137a09d6..28418c054e 100644
--- a/src/gui/kernel/qaction_p.h
+++ b/src/gui/kernel/qaction_p.h
@@ -84,6 +84,7 @@ public:
     }
 
     bool setEnabled(bool enable, bool byGroup);
+    void setVisible(bool b);
 
     QPointer<QActionGroup> group;
     QString text;
diff --git a/src/gui/kernel/qactiongroup.cpp b/src/gui/kernel/qactiongroup.cpp
index 6e2efdb74b..33f9b54ddc 100644
--- a/src/gui/kernel/qactiongroup.cpp
+++ b/src/gui/kernel/qactiongroup.cpp
@@ -169,10 +169,8 @@ QAction *QActionGroup::addAction(QAction* a)
         QObject::connect(a, &QAction::hovered, this, &QActionGroup::_q_actionHovered);
     }
     a->d_func()->setEnabled(d->enabled, true);
-    if (!a->d_func()->forceInvisible) {
-        a->setVisible(d->visible);
-        a->d_func()->forceInvisible = false;
-    }
+    if (!a->d_func()->forceInvisible)
+        a->d_func()->setVisible(d->visible);
     if (a->isChecked())
         d->current = a;
     QActionGroup *oldGroup = a->d_func()->group;
@@ -350,10 +348,8 @@ void QActionGroup::setVisible(bool b)
     Q_D(QActionGroup);
     d->visible = b;
     for (auto action : qAsConst(d->actions)) {
-        if (!action->d_func()->forceInvisible) {
-            action->setVisible(b);
-            action->d_func()->forceInvisible = false;
-        }
+        if (!action->d_func()->forceInvisible)
+            action->d_func()->setVisible(b);
     }
 }
 
diff --git a/tests/auto/gui/kernel/qactiongroup/tst_qactiongroup.cpp b/tests/auto/gui/kernel/qactiongroup/tst_qactiongroup.cpp
index c5a3db7b62..52720ab7ff 100644
--- a/tests/auto/gui/kernel/qactiongroup/tst_qactiongroup.cpp
+++ b/tests/auto/gui/kernel/qactiongroup/tst_qactiongroup.cpp
@@ -100,6 +100,9 @@ void tst_QActionGroup::visiblePropagation()
     QVERIFY( !childAction->isVisible() );
     QVERIFY( !anotherChildAction->isVisible() );
 
+    childAction->setVisible(true);
+    QVERIFY( !childAction->isVisible() );
+
     anotherChildAction->setVisible(false);
 
     testActionGroup.setVisible( true );