Related: tdf#125040 NB: rework print preview context

Current implementation broadcasts the context change before the
new controller was set on the frame (via XFrame::setComponent).
This isn't going to work for anything LayoutManager based, as
it responds to frame events by recreating all toolbars, so the
newly created toolbar won't see the context sent for the old
controller.

Solve that by delaying the context change to SfxShell::Activate,
like anywhere else in the codebase. And make NB listen to frame
events, so it could also switch listening to the new controller.

Also fixed a crash when switching to print preview in Writer with
the navigator in the sidebar being active. Ideally we shouldn't
even try to create sidebar panels in print preview, as the sidebar
as a whole is disabled. But left that for a future investigation.

Change-Id: I07759c676d2a2eb6f752fe778b559b15d2d759ec
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/135492
Tested-by: Jenkins
Reviewed-by: Maxim Monastirsky <momonasmon@gmail.com>

8 files changed, 77 insertions, 77 deletions

diff --git a/include/vcl/notebookbar/notebookbar.hxx b/include/vcl/notebookbar/notebookbar.hxx
index 635bea058f2a..b93e3707fb7c 100644
--- a/include/vcl/notebookbar/notebookbar.hxx
+++ b/include/vcl/notebookbar/notebookbar.hxx
@@ -18,6 +18,7 @@
 
 namespace com::sun::star::ui { class XContextChangeEventListener; }
 
+class NotebookBarContextChangeEventListener;
 class NotebookbarContextControl;
 class SystemWindow;
 class SfxViewShell;
@@ -44,8 +45,7 @@ public:
 
     void DataChanged(const DataChangedEvent& rDCEvt) override;
 
-    void ControlListenerForCurrentController(bool bListen);
-    void StopListeningAllControllers();
+    void SetupListener(bool bListen);
 
     bool IsWelded() const { return m_bIsWelded; }
     VclPtr<vcl::Window>& GetMainContainer() { return m_xVclContentArea; }
@@ -54,10 +54,8 @@ public:
 
 private:
     VclPtr<SystemWindow> m_pSystemWindow;
-    css::uno::Reference<css::ui::XContextChangeEventListener> m_pEventListener;
-    std::set<css::uno::Reference<css::frame::XController>> m_alisteningControllers;
+    rtl::Reference<NotebookBarContextChangeEventListener> m_pEventListener;
     std::vector<NotebookbarContextControl*> m_pContextContainers;
-    css::uno::Reference<css::frame::XFrame> mxFrame;
     const SfxViewShell* m_pViewShell;
 
     VclPtr<vcl::Window> m_xVclContentArea;
