sw/source/core/doc/fmtcol.cxx | 54 ++++--------------------------------------
1 file changed, 6 insertions(+), 48 deletions(-)
New commits:
commit f8c68d3499512a7f559fa990ec8a1648e3a7e5d5
Author: Justin Luth <jl...@mail.com>
AuthorDate: Tue Jul 2 20:04:59 2024 -0400
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Fri Jul 19 17:08:36 2024 +0200
related tdf#134204 : notify change unless all properties are handled #2
This patch is the biggest logic change in this patchset:
- no notify if single property (!pNewChgSet) didn't cause a change
-bContinue can now be set false - avoiding a false notify
- multiple properties: don't block notify
-by doing this, I can remove my earlier fix.
------------------------------------------------
Here is my logic:
else if( pNewChgSet )
bContinue = false
simply was incorrect logic. There is no way we can say that
no notification is needed simply because one property is unchanged.
AT BEST (which will be done in a follow-up patch)
we will need to track how many bContinue = false's there are
and see if they match the number of properties that were passed.
But sufficient should be to only block when Count == 1,
like pOldChgSet now does.
bContinue = pNewChgSet->GetChgSet()->Count() > 1;
The variable pNewChgSet had been used earlier because it then
was checking if pNewChgSet->GetChgdSet...
But we can easily substitute pOldChgSet here, becoming
else if (pOldChgSet)
bContinue = pOldChgSet->GetChgSet()->Count() > 1;
There are three mutually exclusive code paths to consider:
pOldChgSet, bNewParent, or just a single property
Since bNewParent overrides bContinue elsewhere, ignore it
and it also doesn't make sense to continue for a single property,
so this could simply become
else
bContinue = pOld... && pOld...->GetChgSet()->Count() > 1;
which is identical to the bContinue in the if clause,
and thus they could be combined.
Change-Id: Ifa5ba56226f4e77a00f3bc8089a8dcaaca2b91ea
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169898
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
Tested-by: Jenkins
Reviewed-by: Justin Luth <jl...@mail.com>
diff --git a/sw/source/core/doc/fmtcol.cxx b/sw/source/core/doc/fmtcol.cxx
index 6f8edb6c10de..a43038059a71 100644
--- a/sw/source/core/doc/fmtcol.cxx
+++ b/sw/source/core/doc/fmtcol.cxx
@@ -268,11 +268,8 @@ void SwTextFormatColl::SwClientNotify(const SwModify&
rModify, const SfxHint& rH
if( bChg )
{
SetFormatAttr(aNew); // triggered separate notification about
only this one property
- bContinue = pOldChgSet && pOldChgSet->GetChgSet()->Count() >
1; // #3, #4
}
- // We set it to absolute -> do not propagate it further
- else if( pNewChgSet )
- bContinue = false;
+ bContinue = pOldChgSet && pOldChgSet->GetChgSet()->Count() > 1; //
#3, #4
}
}
const SvxTextLeftMarginItem
*pOldTextLeftMargin(GetItemIfSet(RES_MARGIN_TEXTLEFT, false));
@@ -294,11 +291,8 @@ void SwTextFormatColl::SwClientNotify(const SwModify&
rModify, const SfxHint& rH
if( bChg )
{
SetFormatAttr( aNew );
- bContinue = pOldChgSet && pOldChgSet->GetChgSet()->Count() > 1;
}
- // We set it to absolute -> do not propagate it further
- else if( pNewChgSet )
- bContinue = false;
+ bContinue = pOldChgSet && pOldChgSet->GetChgSet()->Count() > 1;
}
}
const SvxRightMarginItem *pOldRightMargin(GetItemIfSet(RES_MARGIN_RIGHT,
false));
@@ -318,11 +312,8 @@ void SwTextFormatColl::SwClientNotify(const SwModify&
rModify, const SfxHint& rH
if( bChg )
{
SetFormatAttr( aNew );
- bContinue = pOldChgSet && pOldChgSet->GetChgSet()->Count() > 1;
}
- // We set it to absolute -> do not propagate it further
- else if( pNewChgSet )
- bContinue = false;
+ bContinue = pOldChgSet && pOldChgSet->GetChgSet()->Count() > 1;
}
}
@@ -348,11 +339,8 @@ void SwTextFormatColl::SwClientNotify(const SwModify&
rModify, const SfxHint& rH
if( bChg )
{
SetFormatAttr( aNew );
- bContinue = pOldChgSet && pOldChgSet->GetChgSet()->Count() > 1;
}
- // We set it to absolute -> do not propagate it further
- else if( pNewChgSet )
- bContinue = false;
+ bContinue = pOldChgSet && pOldChgSet->GetChgSet()->Count() > 1;
}
for (size_t nC = 0; nC < SAL_N_ELEMENTS(aFontSizeArr); ++nC)
@@ -363,14 +351,7 @@ void SwTextFormatColl::SwClientNotify(const SwModify&
rModify, const SfxHint& rH
// Avoid recursion (SetAttr!)
!SfxPoolItem::areSame(pFSize, pOldFSize) )
{
- if( 100 == pOldFSize->GetProp() &&
- MapUnit::MapRelative == pOldFSize->GetPropUnit() )
- {
- // We set it to absolute -> do not propagate it further
- if( pNewChgSet )
- bContinue = false;
- }
- else
+ if (100 != pOldFSize->GetProp() || MapUnit::MapRelative !=
pOldFSize->GetPropUnit())
{
// We had a relative value -> recalculate
const sal_uInt32 nOld = pOldFSize->GetHeight();
@@ -380,12 +361,9 @@ void SwTextFormatColl::SwClientNotify(const SwModify&
rModify, const SfxHint& rH
if (nOld != aNew.GetHeight())
{
SetFormatAttr( aNew );
- bContinue = pOldChgSet && pOldChgSet->GetChgSet()->Count()
> 1;
}
- // We set it to absolute -> do not propagate it further
- else if( pNewChgSet )
- bContinue = false;
}
+ bContinue = pOldChgSet && pOldChgSet->GetChgSet()->Count() > 1;
}
}
@@ -393,26 +371,6 @@ void SwTextFormatColl::SwClientNotify(const SwModify&
rModify, const SfxHint& rH
if (!bContinue && bNewParent) // #4
bContinue = true;
- // If there are any attributes in addition to the special ones already
handled, then notify...
- if (!bContinue && pNewChgSet && pNewChgSet->GetChgSet())
- {
- SfxItemIter aIter(*pNewChgSet->GetChgSet());
- for (const SfxPoolItem* pItem = aIter.GetCurItem(); pItem; pItem =
aIter.NextItem())
- {
- const sal_uInt16 nWhich = pItem->Which();
- if (nWhich == RES_CHRATR_FONTSIZE || nWhich ==
RES_CHRATR_CTL_FONTSIZE
- || nWhich == RES_CHRATR_CJK_FONTSIZE || nWhich == RES_UL_SPACE
- || nWhich == RES_MARGIN_TEXTLEFT || nWhich == RES_MARGIN_RIGHT
- || nWhich == RES_MARGIN_FIRSTLINE)
- {
- continue; // already considered for bContinue
- }
-
- bContinue = true;
- break; // only one new condition needed to notify that this style
has changes
- }
- }
-
if( bContinue )
SwFormatColl::SwClientNotify(rModify, rHint);
}
