tdf#145296 better fix to launch dialog when active radiobutton is clicked

this basically reverts

commit 312a32f6b6c83c5747b617249d794fc87ffd8b9b
tdf#145296 use a 'clickable' widget for custom spacing
commit 592235d64d6f9638a61865712ca385ec58d688c4
tdf#145296 use a 'clickable' widget for custom depth

and uses a mouse-release workaround to detect the case of clicking on an
already active radiobutton which gives a result basically
indistinguishable from the pre 7.2 experience

Change-Id: Ic7ac6fa20843466b1d0c77586e25f4aabf814328
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/124285
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolanm@redhat.com>

3 files changed, 147 insertions, 89 deletions

diff --git a/svx/source/tbxctrls/extrusioncontrols.cxx b/svx/source/tbxctrls/extrusioncontrols.cxx
index 5f6544404a67..face95949f88 100644
--- a/svx/source/tbxctrls/extrusioncontrols.cxx
+++ b/svx/source/tbxctrls/extrusioncontrols.cxx
@@ -331,23 +331,26 @@ constexpr OUStringLiteral gsMetricUnit(     u".uno:MetricUnit"     );
 ExtrusionDepthWindow::ExtrusionDepthWindow(svt::PopupWindowController* pControl, weld::Widget* pParent)
     : WeldToolbarPopup(pControl->getFrameInterface(), pParent, "svx/ui/depthwindow.ui", "DepthWindow")
     , mxControl(pControl)
-    , mxDepth0(m_xBuilder->weld_toggle_button("depth0"))
-    , mxDepth1(m_xBuilder->weld_toggle_button("depth1"))
-    , mxDepth2(m_xBuilder->weld_toggle_button("depth2"))
-    , mxDepth3(m_xBuilder->weld_toggle_button("depth3"))
-    , mxDepth4(m_xBuilder->weld_toggle_button("depth4"))
-    , mxInfinity(m_xBuilder->weld_toggle_button("infinity"))
-    , mxCustom(m_xBuilder->weld_toggle_button("custom"))
+    , mxDepth0(m_xBuilder->weld_radio_button("depth0"))
+    , mxDepth1(m_xBuilder->weld_radio_button("depth1"))
+    , mxDepth2(m_xBuilder->weld_radio_button("depth2"))
+    , mxDepth3(m_xBuilder->weld_radio_button("depth3"))
+    , mxDepth4(m_xBuilder->weld_radio_button("depth4"))
+    , mxInfinity(m_xBuilder->weld_radio_button("infinity"))
+    , mxCustom(m_xBuilder->weld_radio_button("custom"))
     , meUnit(FieldUnit::NONE)
     , mfDepth( -1.0 )
