Clarify MsgBox buttons handling code

Change-Id: I0095b4b5312c851f1f64ec013c64f26e1b24ce09
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171454
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Tested-by: Jenkins

3 files changed, 33 insertions, 54 deletions

diff --git a/basic/source/inc/runtime.hxx b/basic/source/inc/runtime.hxx
index 0612ea0d1acb..a15abe02062d 100644
--- a/basic/source/inc/runtime.hxx
+++ b/basic/source/inc/runtime.hxx
@@ -119,10 +119,9 @@ constexpr sal_Int16 DEFBUTTON2 = 256;
 constexpr sal_Int16 DEFBUTTON3 = 512;
 constexpr sal_Int16 APPLMODAL = 0;
 constexpr sal_Int16 SYSTEMMODAL = 4096;
-}
 
 // Related to: MsgBox (return value)
-namespace SbMBID
+namespace Response
 {
 constexpr sal_Int16 OK = 1;
 constexpr sal_Int16 CANCEL = 2;
@@ -132,6 +131,7 @@ constexpr sal_Int16 IGNORE = 5;
 constexpr sal_Int16 YES = 6;
 constexpr sal_Int16 NO = 7;
 }
+}
 
 // Related to: SwFieldTypesEnum in sw/inc/fldbas.hxx
 namespace SbTYP
diff --git a/basic/source/runtime/methods.cxx b/basic/source/runtime/methods.cxx
index 05939417c688..d0cf61f9fa42 100644
--- a/basic/source/runtime/methods.cxx
+++ b/basic/source/runtime/methods.cxx
@@ -4257,68 +4257,47 @@ void SbRtl_MsgBox(StarBASIC *, SbxArray & rPar, bool)
     std::unique_ptr<weld::MessageDialog> xBox(Application::CreateMessageDialog(pParent,
                 eType, VclButtonsType::NONE, aMsg, GetpApp()));
 
+    std::vector<std::pair<StandardButtonType, sal_Int16>> buttons;
     switch (nType & 0x0F) // delete bits 4-16
     {
         case SbMB::OK:
         default:
-            xBox->add_button(GetStandardText(StandardButtonType::OK), SbMBID::OK);
+            buttons.emplace_back(StandardButtonType::OK, SbMB::Response::OK);
             break;
         case SbMB::OKCANCEL:
-            xBox->add_button(GetStandardText(StandardButtonType::OK), SbMBID::OK);
-            xBox->add_button(GetStandardText(StandardButtonType::Cancel), SbMBID::CANCEL);
-
-            if (nType & SbMB::DEFBUTTON2 || nType & SbMB::DEFBUTTON3)
-                xBox->set_default_response(SbMBID::CANCEL);
-            else
-                xBox->set_default_response(SbMBID::OK);
-
+            buttons.emplace_back(StandardButtonType::OK, SbMB::Response::OK);
+            buttons.emplace_back(StandardButtonType::Cancel, SbMB::Response::CANCEL);
             break;
         case SbMB::ABORTRETRYIGNORE:
-            xBox->add_button(GetStandardText(StandardButtonType::Abort), SbMBID::ABORT);
-            xBox->add_button(GetStandardText(StandardButtonType::Retry), SbMBID::RETRY);
-            xBox->add_button(GetStandardText(StandardButtonType::Ignore), SbMBID::IGNORE);
-
-            if (nType & SbMB::DEFBUTTON2)
-                xBox->set_default_response(SbMBID::RETRY);
-            else if (nType & SbMB::DEFBUTTON3)
-                xBox->set_default_response(SbMBID::IGNORE);
-            else
-                xBox->set_default_response(SbMBID::CANCEL);
-
+            buttons.emplace_back(StandardButtonType::Abort, SbMB::Response::ABORT);
+            buttons.emplace_back(StandardButtonType::Retry, SbMB::Response::RETRY);
+            buttons.emplace_back(StandardButtonType::Ignore, SbMB::Response::IGNORE);
             break;
         case SbMB::YESNOCANCEL:
-            xBox->add_button(GetStandardText(StandardButtonType::Yes), SbMBID::YES);
-            xBox->add_button(GetStandardText(StandardButtonType::No), SbMBID::NO);
-            xBox->add_button(GetStandardText(StandardButtonType::Cancel), SbMBID::CANCEL);
-
-            if (nType & SbMB::DEFBUTTON2 || nType & SbMB::DEFBUTTON3)
-                xBox->set_default_response(SbMBID::CANCEL);
-            else
-                xBox->set_default_response(SbMBID::YES);
-
+            buttons.emplace_back(StandardButtonType::Yes, SbMB::Response::YES);
+            buttons.emplace_back(StandardButtonType::No, SbMB::Response::NO);
+            buttons.emplace_back(StandardButtonType::Cancel, SbMB::Response::CANCEL);
             break;
         case SbMB::YESNO:
-            xBox->add_button(GetStandardText(StandardButtonType::Yes), SbMBID::YES);
-            xBox->add_button(GetStandardText(StandardButtonType::No), SbMBID::NO);
-
-            if (nType & SbMB::DEFBUTTON2 || nType & SbMB::DEFBUTTON3)
-                xBox->set_default_response(SbMBID::NO);
-            else
-                xBox->set_default_response(SbMBID::YES);
-
+            buttons.emplace_back(StandardButtonType::Yes, SbMB::Response::YES);
+            buttons.emplace_back(StandardButtonType::No, SbMB::Response::NO);
             break;
         case SbMB::RETRYCANCEL:
-            xBox->add_button(GetStandardText(StandardButtonType::Retry), SbMBID::RETRY);
-            xBox->add_button(GetStandardText(StandardButtonType::Cancel), SbMBID::CANCEL);
-
-            if (nType & SbMB::DEFBUTTON2 || nType & SbMB::DEFBUTTON3)
-                xBox->set_default_response(SbMBID::CANCEL);
-            else
-                xBox->set_default_response(SbMBID::RETRY);
-
+            buttons.emplace_back(StandardButtonType::Retry, SbMB::Response::RETRY);
+            buttons.emplace_back(StandardButtonType::Cancel, SbMB::Response::CANCEL);
             break;
     }
 
