From 31cb5b5538b9fd91dafb067ce961f2540555ad2b Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Tue, 23 Aug 2022 11:58:29 +0200 Subject: 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 Tested-by: Jenkins --- sw/qa/uibase/uiview/uiview.cxx | 168 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 168 insertions(+) (limited to 'sw/qa/uibase/uiview') 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 #include #include +#include #include #include @@ -20,11 +21,13 @@ #include #include #include +#include #include #include #include #include +#include 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 +{ + uno::Reference m_xSelectionSupplier; + uno::Reference m_xMaster; + uno::Reference m_xSlave; + int m_nEnabled = 0; + int m_nDisabled = 0; + +public: + ImageInterceptor(const uno::Reference& xComponent); + + // XDispatchProviderInterceptor + uno::Reference SAL_CALL getMasterDispatchProvider() override; + uno::Reference SAL_CALL getSlaveDispatchProvider() override; + void SAL_CALL setMasterDispatchProvider( + const uno::Reference& xNewSupplier) override; + void SAL_CALL + setSlaveDispatchProvider(const uno::Reference& xNewSupplier) override; + + // XDispatchProvider + uno::Reference SAL_CALL queryDispatch(const util::URL& rURL, + const OUString& rTargetFrameName, + sal_Int32 SearchFlags) override; + uno::Sequence> SAL_CALL + queryDispatches(const uno::Sequence& rRequests) override; +}; +} + +ImageInterceptor::ImageInterceptor(const uno::Reference& xComponent) +{ + uno::Reference xModel(xComponent, uno::UNO_QUERY); + CPPUNIT_ASSERT(xModel.is()); + m_xSelectionSupplier.set(xModel->getCurrentController(), uno::UNO_QUERY); + CPPUNIT_ASSERT(m_xSelectionSupplier.is()); +} + +uno::Reference ImageInterceptor::getMasterDispatchProvider() +{ + return m_xMaster; +} + +uno::Reference ImageInterceptor::getSlaveDispatchProvider() +{ + return m_xSlave; +} + +void ImageInterceptor::setMasterDispatchProvider( + const uno::Reference& xNewSupplier) +{ + m_xMaster = xNewSupplier; +} + +void ImageInterceptor::setSlaveDispatchProvider( + const uno::Reference& xNewSupplier) +{ + m_xSlave = xNewSupplier; +} + +uno::Reference 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 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> +ImageInterceptor::queryDispatches(const uno::Sequence& /*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 xMSF(mxComponent, uno::UNO_QUERY); + uno::Reference xTextDocument(mxComponent, uno::UNO_QUERY); + uno::Reference xText = xTextDocument->getText(); + uno::Reference xCursor = xText->createTextCursor(); + for (int i = 0; i < 2; ++i) + { + uno::Reference 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 xTextContent(xTextGraphic, uno::UNO_QUERY); + xText->insertTextContent(xCursor, xTextContent, false); + } + pWrtShell->SttEndDoc(/*bStt=*/false); + uno::Reference xModel(mxComponent, uno::UNO_QUERY); + uno::Reference 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 xGraphicObjectsSupplier(mxComponent, + uno::UNO_QUERY); + uno::Reference 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: */ -- cgit