sw/CppunitTest_sw_uibase_uiview.mk | 3 sw/inc/view.hxx | 1 sw/qa/uibase/uiview/uiview.cxx | 171 +++++++++++++++++++++++++++++++++++++ sw/source/uibase/uiview/view.cxx | 28 ++++++ 4 files changed, 202 insertions(+), 1 deletion(-)
New commits: commit 6c1fca897e148ce6200ad57a3b6eaf188448c069 Author: Miklos Vajna <[email protected]> AuthorDate: Tue Aug 23 11:58:29 2022 +0200 Commit: Miklos Vajna <[email protected]> CommitDate: Wed Aug 24 10:22:15 2022 +0200 sw: fix missing cache invalidation when switching between images It is possible to disable toolbar buttons from UNO API client code by registering a dispatch provider interceptor and then returning an empty reference when the UNO command associated with that toolbar button is queried for a dispatch. Such querying of a dispatch happens when changing context (e.g. text -> image selection), but not when switching between two images. A benefit of the current approach is that once a button is disabled this way, it remains disabled without re-querying the dispatch provider, which helps performance. A downside is that in case the dispatch provider intercepts the command based on the current selection (e.g. currently selected image), then switching to an other image won't re-query the dispatch provider, for at least two reasons: - SfxBindings::Register_Impl() is only called when the dispatcher is an internal one (e.g. not implemented in Java), so there is no listener that would re-query the state on selection change - even if we re-query the dispatch provider, the actual toolbar button won't be updated if the initial dispatch was an empty reference, since updating works by registering a status listener on the returned dispatch object in svt::ToolboxController::bindListener() Fix the problem by explicitly calling contextChanged() on the current frame when switching between images (but not changing context), similar to how SvtCommandOptions_Impl::Notify() invalidates registered dispatch objects when the configuration (on what commands are disabled) changes. This only helps with images and OLE objects, other object types are kept unchanged for now. (cherry picked from commit 31cb5b5538b9fd91dafb067ce961f2540555ad2b) Change-Id: I7f33dd2804067acf5cb0ca836f6a2a69fa950a8b diff --git a/sw/CppunitTest_sw_uibase_uiview.mk b/sw/CppunitTest_sw_uibase_uiview.mk index 66c95375bd56..08d0976c13c4 100644 --- a/sw/CppunitTest_sw_uibase_uiview.mk +++ b/sw/CppunitTest_sw_uibase_uiview.mk @@ -62,7 +62,8 @@ $(eval $(call gb_CppunitTest_use_custom_headers,sw_uibase_uiview,\ officecfg/registry \ )) -$(eval $(call gb_CppunitTest_use_configuration,sw_uibase_uiview)) +$(eval $(call gb_CppunitTest_use_instdir_configuration,sw_uibase_uiview)) +$(eval $(call gb_CppunitTest_use_common_configuration,sw_uibase_uiview)) $(eval $(call gb_CppunitTest_use_uiconfigs,sw_uibase_uiview, \ modules/swriter \ diff --git a/sw/inc/view.hxx b/sw/inc/view.hxx index 2a7bc89ab7e3..6d9d2cf19c66 100644 --- a/sw/inc/view.hxx +++ b/sw/inc/view.hxx @@ -202,6 +202,7 @@ class SW_DLLPUBLIC SwView: public SfxViewShell std::unique_ptr<SwDrawBase> m_pDrawActual; const SwFrameFormat *m_pLastTableFormat; + const SwFrameFormat* m_pLastFlyFormat; std::unique_ptr<SwFormatClipboard> m_pFormatClipboard; //holds data for format paintbrush diff --git a/sw/qa/uibase/uiview/uiview.cxx b/sw/qa/uibase/uiview/uiview.cxx index b7826bc9ad52..d49e3ee70dc8 100644 --- a/sw/qa/uibase/uiview/uiview.cxx +++ b/sw/qa/uibase/uiview/uiview.cxx @@ -14,17 +14,20 @@ #include <osl/file.hxx> #include <comphelper/propertyvalue.hxx> #include <comphelper/scopeguard.hxx> +#include <vcl/scheduler.hxx> #include <com/sun/star/frame/DispatchHelper.hpp> #include <com/sun/star/frame/XComponentLoader.hpp> #include <com/sun/star/lang/XMultiServiceFactory.hpp> #include <com/sun/star/packages/zip/ZipFileAccess.hpp> +#include <com/sun/star/view/XSelectionSupplier.hpp> #include <unotxdoc.hxx> #include <docsh.hxx> #include <wrtsh.hxx> #include <swdtflvr.hxx> #include <swmodule.hxx> +#include <view.hxx> char const DATA_DIRECTORY[] = "/sw/qa/uibase/uiview/data/"; @@ -146,6 +149,174 @@ CPPUNIT_TEST_FIXTURE(SwUibaseUiviewTest, testKeepRatio) assertXPathContent(pXmlDoc, "//config:config-item[@config:name='KeepRatio']", "true"); } +namespace +{ +/// Interception implementation that disables .uno:Zoom on Image1, but not on Image2. +struct ImageInterceptor : public cppu::WeakImplHelper<frame::XDispatchProviderInterceptor> +{ + uno::Reference<view::XSelectionSupplier> m_xSelectionSupplier; + uno::Reference<frame::XDispatchProvider> m_xMaster; + uno::Reference<frame::XDispatchProvider> m_xSlave; + int m_nEnabled = 0; + int m_nDisabled = 0; + +public: + ImageInterceptor(const uno::Reference<lang::XComponent>& xComponent); + + // XDispatchProviderInterceptor + uno::Reference<frame::XDispatchProvider> SAL_CALL getMasterDispatchProvider() override; + uno::Reference<frame::XDispatchProvider> SAL_CALL getSlaveDispatchProvider() override; + void SAL_CALL setMasterDispatchProvider( + const uno::Reference<frame::XDispatchProvider>& xNewSupplier) override; + void SAL_CALL + setSlaveDispatchProvider(const uno::Reference<frame::XDispatchProvider>& xNewSupplier) override; + + // XDispatchProvider + uno::Reference<frame::XDispatch> SAL_CALL queryDispatch(const util::URL& rURL, + const OUString& rTargetFrameName, + sal_Int32 SearchFlags) override; + uno::Sequence<uno::Reference<frame::XDispatch>> SAL_CALL + queryDispatches(const uno::Sequence<frame::DispatchDescriptor>& rRequests) override; +}; +} + +ImageInterceptor::ImageInterceptor(const uno::Reference<lang::XComponent>& xComponent) +{ + uno::Reference<frame::XModel2> xModel(xComponent, uno::UNO_QUERY); + CPPUNIT_ASSERT(xModel.is()); + m_xSelectionSupplier.set(xModel->getCurrentController(), uno::UNO_QUERY); + CPPUNIT_ASSERT(m_xSelectionSupplier.is()); +} + +uno::Reference<frame::XDispatchProvider> ImageInterceptor::getMasterDispatchProvider() +{ + return m_xMaster; +} + +uno::Reference<frame::XDispatchProvider> ImageInterceptor::getSlaveDispatchProvider() +{ + return m_xSlave; +} + +void ImageInterceptor::setMasterDispatchProvider( + const uno::Reference<frame::XDispatchProvider>& xNewSupplier) +{ + m_xMaster = xNewSupplier; +} + +void ImageInterceptor::setSlaveDispatchProvider( + const uno::Reference<frame::XDispatchProvider>& xNewSupplier) +{ + m_xSlave = xNewSupplier; +} + +uno::Reference<frame::XDispatch> ImageInterceptor::queryDispatch(const util::URL& rURL, + const OUString& rTargetFrameName, + sal_Int32 nSearchFlags) +{ + // Disable the UNO command based on the currently selected image, i.e. this can't be cached when + // a different image is selected. Originally this was .uno:SetBorderStyle, but let's pick a + // command which is active when running cppunit tests: + if (rURL.Complete == ".uno:Zoom") + { + uno::Reference<container::XNamed> xImage; + m_xSelectionSupplier->getSelection() >>= xImage; + if (xImage.is() && xImage->getName() == "Image1") + { + ++m_nDisabled; + return {}; + } + + ++m_nEnabled; + } + + return m_xSlave->queryDispatch(rURL, rTargetFrameName, nSearchFlags); +} + +uno::Sequence<uno::Reference<frame::XDispatch>> +ImageInterceptor::queryDispatches(const uno::Sequence<frame::DispatchDescriptor>& /*rRequests*/) +{ + return {}; +} + +CPPUNIT_TEST_FIXTURE(SwUibaseUiviewTest, testSwitchBetweenImages) +{ + // Given a document with 2 images, and an interceptor catching an UNO command that specific to + // the current selection: + loadURL("private:factory/swriter", nullptr); + SwXTextDocument* pTextDoc = dynamic_cast<SwXTextDocument*>(mxComponent.get()); + SwDoc* pDoc = pTextDoc->GetDocShell()->GetDoc(); + SwWrtShell* pWrtShell = pDoc->GetDocShell()->GetWrtShell(); + uno::Reference<lang::XMultiServiceFactory> xMSF(mxComponent, uno::UNO_QUERY); + uno::Reference<text::XTextDocument> xTextDocument(mxComponent, uno::UNO_QUERY); + uno::Reference<text::XText> xText = xTextDocument->getText(); + uno::Reference<text::XTextCursor> xCursor = xText->createTextCursor(); + for (int i = 0; i < 2; ++i) + { + uno::Reference<beans::XPropertySet> xTextGraphic( + xMSF->createInstance("com.sun.star.text.TextGraphicObject"), uno::UNO_QUERY); + xTextGraphic->setPropertyValue("AnchorType", + uno::Any(text::TextContentAnchorType_AS_CHARACTER)); + xTextGraphic->setPropertyValue("Size", uno::Any(awt::Size(5000, 5000))); + uno::Reference<text::XTextContent> xTextContent(xTextGraphic, uno::UNO_QUERY); + xText->insertTextContent(xCursor, xTextContent, false); + } + pWrtShell->SttEndDoc(/*bStt=*/false); + uno::Reference<frame::XModel> xModel(mxComponent, uno::UNO_QUERY); + uno::Reference<frame::XDispatchProviderInterception> xRegistration( + xModel->getCurrentController()->getFrame(), uno::UNO_QUERY); + rtl::Reference pInterceptor(new ImageInterceptor(mxComponent)); + uno::Reference pUnoInterceptor(pInterceptor.get()); + + xRegistration->registerDispatchProviderInterceptor(pUnoInterceptor); + pInterceptor->m_nEnabled = 0; + pInterceptor->m_nDisabled = 0; + + // When selecting the first image: + uno::Reference<text::XTextGraphicObjectsSupplier> xGraphicObjectsSupplier(mxComponent, + uno::UNO_QUERY); + uno::Reference<container::XIndexAccess> xGraphicObjects( + xGraphicObjectsSupplier->getGraphicObjects(), uno::UNO_QUERY); + pInterceptor->m_xSelectionSupplier->select(xGraphicObjects->getByIndex(0)); + Scheduler::ProcessEventsToIdle(); + SwView* pView = pDoc->GetDocShell()->GetView(); + pView->StopShellTimer(); + + // Then make sure the UNO command is disabled: + CPPUNIT_ASSERT_EQUAL(0, pInterceptor->m_nEnabled); + CPPUNIT_ASSERT_GREATEREQUAL(1, pInterceptor->m_nDisabled); + + // Given a clean state: + pInterceptor->m_nEnabled = 0; + pInterceptor->m_nDisabled = 0; + + // When selecting the second image: + pInterceptor->m_xSelectionSupplier->select(xGraphicObjects->getByIndex(1)); + Scheduler::ProcessEventsToIdle(); + pView->StopShellTimer(); + + // Then make sure the UNO command is enabled: + CPPUNIT_ASSERT_GREATEREQUAL(1, pInterceptor->m_nEnabled); + CPPUNIT_ASSERT_EQUAL(0, pInterceptor->m_nDisabled); + + // Given a clean state: + pInterceptor->m_nEnabled = 0; + pInterceptor->m_nDisabled = 0; + + // When selecting the first image, again (this time not changing the selection type): + pInterceptor->m_xSelectionSupplier->select(xGraphicObjects->getByIndex(0)); + Scheduler::ProcessEventsToIdle(); + pView->StopShellTimer(); + + // Then make sure the UNO command is disabled: + CPPUNIT_ASSERT_EQUAL(0, pInterceptor->m_nEnabled); + // Without the accompanying fix in place, this test would have failed with: + // - Expected greater or equal than: 1 + // - Actual : 0 + // i.e. selecting the first image didn't result in a disabled UNO command. + CPPUNIT_ASSERT_GREATEREQUAL(1, pInterceptor->m_nDisabled); +} + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sw/source/uibase/uiview/view.cxx b/sw/source/uibase/uiview/view.cxx index 2a701452a6c8..bfe356615f4c 100644 --- a/sw/source/uibase/uiview/view.cxx +++ b/sw/source/uibase/uiview/view.cxx @@ -257,6 +257,19 @@ void SwView::SelectShell() SelectionType nNewSelectionType = m_pWrtShell->GetSelectionType() & ~SelectionType::TableCell; + // Determine if a different fly frame was selected. + bool bUpdateFly = false; + const SwFrameFormat* pCurFlyFormat = nullptr; + if (m_nSelectionType & SelectionType::Ole || m_nSelectionType & SelectionType::Graphic) + { + pCurFlyFormat = m_pWrtShell->GetFlyFrameFormat(); + } + if (pCurFlyFormat && pCurFlyFormat != m_pLastFlyFormat) + { + bUpdateFly = true; + } + m_pLastFlyFormat = pCurFlyFormat; + if ( m_pFormShell && m_pFormShell->IsActiveControl() ) nNewSelectionType |= SelectionType::FormControl; @@ -267,6 +280,20 @@ void SwView::SelectShell() m_nSelectionType & SelectionType::Graphic ) // For graphs and OLE the verb can be modified of course! ImpSetVerb( nNewSelectionType ); + + if (bUpdateFly) + { + SfxViewFrame* pViewFrame = GetViewFrame(); + if (pViewFrame) + { + uno::Reference<frame::XFrame> xFrame = pViewFrame->GetFrame().GetFrameInterface(); + if (xFrame.is()) + { + // Invalidate cached dispatch objects. + xFrame->contextChanged(); + } + } + } } else { @@ -715,6 +742,7 @@ SwView::SwView( SfxViewFrame *_pFrame, SfxViewShell* pOldSh ) GetViewFrame()->GetBindings(), WB_VSCROLL | WB_EXTRAFIELD | WB_BORDER )), m_pLastTableFormat(nullptr), + m_pLastFlyFormat(nullptr), m_pFormatClipboard(new SwFormatClipboard()), m_nSelectionType( SelectionType::All ), m_nPageCnt(0),
