Dialog: emit accepted()/rejected() before closed()

Before this patch, the documentation stated the following for e.g.
accept():

"Closes the dialog and emits the accepted() signal."

In practice, styles that have enter and/or exit transitions for Dialog
result in closed() being emitted after accepted() (and rejected()).

As there is no way to guarantee this particular order, we should be
consistent and swap it around so that accepted()/rejected() are emitted
first.

[ChangeLog][Important Behavior Changes] Dialog's accepted() and
rejected() signals are now emitted before closed() when calling done(),
accept() and reject().

Fixes: QTBUG-85748
Change-Id: I706dda8de28c350d65e3188787af8f66a2c5f07d
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

2 files changed, 36 insertions, 8 deletions

diff --git a/src/quicktemplates2/qquickdialog.cpp b/src/quicktemplates2/qquickdialog.cpp
index a13b42ee..1fe3545d 100644
--- a/src/quicktemplates2/qquickdialog.cpp
+++ b/src/quicktemplates2/qquickdialog.cpp
@@ -481,7 +481,7 @@ qreal QQuickDialog::implicitFooterHeight() const
 /*!
     \qmlmethod void QtQuick.Controls::Dialog::accept()
 
-    Closes the dialog and emits the \l accepted() signal.
+    Emits the \l accepted() signal and closes the dialog.
 
     \sa reject(), done()
 */
@@ -493,7 +493,7 @@ void QQuickDialog::accept()
 /*!
     \qmlmethod void QtQuick.Controls::Dialog::reject()
 
-    Closes the dialog and emits the \l rejected() signal.
+    Emits the \l rejected() signal and closes the dialog.
 
     \sa accept(), done()
 */
@@ -506,21 +506,26 @@ void QQuickDialog::reject()
     \since QtQuick.Controls 2.3 (Qt 5.10)
     \qmlmethod void QtQuick.Controls::Dialog::done(int result)
 
-    Closes the dialog, sets the \a result, and emits \l accepted() or
-    \l rejected() depending on whether the result is \c Dialog.Accepted
-    or \c Dialog.Rejected, respectively.
+    \list 1
+    \li Sets the \a result.
+    \li Emits \l accepted() or \l rejected() depending on
+    whether the result is \c Dialog.Accepted or \c Dialog.Rejected,
+    respectively.
+    \li Emits \l closed().
+    \endlist
 
     \sa accept(), reject(), result
 */
 void QQuickDialog::done(int result)
 {
-    close();
     setResult(result);
 
     if (result == Accepted)
         emit accepted();
     else if (result == Rejected)
         emit rejected();
+
+    close();
 }
 
 #if QT_CONFIG(accessibility)
diff --git a/tests/auto/controls/data/tst_dialog.qml b/tests/auto/controls/data/tst_dialog.qml
index 53d27fd3..f8bf5bb5 100644
--- a/tests/auto/controls/data/tst_dialog.qml
+++ b/tests/auto/controls/data/tst_dialog.qml
@@ -49,6 +49,7 @@
 ****************************************************************************/
 
 import QtQuick
+import QtQuick.Window
 import QtTest
 import QtQuick.Controls
 import QtQuick.Templates as T
@@ -84,6 +85,10 @@ TestCase {
         SignalSpy { }
     }
 
+    function init() {
+        tryCompare(testCase.Window.window, "active", true)
+    }
+
     function test_defaults() {
         var control = createTemporaryObject(dialog, testCase)
         verify(control)
@@ -105,16 +110,20 @@ TestCase {
 
         var acceptedSpy = createTemporaryObject(signalSpy, testCase, {target: control, signalName: "accepted"})
         verify(acceptedSpy.valid)
+
+        var closedSpy = createTemporaryObject(signalSpy, testCase, {target: control, signalName: "closed"})
+        verify(closedSpy.valid)
+
         control.accept()
         compare(acceptedSpy.count, 1)
         compare(control.result, Dialog.Accepted)
 
         tryCompare(control, "visible", false)
+        compare(acceptedSpy.count, 1)
+        compare(closedSpy.count, 1)
     }
 
     function test_reject() {
-        skip("QTBUG-62549, QTBUG-62628")
-
         var control = createTemporaryObject(dialog, testCase)
 
         var openedSpy = createTemporaryObject(signalSpy, testCase, {target: control, signalName: "opened"})
@@ -127,11 +136,17 @@ TestCase {
 
         var rejectedSpy = createTemporaryObject(signalSpy, testCase, {target: control, signalName: "rejected"})
         verify(rejectedSpy.valid)
+
+        var closedSpy = createTemporaryObject(signalSpy, testCase, {target: control, signalName: "closed"})
+        verify(closedSpy.valid)
+
         control.reject()
         compare(rejectedSpy.count, 1)
         compare(control.result, Dialog.Rejected)
 
         tryCompare(control, "visible", false)
+        compare(rejectedSpy.count, 1)
+        compare(closedSpy.count, 1)
 
         // Check that rejected() is emitted when CloseOnEscape is triggered.
         control.x = 10
@@ -145,9 +160,12 @@ TestCase {
         keyPress(Qt.Key_Escape)
         compare(rejectedSpy.count, 2)
         tryCompare(control, "visible", false)
+        compare(rejectedSpy.count, 2)
+        compare(closedSpy.count, 2)
 
         keyRelease(Qt.Key_Escape)
         compare(rejectedSpy.count, 2)
+        compare(closedSpy.count, 2)
 
         // Check that rejected() is emitted when CloseOnPressOutside is triggered.
         control.closePolicy = Popup.CloseOnPressOutside
@@ -157,9 +175,12 @@ TestCase {
         mousePress(testCase, 1, 1)
         compare(rejectedSpy.count, 3)
         tryCompare(control, "visible", false)
+        compare(rejectedSpy.count, 3)
+        compare(closedSpy.count, 3)
 
         mouseRelease(testCase, 1, 1)
         compare(rejectedSpy.count, 3)
+        compare(closedSpy.count, 3)
 
         // Check that rejected() is emitted when CloseOnReleaseOutside is triggered.
         // For this, we need to make the dialog modal, because the overlay won't accept
@@ -176,6 +197,8 @@ TestCase {
         mouseRelease(testCase, 1, 1)
         compare(rejectedSpy.count, 4)
         tryCompare(control, "visible", false)
+        compare(rejectedSpy.count, 4)
+        compare(closedSpy.count, 4)
     }
 
     function test_buttonBox_data() {