+    for (auto [buttonType, buttonResponse] : buttons)
+        xBox->add_button(GetStandardText(buttonType), buttonResponse);
+
+    std::size_t default_button = 0;
+    if (nType & SbMB::DEFBUTTON2)
+        default_button = 1;
+    else if (nType & SbMB::DEFBUTTON3)
+        default_button = 2;
+    xBox->set_default_response(buttons[std::min(default_button, buttons.size() - 1)].second);
+
     xBox->set_title(aTitle);
     sal_Int16 nRet = xBox->run();
     rPar.Get(0)->PutInteger(nRet);
diff --git a/basic/source/runtime/stdobj.cxx b/basic/source/runtime/stdobj.cxx
index 27f6deddc93f..a8284479d0e4 100644
--- a/basic/source/runtime/stdobj.cxx
+++ b/basic/source/runtime/stdobj.cxx
@@ -463,13 +463,13 @@ constexpr Method aMethods[] = {
     arg(u"Date", SbxDATE),
 
 // Related to: MsgBox (return value)
-{ u"IDABORT",                       SbxINTEGER,      CPROP_,       ConstInt<SbMBID::ABORT>        },
-{ u"IDCANCEL",                      SbxINTEGER,      CPROP_,       ConstInt<SbMBID::CANCEL>       },
-{ u"IDIGNORE",                      SbxINTEGER,      CPROP_,       ConstInt<SbMBID::IGNORE>       },
-{ u"IDNO",                          SbxINTEGER,      CPROP_,       ConstInt<SbMBID::NO>           },
-{ u"IDOK",                          SbxINTEGER,      CPROP_,       ConstInt<SbMBID::OK>           },
-{ u"IDRETRY",                       SbxINTEGER,      CPROP_,       ConstInt<SbMBID::RETRY>        },
-{ u"IDYES",                         SbxINTEGER,      CPROP_,       ConstInt<SbMBID::YES>          },
+{ u"IDABORT",                       SbxINTEGER,      CPROP_,     ConstInt<SbMB::Response::ABORT>  },
+{ u"IDCANCEL",                      SbxINTEGER,      CPROP_,     ConstInt<SbMB::Response::CANCEL> },
+{ u"IDIGNORE",                      SbxINTEGER,      CPROP_,     ConstInt<SbMB::Response::IGNORE> },
+{ u"IDNO",                          SbxINTEGER,      CPROP_,     ConstInt<SbMB::Response::NO>     },
+{ u"IDOK",                          SbxINTEGER,      CPROP_,     ConstInt<SbMB::Response::OK>     },
+{ u"IDRETRY",                       SbxINTEGER,      CPROP_,     ConstInt<SbMB::Response::RETRY>  },
+{ u"IDYES",                         SbxINTEGER,      CPROP_,     ConstInt<SbMB::Response::YES>    },
 
 { u"Iif",                           SbxVARIANT,   3 | FUNCTION_,       SbRtl_Iif                  },
     arg(u"Bool",     SbxBOOL),