chart2/source/view/main/ChartView.cxx | 8 ++-- include/svx/svdmodel.hxx | 10 +++++ sc/source/core/data/drwlayer.cxx | 3 + sd/qa/uitest/impress_tests2/tdf170386.py | 54 +++++++++++++++++++++++++++++++ svx/source/svdraw/svdmodel.cxx | 28 ++++++++++++++++ svx/source/svdraw/svdobj.cxx | 9 ++++- 6 files changed, 108 insertions(+), 4 deletions(-)
New commits: commit 29ccd4ca89316c0cf6c8acd98a5057fe12a37e50 Author: Mike Kaganski <[email protected]> AuthorDate: Tue Jan 27 13:07:22 2026 +0500 Commit: Mike Kaganski <[email protected]> CommitDate: Wed Jan 28 14:52:34 2026 +0500 tdf#170386: Delay broadcasting object changes, not drop it Creating a SvxShapeText (and inserting it into the model) will call SetEditSource with a newly created SvxTextEditSource; and that one will register its impl as a listener to underlying SdrObject. Saving the document, the SvxTextEditSource will be queried for the properties of the text, when generating autostyles. Therefore, it is important that the properties known to SvxTextEditSource match the properties applied to the SdrObject. The problem was, that the notifications to the listeners were not sent, when the model was locked; and when it was unlocked, there was no mechanism to re-send notifications at this time. This meant that SvxTextEditSource was unaware of e.g. weight changes applied to SdrObject inside lockControllers ... unlockControllers pair. This change introduces a list where objects can register for a postponed change notification. The unit test is implemented as UITest; trying to do that in a CppunitTest failed (the test succeeded even without the fix, maybe because the CppunitTest has no UI, and so the mechanics of using SvxTextEditSource differ). There already was a case that relied on dropping the notifications, specifically ScDrawLayer::SetPageSize, which can move / resize objects as a result of underlying page size change (like hiding columns), but doesn't want the objects to update their anchoring information. That was addressed by adding a flag to ignore adding deferred notifications. Change-Id: I3aac61bf91a2461b024b26ad7e7c199cc7def37b Reviewed-on: https://gerrit.libreoffice.org/c/core/+/198187 Reviewed-by: Noel Grandin <[email protected]> Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> diff --git a/chart2/source/view/main/ChartView.cxx b/chart2/source/view/main/ChartView.cxx index bed80f49b326..04dbdf4ec14e 100644 --- a/chart2/source/view/main/ChartView.cxx +++ b/chart2/source/view/main/ChartView.cxx @@ -1458,6 +1458,11 @@ void ChartView::impl_updateView( bool bCheckLockedCtrler ) return; m_bInViewUpdate = true; + + // Rendering the chart must not set its (or its parent) modified status. + // Note that unlockControllers() may send notifications that set modified state. + ChartModelDisableSetModified dontSetModified(mrChartModel); + //bool bOldRefreshAddIn = m_bRefreshAddIn; //m_bRefreshAddIn = false; try @@ -1470,9 +1475,6 @@ void ChartView::impl_updateView( bool bCheckLockedCtrler ) m_pDrawModelWrapper->lockControllers(); } - // Rendering the chart must not set its (or its parent) modified status - ChartModelDisableSetModified dontSetModified(mrChartModel); - //create chart view { m_bViewDirty = false; diff --git a/include/svx/svdmodel.hxx b/include/svx/svdmodel.hxx index 81ec0927db4c..ae90febcda91 100644 --- a/include/svx/svdmodel.hxx +++ b/include/svx/svdmodel.hxx @@ -252,6 +252,16 @@ public: tools::Long nUpper = 0, tools::Long nLower = 0); + // When this model is locked (mbModelLocked), and an SdrObject wanted to BroadcastObjectChange, + // it adds itself to the deferred change list instead, to batch-broadcast when unlocked. + void addDeferredObjectChanges(const SdrObject* pObject); + + // When this flag is set, ignore AddDeferredObjectChanges calls. Used as a hack in special + // cases, where the broadcasts are not wanted at all (e.g., ScDrawLayer::SetPageSize, which + // updates the objects, but doesn't want related anchor point recalculations). + void setIgnoreDeferredObjectChanges(bool bIgnore); + bool isIgnoringDeferredObjectChanges() const; + protected: void implDtorClearModel(); virtual css::uno::Reference< css::frame::XModel > createUnoModel(); diff --git a/sc/source/core/data/drwlayer.cxx b/sc/source/core/data/drwlayer.cxx index 3608b47ef412..ecfe47f7150f 100644 --- a/sc/source/core/data/drwlayer.cxx +++ b/sc/source/core/data/drwlayer.cxx @@ -669,6 +669,8 @@ void ScDrawLayer::SetPageSize(sal_uInt16 nPageNo, const Size& rSize, bool bUpdat // Disable mass broadcasts from drawing objects' position changes. bool bWasLocked = isLocked(); setLock(true); + bool bWasIgnoring = isIgnoringDeferredObjectChanges(); + setIgnoreDeferredObjectChanges(true); for (const rtl::Reference<SdrObject>& pObj : *pPage) { @@ -710,6 +712,7 @@ void ScDrawLayer::SetPageSize(sal_uInt16 nPageNo, const Size& rSize, bool bUpdat } } + setIgnoreDeferredObjectChanges(bWasIgnoring); setLock(bWasLocked); } diff --git a/sd/qa/uitest/impress_tests2/tdf170386.py b/sd/qa/uitest/impress_tests2/tdf170386.py new file mode 100644 index 000000000000..6fe3d0e56aee --- /dev/null +++ b/sd/qa/uitest/impress_tests2/tdf170386.py @@ -0,0 +1,54 @@ +# -*- tab-width: 4; indent-tabs-mode: nil; py-indent-offset: 4 -*- +# +# This file is part of the LibreOffice project. +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# + +from uitest.framework import UITestCase +from com.sun.star.awt import Point +import pathlib +import tempfile + +class tdf170386(UITestCase): + def test_tdf170386(self): + # Create a Draw document, lock its controllers, add a text shape, and apply bold to it. + # Unlock the controllers, save and reload. The expectation is, that the applied bold + # remains applied to the text. + # There are two levels where bold is applied to the shape and its text: + # 1. The shape itself (draw:frame element) has a paragraph autostyle applied through + # draw:text-style-name attribute, and that defined the font weight; + # 2. The paragraphs of text (text:p elements) may apply their own paragraph autoformats. + # It is a question, why the draw:frame's draw:text-style-name apparently is not applied + # to the paragraphs inside the shape, when the paragraphs don't define their own style; + # but the bug was, that the paragraphs missed their autostyles, and so the text runs were + # not bold (even though the shape-level property was present). It happened because the + # object used to collect text properties on save wasn't notified on the changes that + # happened when the model was locked. + + with tempfile.NamedTemporaryFile(suffix=".odg", delete_on_close=False) as temp: + temp.close() + url = pathlib.Path(temp.name).as_uri() + + with self.ui_test.create_doc_in_start_center("draw") as xModel: + xPage = xModel.getDrawPages().getByIndex(0) + xModel.lockControllers() + xShape = xModel.createInstance("com.sun.star.drawing.TextShape") + xPage.add(xShape) + xShape.setString("Some Text") + xShape.setPosition(Point(2000, 600)) + xShape.setPropertyValue("CharWeight", 150.0) + xModel.unlockControllers() + xModel.storeAsURL(url, []) + + with self.ui_test.load_file(url) as xModel: + xShape = xModel.getDrawPages().getByIndex(0).getByIndex(0) + # The shape-level property was OK even without the fix: + self.assertEqual(150.0, xShape.getPropertyValue("CharWeight")) + # Test that the text inside the shape (accessed using cursor) has correct weight. + # Without the fix, this failed with "150.0 != 100.0" + self.assertEqual(150.0, xShape.createTextCursor().getPropertyValue("CharWeight")) + +# vim: set shiftwidth=4 softtabstop=4 expandtab: diff --git a/svx/source/svdraw/svdmodel.cxx b/svx/source/svdraw/svdmodel.cxx index 90fc769e20b7..ab14085ee337 100644 --- a/svx/source/svdraw/svdmodel.cxx +++ b/svx/source/svdraw/svdmodel.cxx @@ -92,6 +92,10 @@ struct SdrModelImpl std::shared_ptr<model::Theme> mpTheme; std::shared_ptr<svx::IThemeColorChanger> mpThemeColorChanger; + // A set of SdrObjects that want to BroadcastObjectChange when this is unlocked. + std::unordered_set<rtl::Reference<SdrObject>> maDeferredChanges; + bool mbIgnoreDeferredChanges = false; + SdrModelImpl() : mpUndoManager(nullptr) , mpUndoFactory(nullptr) @@ -1619,11 +1623,35 @@ void SdrModel::setLock( bool bLock ) if( !bLock ) { + // Catch unbalanced calls to SetIgnoreDeferredObjectChanges + assert(!mpImpl->mbIgnoreDeferredChanges); + for (auto& pObject : std::exchange(mpImpl->maDeferredChanges, {})) + pObject->BroadcastObjectChange(); + ImpReformatAllEdgeObjects(); } } } +void SdrModel::addDeferredObjectChanges(const SdrObject* pObject) +{ + assert(mbModelLocked); // Only makes sense when model is locked + assert(pObject); + if (!mpImpl->mbIgnoreDeferredChanges) + mpImpl->maDeferredChanges.insert(const_cast<SdrObject*>(pObject)); +} + +void SdrModel::setIgnoreDeferredObjectChanges(bool bIgnore) +{ + assert(mbModelLocked); // Only makes sense when model is locked + mpImpl->mbIgnoreDeferredChanges = bIgnore; +} + +bool SdrModel::isIgnoringDeferredObjectChanges() const +{ + assert(mbModelLocked); // Only makes sense when model is locked + return mpImpl->mbIgnoreDeferredChanges; +} void SdrModel::MigrateItemSet( const SfxItemSet* pSourceSet, SfxItemSet* pDestSet, SdrModel* pNewModelel ) { diff --git a/svx/source/svdraw/svdobj.cxx b/svx/source/svdraw/svdobj.cxx index 3c9bba0ded93..0114e3ce7800 100644 --- a/svx/source/svdraw/svdobj.cxx +++ b/svx/source/svdraw/svdobj.cxx @@ -1010,7 +1010,7 @@ void SdrObject::RecalcBoundRect() void SdrObject::BroadcastObjectChange() const { - if ((getSdrModelFromSdrObject().isLocked()) || utl::ConfigManager::IsFuzzing()) + if (utl::ConfigManager::IsFuzzing()) return; bool bPlusDataBroadcast(m_pPlusData && m_pPlusData->pBroadcast); @@ -1019,6 +1019,13 @@ void SdrObject::BroadcastObjectChange() const if(!(bPlusDataBroadcast || bObjectChange)) return; + // Only check if we want to defer, after we decided that we really need to broadcast something + if (getSdrModelFromSdrObject().isLocked()) + { + getSdrModelFromSdrObject().addDeferredObjectChanges(this); + return; + } + SdrHint aHint(SdrHintKind::ObjectChange, *this); if(bPlusDataBroadcast)