-{
-    mxDepth0->connect_clicked(LINK(this, ExtrusionDepthWindow, SelectHdl));
-    mxDepth1->connect_clicked(LINK(this, ExtrusionDepthWindow, SelectHdl));
-    mxDepth2->connect_clicked(LINK(this, ExtrusionDepthWindow, SelectHdl));
-    mxDepth3->connect_clicked(LINK(this, ExtrusionDepthWindow, SelectHdl));
-    mxDepth4->connect_clicked(LINK(this, ExtrusionDepthWindow, SelectHdl));
-    mxInfinity->connect_clicked(LINK(this, ExtrusionDepthWindow, SelectHdl));
-    mxCustom->connect_clicked(LINK(this, ExtrusionDepthWindow, SelectHdl));
+    , mbSettingValue(false)
+    , mbCommandDispatched(false)
+{
+    mxDepth0->connect_toggled(LINK(this, ExtrusionDepthWindow, SelectHdl));
+    mxDepth1->connect_toggled(LINK(this, ExtrusionDepthWindow, SelectHdl));
+    mxDepth2->connect_toggled(LINK(this, ExtrusionDepthWindow, SelectHdl));
+    mxDepth3->connect_toggled(LINK(this, ExtrusionDepthWindow, SelectHdl));
+    mxDepth4->connect_toggled(LINK(this, ExtrusionDepthWindow, SelectHdl));
+    mxInfinity->connect_toggled(LINK(this, ExtrusionDepthWindow, SelectHdl));
+    mxCustom->connect_toggled(LINK(this, ExtrusionDepthWindow, SelectHdl));
+    mxCustom->connect_mouse_release(LINK(this, ExtrusionDepthWindow, MouseReleaseHdl));
 
     AddStatusListener( gsExtrusionDepth );
     AddStatusListener( gsMetricUnit );
@@ -362,6 +365,10 @@ void ExtrusionDepthWindow::implSetDepth( double fDepth )
 {
     mfDepth = fDepth;
 
+    bool bSettingValue = mbSettingValue;
+    mbSettingValue = true;
+
+    mxCustom->set_active(true);
     bool bIsMetric = IsMetric(meUnit);
     mxDepth0->set_active(fDepth == (bIsMetric ? aDepthListMM[0] : aDepthListInch[0]));
     mxDepth1->set_active(fDepth == (bIsMetric ? aDepthListMM[1] : aDepthListInch[1]));
@@ -369,12 +376,8 @@ void ExtrusionDepthWindow::implSetDepth( double fDepth )
     mxDepth3->set_active(fDepth == (bIsMetric ? aDepthListMM[3] : aDepthListInch[3]));
     mxDepth4->set_active(fDepth == (bIsMetric ? aDepthListMM[4] : aDepthListInch[4]));
     mxInfinity->set_active(fDepth >= 338666);
-    mxCustom->set_active(!mxDepth0->get_active() &&
-                         !mxDepth1->get_active() &&
-                         !mxDepth2->get_active() &&
-                         !mxDepth3->get_active() &&
-                         !mxDepth4->get_active() &&
-                         !mxInfinity->get_active());
+
+    mbSettingValue = bSettingValue;
 }
 
 void ExtrusionDepthWindow::implFillStrings( FieldUnit eUnit )
@@ -442,28 +445,33 @@ void ExtrusionDepthWindow::statusChanged(
     }
 }
 
-IMPL_LINK(ExtrusionDepthWindow, SelectHdl, weld::Button&, rButton, void)
+void ExtrusionDepthWindow::DispatchDepthDialog()
 {
-    mxDepth0->set_active(&rButton == mxDepth0.get());
-    mxDepth1->set_active(&rButton == mxDepth1.get());
-    mxDepth2->set_active(&rButton == mxDepth2.get());
-    mxDepth3->set_active(&rButton == mxDepth3.get());
-    mxDepth4->set_active(&rButton == mxDepth4.get());
-    mxInfinity->set_active(&rButton == mxInfinity.get());
-    mxCustom->set_active(&rButton == mxCustom.get());
+    Sequence< PropertyValue > aArgs( 2 );
+    aArgs[0].Name = "Depth";
+    aArgs[0].Value <<= mfDepth;
+    aArgs[1].Name = "Metric";
+    aArgs[1].Value <<= static_cast<sal_Int32>( meUnit );
+
+    rtl::Reference<svt::PopupWindowController> xControl(mxControl);
+    xControl->EndPopupMode();
+    xControl->dispatchCommand(".uno:ExtrusionDepthDialog", aArgs);
+    mbCommandDispatched = true;
+}
+
+IMPL_LINK(ExtrusionDepthWindow, SelectHdl, weld::Toggleable&, rButton, void)
+{
+    if (mbSettingValue || !rButton.get_active())
+        return;
+
+    // see MouseReleaseHdl for mbCommandDispatched check, there's no guarantee
+    // this toggle will happen before that mouse release though it does in
+    // practice for vcl and gtk
+    if (mbCommandDispatched)
+        return;
 
     if (mxCustom->get_active())
-    {
-        Sequence< PropertyValue > aArgs( 2 );
-        aArgs[0].Name = "Depth";
-        aArgs[0].Value <<= mfDepth;
-        aArgs[1].Name = "Metric";
-        aArgs[1].Value <<= static_cast<sal_Int32>( meUnit );
-
-        rtl::Reference<svt::PopupWindowController> xControl(mxControl);
-        xControl->EndPopupMode();
-        xControl->dispatchCommand(".uno:ExtrusionDepthDialog", aArgs);
-    }
+        DispatchDepthDialog();
     else
     {
         double fDepth;
@@ -494,12 +502,31 @@ IMPL_LINK(ExtrusionDepthWindow, SelectHdl, weld::Button&, rButton, void)
         aArgs[0].Value <<= fDepth;
 
         mxControl->dispatchCommand( gsExtrusionDepth,  aArgs );
+        mbCommandDispatched = true;
         implSetDepth( fDepth );
 
         mxControl->EndPopupMode();
     }
 }
 
+IMPL_LINK_NOARG(ExtrusionDepthWindow, MouseReleaseHdl, const MouseEvent&, bool)
+{
+    /*
+     tdf#145296 if the "custom" radiobutton was presented preselected as
+     toggled on and the user clicked on it then there's no toggled signal sent
+     because the item was already toggled on and didn't change state.
+
+     So if that happens launch the custom spacing dialog explicitly here on
+     mouse release.
+    */
+    if (mxCustom->get_active() && !mbCommandDispatched)
+    {
+        DispatchDepthDialog();
+        return true;
+    }
+    return false;
+}
+
 // ExtrusionDirectionControl
 ExtrusionDepthController::ExtrusionDepthController(
     const Reference< XComponentContext >& rxContext
diff --git a/svx/source/tbxctrls/extrusioncontrols.hxx b/svx/source/tbxctrls/extrusioncontrols.hxx
index 531ae48bbfb7..7e0043487274 100644
--- a/svx/source/tbxctrls/extrusioncontrols.hxx
+++ b/svx/source/tbxctrls/extrusioncontrols.hxx
@@ -93,22 +93,27 @@ class ExtrusionDepthWindow final : public WeldToolbarPopup
 {
 private:
     rtl::Reference<svt::PopupWindowController> mxControl;
-    std::unique_ptr<weld::ToggleButton> mxDepth0;
-    std::unique_ptr<weld::ToggleButton> mxDepth1;
-    std::unique_ptr<weld::ToggleButton> mxDepth2;
-    std::unique_ptr<weld::ToggleButton> mxDepth3;
-    std::unique_ptr<weld::ToggleButton> mxDepth4;
-    std::unique_ptr<weld::ToggleButton> mxInfinity;
-    std::unique_ptr<weld::ToggleButton> mxCustom;
+    std::unique_ptr<weld::RadioButton> mxDepth0;
+    std::unique_ptr<weld::RadioButton> mxDepth1;
+    std::unique_ptr<weld::RadioButton> mxDepth2;
+    std::unique_ptr<weld::RadioButton> mxDepth3;
+    std::unique_ptr<weld::RadioButton> mxDepth4;
+    std::unique_ptr<weld::RadioButton> mxInfinity;
+    std::unique_ptr<weld::RadioButton> mxCustom;
 
     FieldUnit   meUnit;
     double      mfDepth;
+    bool        mbSettingValue;
+    bool        mbCommandDispatched;
 
-    DECL_LINK( SelectHdl, weld::Button&, void );
+    DECL_LINK( SelectHdl, weld::Toggleable&, void );
+    DECL_LINK( MouseReleaseHdl, const MouseEvent&, bool );
 
     void    implFillStrings( FieldUnit eUnit );
     void    implSetDepth( double fDepth );
 
+    void    DispatchDepthDialog();
+
 public:
     ExtrusionDepthWindow(svt::PopupWindowController* pControl, weld::Widget* pParentWindow);
     virtual void GrabFocus() override;
diff --git a/svx/source/tbxctrls/fontworkgallery.cxx b/svx/source/tbxctrls/fontworkgallery.cxx
index 754209a06760..7bfec4b860c0 100644
--- a/svx/source/tbxctrls/fontworkgallery.cxx
+++ b/svx/source/tbxctrls/fontworkgallery.cxx
@@ -470,21 +470,24 @@ public:
     virtual void statusChanged( const css::frame::FeatureStateEvent& Event ) override;
 private:
     rtl::Reference<svt::PopupWindowController> mxControl;
-    std::unique_ptr<weld::ToggleButton> mxVeryTight;
-    std::unique_ptr<weld::ToggleButton> mxTight;
-    std::unique_ptr<weld::ToggleButton> mxNormal;
-    std::unique_ptr<weld::ToggleButton> mxLoose;
-    std::unique_ptr<weld::ToggleButton> mxVeryLoose;
-    std::unique_ptr<weld::ToggleButton> mxCustom;
+    std::unique_ptr<weld::RadioButton> mxVeryTight;
+    std::unique_ptr<weld::RadioButton> mxTight;
+    std::unique_ptr<weld::RadioButton> mxNormal;
+    std::unique_ptr<weld::RadioButton> mxLoose;
+    std::unique_ptr<weld::RadioButton> mxVeryLoose;
+    std::unique_ptr<weld::RadioButton> mxCustom;
     std::unique_ptr<weld::CheckButton> mxKernPairs;
     sal_Int32 mnCharacterSpacing;
     bool mbSettingValue;
+    bool mbCommandDispatched;
 
     DECL_LINK( KernSelectHdl, weld::Toggleable&, void );
-    DECL_LINK( SelectHdl, weld::Button&, void );
+    DECL_LINK( SelectHdl, weld::Toggleable&, void );
+    DECL_LINK( MouseReleaseHdl, const MouseEvent&, bool );
 
     void    implSetCharacterSpacing( sal_Int32 nCharacterSpacing, bool bEnabled );
     void    implSetKernCharacterPairs(bool bKernOnOff, bool bEnabled);
+    void    DispatchSpacingDialog();
 };
 
 }
@@ -495,22 +498,24 @@ constexpr OUStringLiteral gsFontworkKernCharacterPairs(u".uno:FontworkKernCharac
 FontworkCharacterSpacingWindow::FontworkCharacterSpacingWindow(svt::PopupWindowController* pControl, weld::Widget* pParent)
     : WeldToolbarPopup(pControl->getFrameInterface(), pParent, "svx/ui/fontworkcharacterspacingcontrol.ui", "FontworkCharacterSpacingControl")
     , mxControl(pControl)
-    , mxVeryTight(m_xBuilder->weld_toggle_button("verytight"))
-    , mxTight(m_xBuilder->weld_toggle_button("tight"))
-    , mxNormal(m_xBuilder->weld_toggle_button("normal"))
-    , mxLoose(m_xBuilder->weld_toggle_button("loose"))
-    , mxVeryLoose(m_xBuilder->weld_toggle_button("veryloose"))
-    , mxCustom(m_xBuilder->weld_toggle_button("custom"))
+    , mxVeryTight(m_xBuilder->weld_radio_button("verytight"))
+    , mxTight(m_xBuilder->weld_radio_button("tight"))
+    , mxNormal(m_xBuilder->weld_radio_button("normal"))
+    , mxLoose(m_xBuilder->weld_radio_button("loose"))
+    , mxVeryLoose(m_xBuilder->weld_radio_button("veryloose"))
+    , mxCustom(m_xBuilder->weld_radio_button("custom"))
     , mxKernPairs(m_xBuilder->weld_check_button("kernpairs"))
     , mnCharacterSpacing(0)
     , mbSettingValue(false)
+    , mbCommandDispatched(false)
 {
-    mxVeryTight->connect_clicked(LINK(this, FontworkCharacterSpacingWindow, SelectHdl));
-    mxTight->connect_clicked(LINK(this, FontworkCharacterSpacingWindow, SelectHdl));
-    mxNormal->connect_clicked(LINK(this, FontworkCharacterSpacingWindow, SelectHdl));
-    mxLoose->connect_clicked(LINK(this, FontworkCharacterSpacingWindow, SelectHdl));
-    mxVeryLoose->connect_clicked(LINK(this, FontworkCharacterSpacingWindow, SelectHdl));
-    mxCustom->connect_clicked(LINK(this, FontworkCharacterSpacingWindow, SelectHdl));
+    mxVeryTight->connect_toggled(LINK(this, FontworkCharacterSpacingWindow, SelectHdl));
+    mxTight->connect_toggled(LINK(this, FontworkCharacterSpacingWindow, SelectHdl));
+    mxNormal->connect_toggled(LINK(this, FontworkCharacterSpacingWindow, SelectHdl));
+    mxLoose->connect_toggled(LINK(this, FontworkCharacterSpacingWindow, SelectHdl));
+    mxVeryLoose->connect_toggled(LINK(this, FontworkCharacterSpacingWindow, SelectHdl));
+    mxCustom->connect_toggled(LINK(this, FontworkCharacterSpacingWindow, SelectHdl));
+    mxCustom->connect_mouse_release(LINK(this, FontworkCharacterSpacingWindow, MouseReleaseHdl));
 
     mxKernPairs->connect_toggled(LINK(this, FontworkCharacterSpacingWindow, KernSelectHdl));
 
@@ -544,6 +549,7 @@ void FontworkCharacterSpacingWindow::implSetCharacterSpacing( sal_Int32 nCharact
     mxNormal->set_active(false);
     mxLoose->set_active(false);
     mxVeryLoose->set_active(false);
+    mxCustom->set_active(true);
 
     switch(nCharacterSpacing)
     {
@@ -564,12 +570,6 @@ void FontworkCharacterSpacingWindow::implSetCharacterSpacing( sal_Int32 nCharact
             break;
     }
 
-    mxCustom->set_active(!mxVeryTight->get_active() &&
-                         !mxTight->get_active() &&
-                         !mxNormal->get_active() &&
-                         !mxLoose->get_active() &&
-                         !mxVeryLoose->get_active());
-
     mnCharacterSpacing = nCharacterSpacing;
 
     mbSettingValue = bSettingValue;
@@ -622,34 +622,41 @@ IMPL_LINK_NOARG(FontworkCharacterSpacingWindow, KernSelectHdl, weld::Toggleable&
     aArgs[0].Value <<= bKernOnOff;
 
     mxControl->dispatchCommand( gsFontworkKernCharacterPairs, aArgs );
+    mbCommandDispatched = true;
 
     implSetKernCharacterPairs(bKernOnOff, true);
 
     mxControl->EndPopupMode();
 }
 
-IMPL_LINK(FontworkCharacterSpacingWindow, SelectHdl, weld::Button&, rButton, void)
+void FontworkCharacterSpacingWindow::DispatchSpacingDialog()
 {
-    mxVeryTight->set_active(&rButton == mxVeryTight.get());
-    mxTight->set_active(&rButton == mxTight.get());
-    mxNormal->set_active(&rButton == mxNormal.get());
-    mxLoose->set_active(&rButton == mxLoose.get());
-    mxVeryLoose->set_active(&rButton == mxVeryLoose.get());
-    mxCustom->set_active(&rButton == mxCustom.get());
+    Sequence< PropertyValue > aArgs( 1 );
+    aArgs[0].Name = OUString(gsFontworkCharacterSpacing).copy(5);
+    aArgs[0].Value <<= mnCharacterSpacing;
+
+    rtl::Reference<svt::PopupWindowController> xControl(mxControl);
+    xControl->EndPopupMode();
+    xControl->dispatchCommand(".uno:FontworkCharacterSpacingDialog", aArgs);
+    mbCommandDispatched = true;
+}
+
+IMPL_LINK(FontworkCharacterSpacingWindow, SelectHdl, weld::Toggleable&, rButton, void)
+{
+    if (!rButton.get_active())
+        return;
 
     if (mbSettingValue)
         return;
 
-    if (mxCustom->get_active())
-    {
-        Sequence< PropertyValue > aArgs( 1 );
-        aArgs[0].Name = OUString(gsFontworkCharacterSpacing).copy(5);
-        aArgs[0].Value <<= mnCharacterSpacing;
+    // see MouseReleaseHdl for mbCommandDispatched check, there's no guarantee
+    // this toggle will happen before that mouse release though it does in
+    // practice for vcl and gtk
+    if (mbCommandDispatched)
+        return;
 
-        rtl::Reference<svt::PopupWindowController> xControl(mxControl);
-        xControl->EndPopupMode();
-        xControl->dispatchCommand(".uno:FontworkCharacterSpacingDialog", aArgs);
-    }
+    if (mxCustom->get_active())
+        DispatchSpacingDialog();
     else
     {
         sal_Int32 nCharacterSpacing;
@@ -669,6 +676,7 @@ IMPL_LINK(FontworkCharacterSpacingWindow, SelectHdl, weld::Button&, rButton, voi
         aArgs[0].Value <<= nCharacterSpacing;
 
         mxControl->dispatchCommand( gsFontworkCharacterSpacing,  aArgs );
+        mbCommandDispatched = true;
 
         implSetCharacterSpacing( nCharacterSpacing, true );
     }
@@ -676,6 +684,24 @@ IMPL_LINK(FontworkCharacterSpacingWindow, SelectHdl, weld::Button&, rButton, voi
     mxControl->EndPopupMode();
 }
 
+IMPL_LINK_NOARG(FontworkCharacterSpacingWindow, MouseReleaseHdl, const MouseEvent&, bool)
+{
+    /*
+     tdf#145296 if the "custom" radiobutton was presented preselected as
+     toggled on and the user clicked on it then there's no toggled signal sent
+     because the item was already toggled on and didn't change state.
+
+     So if that happens launch the custom spacing dialog explicitly here on
+     mouse release.
+    */
+    if (mxCustom->get_active() && !mbCommandDispatched)
+    {
+        DispatchSpacingDialog();
+        return true;
+    }
+    return false;
+}
+
 namespace {
 
 class FontworkCharacterSpacingControl : public svt::PopupWindowController