diff --git a/sc/source/ui/view/prevwsh.cxx b/sc/source/ui/view/prevwsh.cxx
index 18cae40634d7..d38b4311f143 100644
--- a/sc/source/ui/view/prevwsh.cxx
+++ b/sc/source/ui/view/prevwsh.cxx
@@ -155,26 +155,7 @@ ScPreviewShell::ScPreviewShell( SfxViewFrame* pViewFrame,
     nMaxVertPos(0)
 {
     Construct( &pViewFrame->GetWindow() );
-
-    try
-    {
-        SfxShell::SetContextBroadcasterEnabled(true);
-        SfxShell::SetContextName(
-            vcl::EnumContext::GetContextName(vcl::EnumContext::Context::Printpreview));
-        SfxShell::BroadcastContextForActivation(true);
-    }
-    catch (const css::uno::RuntimeException& e)
-    {
-        // tdf#130559: allow BackingComp to fail adding listener when opening document
-        css::uno::Reference<css::lang::XServiceInfo> xServiceInfo(e.Context, css::uno::UNO_QUERY);
-        if (!xServiceInfo || !xServiceInfo->supportsService("com.sun.star.frame.StartModule"))
-            throw;
-        SAL_WARN("sc.ui", "Opening file from StartModule straight into print preview");
-    }
-
-    auto& pNotebookBar = pViewFrame->GetWindow().GetSystemWindow()->GetNotebookBar();
-    if (pNotebookBar)
-        pNotebookBar->ControlListenerForCurrentController(false); // stop listening
+    SfxShell::SetContextName(vcl::EnumContext::GetContextName(vcl::EnumContext::Context::Printpreview));
 
     if ( auto pTabViewShell = dynamic_cast<ScTabViewShell*>( pOldSh) )
     {
@@ -202,9 +183,6 @@ ScPreviewShell::~ScPreviewShell()
     if (mpFrameWindow)
         mpFrameWindow->SetCloseHdl(Link<SystemWindow&,void>()); // Remove close handler.
 
-    if (auto& pBar = GetViewFrame()->GetWindow().GetSystemWindow()->GetNotebookBar())
-        pBar->ControlListenerForCurrentController(true); // let it start listening now
-
     // #108333#; notify Accessibility that Shell is dying and before destroy all
     BroadcastAccessibility( SfxHint( SfxHintId::Dying ) );
     pAccessibilityBroadcaster.reset();
@@ -560,6 +538,8 @@ void ScPreviewShell::Activate(bool bMDI)
         if ( pInputHdl )
             pInputHdl->NotifyChange( nullptr );
     }
+
+    SfxShell::Activate(bMDI);
 }
 
 void ScPreviewShell::Execute( SfxRequest& rReq )
diff --git a/sfx2/source/notebookbar/SfxNotebookBar.cxx b/sfx2/source/notebookbar/SfxNotebookBar.cxx
index aa602ba17a60..451c94cf3966 100644
--- a/sfx2/source/notebookbar/SfxNotebookBar.cxx
+++ b/sfx2/source/notebookbar/SfxNotebookBar.cxx
@@ -202,7 +202,6 @@ void SfxNotebookBar::CloseMethod(SystemWindow* pSysWindow)
 {
     if (pSysWindow)
     {
-        RemoveListeners(pSysWindow);
         if(pSysWindow->GetNotebookBar())
             pSysWindow->CloseNotebookBar();
         if (SfxViewFrame::Current())
@@ -432,7 +431,7 @@ bool SfxNotebookBar::StateMethod(SystemWindow* pSysWindow,
 
             if(pView)
             {
-                pNotebookBar->ControlListenerForCurrentController(true);
+                pNotebookBar->SetupListener(true);
             }
         }
 
@@ -441,7 +440,6 @@ bool SfxNotebookBar::StateMethod(SystemWindow* pSysWindow,
     else if (auto pNotebookBar = pSysWindow->GetNotebookBar())
     {
         vcl::Window* pParent = pNotebookBar->GetParent();
-        RemoveListeners(pSysWindow);
         pSysWindow->CloseNotebookBar();
         pParent->Resize();
         SfxNotebookBar::ShowMenubar(true);
@@ -454,7 +452,7 @@ void SfxNotebookBar::RemoveListeners(SystemWindow const * pSysWindow)
 {
     if (auto pNotebookBar = pSysWindow->GetNotebookBar())
     {
-        pNotebookBar->StopListeningAllControllers();
+        pNotebookBar->SetupListener(false);
     }
 }
 
diff --git a/sw/source/uibase/inc/pview.hxx b/sw/source/uibase/inc/pview.hxx
index 5d749564f5e5..2252c8740d28 100644
--- a/sw/source/uibase/inc/pview.hxx
+++ b/sw/source/uibase/inc/pview.hxx
@@ -220,6 +220,8 @@ class SW_DLLPUBLIC SwPagePreview final : public SfxViewShell
     virtual void    InnerResizePixel( const Point &rOfs, const Size &rSize, bool inplaceEditModeChange ) override;
     virtual void    OuterResizePixel( const Point &rOfs, const Size &rSize ) override;
 
+    void Activate(bool bMDI) override;
+
     void         SetZoom(SvxZoomType eSet, sal_uInt16 nFactor);
 
 public:
diff --git a/sw/source/uibase/uiview/pview.cxx b/sw/source/uibase/uiview/pview.cxx
index 4ae41bb68fff..ffe1e0d3a6e8 100644
--- a/sw/source/uibase/uiview/pview.cxx
+++ b/sw/source/uibase/uiview/pview.cxx
@@ -1152,14 +1152,7 @@ SwPagePreview::SwPagePreview(SfxViewFrame *pViewFrame, SfxViewShell* pOldSh):
     CreateScrollbar( true );
     CreateScrollbar( false );
 
-    //notify notebookbar change in context
-    SfxShell::SetContextBroadcasterEnabled(true);
     SfxShell::SetContextName(vcl::EnumContext::GetContextName(vcl::EnumContext::Context::Printpreview));
-    SfxShell::BroadcastContextForActivation(true);
-    //removelisteners for notebookbar
-    if (SfxViewFrame* pCurrent = SfxViewFrame::Current())
-        if (auto& pBar = pCurrent->GetWindow().GetSystemWindow()->GetNotebookBar())
-            pBar->ControlListenerForCurrentController(false);
 
     SfxObjectShell* pObjShell = pViewFrame->GetObjectShell();
     if ( !pOldSh )
@@ -1225,14 +1218,17 @@ SwPagePreview::~SwPagePreview()
     delete pVShell;
 
     m_pViewWin.disposeAndClear();
-    if (SfxViewFrame* pCurrent = SfxViewFrame::Current())
-        if (auto& pBar = pCurrent->GetWindow().GetSystemWindow()->GetNotebookBar())
-            pBar->ControlListenerForCurrentController(true); // start listening now
     m_pScrollFill.disposeAndClear();
     m_pHScrollbar.disposeAndClear();
     m_pVScrollbar.disposeAndClear();
 }
 
+void SwPagePreview::Activate(bool bMDI)
+{
+    SfxViewShell::Activate(bMDI);
+    SfxShell::Activate(bMDI);
+}
+
 SwDocShell* SwPagePreview::GetDocShell()
 {
     return dynamic_cast<SwDocShell*>( GetViewFrame()->GetObjectShell() );
diff --git a/sw/source/uibase/utlui/content.cxx b/sw/source/uibase/utlui/content.cxx
index de9275d3d1b9..ebf0a02794c7 100644
--- a/sw/source/uibase/utlui/content.cxx
+++ b/sw/source/uibase/utlui/content.cxx
@@ -2956,16 +2956,14 @@ void SwContentTree::SetActiveShell(SwWrtShell* pSh)
     bool bClear = m_pActiveShell != pSh;
     if (State::ACTIVE == m_eState && bClear)
     {
-        if (m_pActiveShell)
-            EndListening(*m_pActiveShell->GetView().GetDocShell());
+        EndListeningAll();
         m_pActiveShell = pSh;
         FindActiveTypeAndRemoveUserData();
         clear();
     }
     else if (State::CONSTANT == m_eState)
     {
-        if (m_pActiveShell)
-            EndListening(*m_pActiveShell->GetView().GetDocShell());
+        EndListeningAll();
         m_pActiveShell = pSh;
         m_eState = State::ACTIVE;
         bClear = true;
@@ -2997,8 +2995,7 @@ void SwContentTree::SetActiveShell(SwWrtShell* pSh)
 
 void SwContentTree::SetConstantShell(SwWrtShell* pSh)
 {
-    if (m_pActiveShell)
-        EndListening(*m_pActiveShell->GetView().GetDocShell());
+    EndListeningAll();
     m_pActiveShell = pSh;
     m_eState = State::CONSTANT;
     StartListening(*m_pActiveShell->GetView().GetDocShell());
diff --git a/sw/source/uibase/utlui/navipi.cxx b/sw/source/uibase/utlui/navipi.cxx
index 669234d6bcf7..773e40baa0c1 100644
--- a/sw/source/uibase/utlui/navipi.cxx
+++ b/sw/source/uibase/utlui/navipi.cxx
@@ -804,6 +804,7 @@ void SwNavigationPI::Notify( SfxBroadcaster& rBrdc, const SfxHint& rHint )
         {
             EndListening(*m_pCreateView);
             m_pCreateView = nullptr;
+            m_xContentTree->SetActiveShell(nullptr);
         }
     }
     else
diff --git a/vcl/source/control/notebookbar.cxx b/vcl/source/control/notebookbar.cxx
index 24e619ff98e1..abbd0c122615 100644
--- a/vcl/source/control/notebookbar.cxx
+++ b/vcl/source/control/notebookbar.cxx
@@ -10,6 +10,7 @@
 #include <sal/config.h>
 
 #include <string_view>
+#include <utility>
 
 #include <vcl/layout.hxx>
 #include <vcl/notebookbar/notebookbar.hxx>
@@ -22,6 +23,7 @@
 #include <osl/file.hxx>
 #include <config_folders.h>
 #include <com/sun/star/frame/XFrame.hpp>
+#include <com/sun/star/frame/FrameAction.hpp>
 #include <com/sun/star/ui/ContextChangeEventMultiplexer.hpp>
 #include <comphelper/lok.hxx>
 
@@ -43,15 +45,27 @@ static bool doesFileExist(std::u16string_view sUIDir, std::u16string_view sUIFil
 /**
  * split from the main class since it needs different ref-counting mana
  */
-class NotebookBarContextChangeEventListener : public ::cppu::WeakImplHelper<css::ui::XContextChangeEventListener>
+class NotebookBarContextChangeEventListener : public ::cppu::WeakImplHelper<css::ui::XContextChangeEventListener, css::frame::XFrameActionListener>
 {
+    bool mbActive;
     VclPtr<NotebookBar> mpParent;
+    css::uno::Reference<css::frame::XFrame> mxFrame;
 public:
-    explicit NotebookBarContextChangeEventListener(NotebookBar *p) : mpParent(p) {}
+    NotebookBarContextChangeEventListener(NotebookBar *p, css::uno::Reference<css::frame::XFrame> xFrame) :
+        mbActive(false),
+        mpParent(p),
+        mxFrame(std::move(xFrame))
+    {}
+
+    void setupFrameListener(bool bListen);
+    void setupListener(bool bListen);
 
     // XContextChangeEventListener
     virtual void SAL_CALL notifyContextChangeEvent(const css::ui::ContextChangeEventObject& rEvent) override;
 
+    // XFrameActionListener
+    virtual void SAL_CALL frameAction(const css::frame::FrameActionEvent& rEvent) override;
+
     virtual void SAL_CALL disposing(const ::css::lang::EventObject&) override;
 };
 
@@ -59,12 +73,12 @@ NotebookBar::NotebookBar(Window* pParent, const OString& rID, const OUString& rU
                          const css::uno::Reference<css::frame::XFrame>& rFrame,
                          const NotebookBarAddonsItem& aNotebookBarAddonsItem)
     : Control(pParent)
-    , m_pEventListener(new NotebookBarContextChangeEventListener(this))
+    , m_pEventListener(new NotebookBarContextChangeEventListener(this, rFrame))
     , m_pViewShell(nullptr)
     , m_bIsWelded(false)
     , m_sUIXMLDescription(rUIXMLDescription)
 {
-    mxFrame = rFrame;
+    m_pEventListener->setupFrameListener(true);
 
     SetStyle(GetStyle() | WB_DIALOGCONTROL);
     OUString sUIDir = AllSettings::GetUIRootDir();
@@ -133,7 +147,8 @@ void NotebookBar::dispose()
     else
         disposeBuilder();
 
-    assert(m_alisteningControllers.empty());
+    m_pEventListener->setupFrameListener(false);
+    m_pEventListener->setupListener(false);
     m_pEventListener.clear();
 
     Control::dispose();
@@ -225,45 +240,58 @@ void SAL_CALL NotebookBarContextChangeEventListener::notifyContextChangeEvent(co
     }
 }
 
-void NotebookBar::ControlListenerForCurrentController(bool bListen)
+void NotebookBarContextChangeEventListener::setupListener(bool bListen)
 {
     if (comphelper::LibreOfficeKit::isActive())
         return;
 
-    auto xController = mxFrame->getController();
-    if(bListen)
+    auto xMultiplexer(css::ui::ContextChangeEventMultiplexer::get(::comphelper::getProcessComponentContext()));
+
+    if (bListen)
     {
-        // add listeners
-        if (m_alisteningControllers.count(xController) == 0)
+        try
         {
-            auto xMultiplexer(css::ui::ContextChangeEventMultiplexer::get(
-                ::comphelper::getProcessComponentContext()));
-            xMultiplexer->addContextChangeEventListener(m_pEventListener, xController);
-            m_alisteningControllers.insert(xController);
+            xMultiplexer->addContextChangeEventListener(this, mxFrame->getController());
         }
-    }
-    else
-    {
-        // remove listeners
-        if (m_alisteningControllers.count(xController))
+        catch (const css::uno::Exception&)
         {
-            auto xMultiplexer(css::ui::ContextChangeEventMultiplexer::get(
-                ::comphelper::getProcessComponentContext()));
-            xMultiplexer->removeContextChangeEventListener(m_pEventListener, xController);
-            m_alisteningControllers.erase(xController);
         }
     }
+    else
+        xMultiplexer->removeAllContextChangeEventListeners(this);
+
+    mbActive = bListen;
 }
 
-void NotebookBar::StopListeningAllControllers()
+void NotebookBarContextChangeEventListener::setupFrameListener(bool bListen)
 {
-    if (comphelper::LibreOfficeKit::isActive())
+    if (bListen)
+        mxFrame->addFrameActionListener(this);
+    else
+        mxFrame->removeFrameActionListener(this);
+}
+
+void SAL_CALL NotebookBarContextChangeEventListener::frameAction(const css::frame::FrameActionEvent& rEvent)
+{
+    if (!mbActive)
         return;
 
-    auto xMultiplexer(
-        css::ui::ContextChangeEventMultiplexer::get(comphelper::getProcessComponentContext()));
-    xMultiplexer->removeAllContextChangeEventListeners(m_pEventListener);
-    m_alisteningControllers.clear();
+    if (rEvent.Action == css::frame::FrameAction_COMPONENT_REATTACHED)
+    {
+        setupListener(true);
+    }
+    else if (rEvent.Action == css::frame::FrameAction_COMPONENT_DETACHING)
+    {
+        setupListener(false);
+        // We don't want to give up on listening; just wait for
+        // another controller to be attached to the frame.
+        mbActive = true;
+    }
+}
+
+void NotebookBar::SetupListener(bool bListen)
+{
+    m_pEventListener->setupListener(bListen);
 }
 
 void SAL_CALL NotebookBarContextChangeEventListener::disposing(const ::css::lang::EventObject&)