tdf#141101 tdf#101886 a11y: Restore previous focus on col/line popup close

Don't request focus for the color window
before the menu button popup opens.

Doing so would prevent properly restoring focus to the
menu button itself after the popup closes again.

Without this change in place, the call to
`MenuButton::Activate` in `MenuButton::ExecuteMenu (s. frame 14/15
in below backtrace) would already set focus to the "Automatic" button
in the color window:

        1  PushButton::GetFocus   button.cxx 1490 0x7f4acfdc83b6
        2  vcl::Window::CompatGetFocus    window.cxx 3888 0x7f4acfd9d26d
        3  vcl::Window::ImplGrabFocus mouse.cxx  384  0x7f4acfcd5215
        4  vcl::Window::GrabFocus window.cxx 2979 0x7f4acfd988e9
        5  SalInstanceWidget::grab_focus  salvtables.cxx 390  0x7f4ad046f505
        6  ColorWindow::GrabFocus tbcontrl.cxx   2127 0x7f4ad3f21af1
        7  ColorListBox::ToggleHdl    tbcontrl.cxx   4291 0x7f4ad3f313d6
        8  ColorListBox::LinkStubToggleHdl    tbcontrl.cxx   4285 0x7f4ad3f31369
        9  Link<weld::Toggleable&, void>::Call    link.hxx   111  0x7f4ad04aec71
        10 weld::Toggleable::signal_toggled   weld.hxx   1539 0x7f4ad04a77a3
        11 SalInstanceMenuButton::ActivateHdl salvtables.cxx 3075 0x7f4ad0483086
        12 SalInstanceMenuButton::LinkStubActivateHdl salvtables.cxx 3071 0x7f4ad0483043
        13 Link<MenuButton *, void>::Call link.hxx   111  0x7f4acfe9e9c1
        14 MenuButton::Activate   menubtn.cxx    237  0x7f4acfe9e136
        15 MenuButton::ExecuteMenu    menubtn.cxx    61   0x7f4acfe9cca1
        16 MenuButton::KeyInput   menubtn.cxx    226  0x7f4acfe9e092
        17 ImplHandleKey  winproc.cxx    1211 0x7f4acfdb0962
        18 ImplWindowFrameProc    winproc.cxx    2724 0x7f4acfdb6fcf
        19 SalFrame::CallCallback salframe.hxx   310  0x7f4ac5aa3dfa
        20 QtFrame::CallCallback  QtFrame.hxx    229  0x7f4ac5aa5336
        ... <More>

As a consequence, this "Automatic" button inside of the color window
would be the UI element remembered as the the one to which focus
focus should be restored when closing the popup, see the

    mxPrevFocusWin = Window::SaveFocus();

in `FloatingWindow::StartPopupMode`, which gets called like this:

    1   FloatingWindow::StartPopupMode           floatwin.cxx         824  0x7f4acfc61a61
    2   ImplDockingWindowWrapper::StartPopupMode dockmgr.cxx          846  0x7f4acfc43bd3
    3   DockingManager::StartPopupMode           dockmgr.cxx          341  0x7f4acfc412c3
    4   MenuButton::ExecuteMenu                  menubtn.cxx          94   0x7f4acfe9cfa9
    5   MenuButton::KeyInput                     menubtn.cxx          226  0x7f4acfe9e092
    6   ImplHandleKey                            winproc.cxx          1211 0x7f4acfdb0962
    7   ImplWindowFrameProc                      winproc.cxx          2724 0x7f4acfdb6fcf
    8   SalFrame::CallCallback                   salframe.hxx         310  0x7f4ac5aa3dfa
    9   QtFrame::CallCallback                    QtFrame.hxx          229  0x7f4ac5aa5336
    10  QtWidget::handleKeyEvent                 QtWidget.cxx         671  0x7f4ac5af9f38
    11  QtWidget::handleEvent                    QtWidget.cxx         707  0x7f4ac5afa094
    12  QtWidget::event                          QtWidget.cxx         730  0x7f4ac5afa1f7
    13  QApplicationPrivate::notify_helper       qapplication.cpp     3287 0x7f4ac37a2414
    14  QApplication::notify                     qapplication.cpp     2715 0x7f4ac379fd5b
    15  QCoreApplication::notifyInternal2        qcoreapplication.cpp 1123 0x7f4ac51a3c34
    16  QCoreApplication::forwardEvent           qcoreapplication.cpp 1138 0x7f4ac51a3cac
    17  QWidgetWindow::handleKeyEvent            qwidgetwindow.cpp    669  0x7f4ac38567b1
    18  QWidgetWindow::event                     qwidgetwindow.cpp    234  0x7f4ac3854924
    19  QApplicationPrivate::notify_helper       qapplication.cpp     3287 0x7f4ac37a2414
    20  QApplication::notify                     qapplication.cpp     3238 0x7f4ac37a2224
    ... <More>

and then properly restoring focus fails in
`FloatingWindow::EndPopupMode`.

Move the call to `Activate` to after showing the
popup instead. This makes sure that the actual
widget that had focus *before* the popup opened
is remembered and focus is correctly restored
on close.

The handler for the toggled signal had been added in

    commit e55a1dc163165cb79fc9113101d16ee8d3db7298
    Date:   Wed Nov 27 14:58:00 2019 +0000

        don't put focus into unmapped windows

        defer until the color selectors are activated to grab focus, otherwise
        esc doesn't work to close a dialog under gtk3 until focus is put
        into some visible widget

which apparently already moved the focus request to later than it was
before.

With this change in place, the NVDA screen reader announces the
menu button again once the color popup closes (tdf#141101) and it also
makes opening the popup menu again right away work
by pressing Alt+Down button again on Windows or with the
gen or qt6 VCL plugins on Linux, which didn't work beforehand,
but required either using the mouse or tabbing to another UI
element and back before that keyboard shortcut would work again.

The same is true for the border line style popup (tdf#101886).

Setting the focus only when the popup shows also
makes the focus correctly be on the previously
selected color for the non-gtk3 case when opening
the popup again. (Previously, the "Automatic" button
would always have focus.)

Ensure that the required preparations for showing the
popup in the `ManagedMenuButton` subclass are still
done before executing the menu by doing what's
needed in the newly named `ManagedMenuButton::PrepareExecute`
method rather than in `Activate` and call that
one before showing the menu.

Change-Id: I82fbfea2ae8b9064979796da279750350deb742d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/155891
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>

4 files changed, 9 insertions, 4 deletions

diff --git a/include/vcl/toolkit/menubtn.hxx b/include/vcl/toolkit/menubtn.hxx
index d02d077e8d8a..2af263fae24f 100644
--- a/include/vcl/toolkit/menubtn.hxx
+++ b/include/vcl/toolkit/menubtn.hxx
@@ -54,6 +54,8 @@ private:
 protected:
     using Window::ImplInit;
     SAL_DLLPRIVATE void    ImplInit( vcl::Window* pParent, WinBits nStyle );
+    // override in derived classes to set up anything needed to execute menu
+    virtual void PrepareExecute() {};
 
 public:
     explicit        MenuButton( vcl::Window* pParent, WinBits nStyle = 0 );
diff --git a/vcl/inc/managedmenubutton.hxx b/vcl/inc/managedmenubutton.hxx
index cf9b0fd9e5f5..0f6492b6fdee 100644
--- a/vcl/inc/managedmenubutton.hxx
+++ b/vcl/inc/managedmenubutton.hxx
@@ -19,10 +19,11 @@ public:
     ManagedMenuButton(vcl::Window* pParent, WinBits nStyle);
     ~ManagedMenuButton() override;
 
-    void Activate() override;
     void dispose() override;
 
 private:
+    void PrepareExecute() override;
+
     css::uno::Reference<css::awt::XPopupMenu> m_xPopupMenu;
     css::uno::Reference<css::frame::XPopupMenuController> m_xPopupController;
 };
diff --git a/vcl/source/control/managedmenubutton.cxx b/vcl/source/control/managedmenubutton.cxx
index bf3065e71cdf..7545dba9b374 100644
--- a/vcl/source/control/managedmenubutton.cxx
+++ b/vcl/source/control/managedmenubutton.cxx
@@ -39,12 +39,12 @@ void ManagedMenuButton::dispose()
     MenuButton::dispose();
 }
 
-void ManagedMenuButton::Activate()
+void ManagedMenuButton::PrepareExecute()
 {
     if (!GetPopupMenu())
         SetPopupMenu(VclPtr<PopupMenu>::Create());
 
-    MenuButton::Activate();
+    MenuButton::PrepareExecute();
 
     if (m_xPopupController.is())
     {
diff --git a/vcl/source/control/menubtn.cxx b/vcl/source/control/menubtn.cxx
index 64aec098db0f..186340d8e047 100644
--- a/vcl/source/control/menubtn.cxx
+++ b/vcl/source/control/menubtn.cxx
@@ -57,7 +57,7 @@ void MenuButton::ExecuteMenu()
 {
     mbStartingMenu = true;
 
-    Activate();
+    PrepareExecute();
 
     if (!mpMenu && !mpFloatingWindow)
     {
@@ -94,6 +94,8 @@ void MenuButton::ExecuteMenu()
         }
     }
 
+    Activate();
+
     mbStartingMenu = false;
 
     SetPressed(false);