sw/CppunitTest_sw_uibase_uiview.mk | 3 sw/inc/view.hxx | 1 sw/qa/uibase/uiview/uiview.cxx | 168 +++++++++++++++++++++++++++++++++++++ sw/source/uibase/uiview/view.cxx | 28 ++++++ 4 files changed, 199 insertions(+), 1 deletion(-)
New commits: commit 6fa42a728738ccd00bc6f44f8bbf90fe13bba56b Author: Miklos Vajna <[email protected]> AuthorDate: Tue Aug 23 11:58:29 2022 +0200 Commit: Xisco Fauli <[email protected]> CommitDate: Mon Oct 3 14:46:33 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. Change-Id: I7f33dd2804067acf5cb0ca836f6a2a69fa950a8b Reviewed-on: https://gerrit.libreoffice.org/c/core/+/138724 Reviewed-by: Miklos Vajna <[email protected]> Tested-by: Jenkins (cherry picked from commit 31cb5b5538b9fd91dafb067ce961f2540555ad2b) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/138831 Reviewed-by: Xisco Fauli <[email protected]> diff --git a/sw/CppunitTest_sw_uibase_uiview.mk b/sw/CppunitTest_sw_uibase_uiview.mk index 5e73ce9c266f..14fbfd9b4c6f 100644 --- a/sw/CppunitTest_sw_uibase_uiview.mk +++ b/sw/CppunitTest_sw_uibase_uiview.mk @@ -63,7 +63,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 b249710715d7..d14088ce12f8 100644 --- a/sw/inc/view.hxx +++ b/sw/inc/view.hxx @@ -214,6 +214,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 880485487207..6b608e0ae309 100644 --- a/sw/qa/uibase/uiview/uiview.cxx +++ b/sw/qa/uibase/uiview/uiview.cxx @@ -13,6 +13,7 @@ #include <osl/file.hxx> #include <comphelper/propertyvalue.hxx> #include <comphelper/scopeguard.hxx> +#include <vcl/scheduler.hxx> #include <com/sun/star/frame/XDispatchHelper.hpp> #include <com/sun/star/frame/XDispatchProvider.hpp> @@ -20,11 +21,13 @@ #include <com/sun/star/frame/XStorable2.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 <swmodule.hxx> +#include <view.hxx> constexpr OUStringLiteral DATA_DIRECTORY = u"/sw/qa/uibase/uiview/data/"; @@ -144,6 +147,171 @@ 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: + SwDoc* pDoc = createSwDoc(); + 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)); + + xRegistration->registerDispatchProviderInterceptor(pInterceptor); + 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 3ecd3450ae6c..03085bcfc248 100644 --- a/sw/source/uibase/uiview/view.cxx +++ b/sw/source/uibase/uiview/view.cxx @@ -269,6 +269,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; @@ -279,6 +292,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 { @@ -730,6 +757,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),
