DialogButtonBox: don't sort buttons based on their memory addresses

When two buttons' roles are equal, the code would fall back to
comparing the buttons' memory addresses. This leads to random results,
which are especially noticeable on Windows and with release builds.

This patch fixes the issue by instead returning false if the roles
are equal. This still satisfies the "comp(a,a)==false" requirement
of strict weak ordering:

https://en.cppreference.com/w/cpp/named_req/Compare

The patch also changes the sorting algorithm used from std::sort()
to std::stable_sort(). Although it doesn't appear to be necessary from
the testing that I did, it is good to ensure that the order of equal
elements is maintained.

Fixes: QTBUG-70451
Change-Id: I47561604108b12bf8ec0c794a2372767f0b2e04e
Reviewed-by: Kai Koehne <kai.koehne@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>

2 files changed, 32 insertions, 2 deletions

diff --git a/src/quicktemplates2/qquickdialogbuttonbox.cpp b/src/quicktemplates2/qquickdialogbuttonbox.cpp
index 03f5f8e8..91fb41f2 100644
--- a/src/quicktemplates2/qquickdialogbuttonbox.cpp
+++ b/src/quicktemplates2/qquickdialogbuttonbox.cpp
@@ -295,6 +295,7 @@ void QQuickDialogButtonBoxPrivate::updateLayout()
             if (firstRole != secondRole && firstRole != QPlatformDialogHelper::InvalidRole && secondRole != QPlatformDialogHelper::InvalidRole) {
                 const int *l = m_layout;
                 while (*l != QPlatformDialogHelper::EOL) {
+                    // Unset the Reverse flag.
                     const int role = (*l & ~QPlatformDialogHelper::Reverse);
                     if (role == firstRole)
                         return true;
@@ -305,14 +306,14 @@ void QQuickDialogButtonBoxPrivate::updateLayout()
             }
 
             if (firstRole == secondRole)
-                return first < second;
+                return false;
 
             return firstRole != QPlatformDialogHelper::InvalidRole;
         }
         const int *m_layout;
     };
 
-    std::sort(buttons.begin(), buttons.end(), ButtonLayout(static_cast<QPlatformDialogHelper::ButtonLayout>(buttonLayout)));
+    std::stable_sort(buttons.begin(), buttons.end(), ButtonLayout(static_cast<QPlatformDialogHelper::ButtonLayout>(buttonLayout)));
 
     for (int i = 0; i < buttons.count() - 1; ++i)
         q->insertItem(i, buttons.at(i));
diff --git a/tests/auto/controls/data/tst_dialogbuttonbox.qml b/tests/auto/controls/data/tst_dialogbuttonbox.qml
index 6eca8569..374b32ff 100644
--- a/tests/auto/controls/data/tst_dialogbuttonbox.qml
+++ b/tests/auto/controls/data/tst_dialogbuttonbox.qml
@@ -424,4 +424,33 @@ TestCase {
             1000, "Expected right edge of button to be within right edge of DialogButtonBox (i.e. less than or equal to " +
                 control.width + "), but it's " + (button.mapToItem(control, 0, 0).x + button.width))
     }
+
+
+    Component {
+        id: noRolesDialog
+
+        Dialog {
+            footer: DialogButtonBox {
+                Button { text: "A" }
+                Button { text: "B" }
+                Button { text: "C" }
+            }
+        }
+    }
+
+    function test_orderWithNoRoles() {
+        for (var i = 0; i < 10; ++i) {
+            var control = createTemporaryObject(noRolesDialog, testCase)
+            verify(control)
+
+            control.open()
+            tryCompare(control, "opened", true)
+            var footer = control.footer
+            verify(footer)
+            waitForRendering(footer)
+            compare(footer.itemAt(0).text, "A")
+            compare(footer.itemAt(1).text, "B")
+            compare(footer.itemAt(2).text, "C")
+        }
+    }
 }