sw/qa/extras/uiwriter/uiwriter2.cxx | 8 ++++++++
sw/source/core/crsr/bookmark.cxx | 19 ++++++++++++-------
sw/source/core/doc/docbm.cxx | 8 +++-----
sw/source/core/txtnode/modeltoviewhelper.cxx | 2 +-
sw/source/core/undo/rolbck.cxx | 8 ++++----
5 files changed, 28 insertions(+), 17 deletions(-)
New commits:
commit e19044a21287b6538ae5962c5fb29b711c09e4f9
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Tue Feb 1 13:39:19 2022 +0100
Commit: Xisco Fauli <xiscofa...@libreoffice.org>
CommitDate: Wed Feb 2 12:06:00 2022 +0100
tdf#147008 sw_fieldmarkhide: fix invalid NonTextFieldmark positions
Commit ab6176e88f78d0b3aa2490fbc7858304c2d4a437 introduced a crash
in ModelToViewHelper when the positions of a NonTextFieldmark are invalid.
The NonTextFieldmark must always contain 1 CH_TXT_ATR_FORMELEMENT
but after SplitNode() the position is
(rr) p *pFieldMark->m_pPos1
$2 = SwPosition (node 10, offset 1)
(rr) p *pFieldMark->m_pPos2
$3 = SwPosition (node 9, offset 0)
This is because in ContentIdxStoreImpl::SaveBkmks() there is an
asymmetry where the m_pPos2 is recorded to be wrongly corrected to node
9, but if the positions were swapped so that m_pPos1 is the start
position, then it will not be recorded and remain in node 10.
So fix this by changing the NonTextFieldmark to insert its
CH_TXT_ATR_FORMELEMENT differently.
There is some very subtle code in SwTextNode::Update() that is again
asymmetric and (non-obviously) prefers to move m_pPos2 and leave m_pPos1
alone (by moving it to aTmpIdxReg) in case the positions are equal.
But then the fieldmark code increments "rEnd" (which is really the
m_pPos1 i.e. the start after InsertString() returns), and then
decrements m_pPos2.
So avoid the problem by removing these 2 pointless adjustments.
Then it turns a bunch of tests fail because other code assumes that
m_pPos1 is the end of the NonTextFieldmark, so fix
MarkManager::changeFormFieldmarkType(), ModelToViewHelper and
SwHistoryNoTextFieldmark to use GetMarkStart().
Change-Id: I7c82f9a67661121662c95727e0f8f15e06d85a3a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/129289
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.st...@allotropia.de>
(cherry picked from commit ea06852ee87531794f07710de496734a647a9062)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/129265
Reviewed-by: Xisco Fauli <xiscofa...@libreoffice.org>
diff --git a/sw/qa/extras/uiwriter/uiwriter2.cxx
b/sw/qa/extras/uiwriter/uiwriter2.cxx
index 79aa517c66f4..65142dbd64e1 100644
--- a/sw/qa/extras/uiwriter/uiwriter2.cxx
+++ b/sw/qa/extras/uiwriter/uiwriter2.cxx
@@ -3600,6 +3600,14 @@ CPPUNIT_TEST_FIXTURE(SwUiWriterTest2,
testCheckboxFormFieldInsertion)
pFieldmark = dynamic_cast<::sw::mark::IFieldmark*>(*aIter);
CPPUNIT_ASSERT(pFieldmark);
CPPUNIT_ASSERT_EQUAL(OUString(ODF_FORMCHECKBOX),
pFieldmark->GetFieldname());
+
+ // tdf#147008 this would crash
+ SwWrtShell* pWrtShell = pDoc->GetDocShell()->GetWrtShell();
+ pWrtShell->StartOfSection(false);
+ pWrtShell->SplitNode();
+ CPPUNIT_ASSERT_EQUAL(pFieldmark->GetMarkPos().nNode,
pFieldmark->GetOtherMarkPos().nNode);
+
CPPUNIT_ASSERT_EQUAL(sal_Int32(pFieldmark->GetMarkPos().nContent.GetIndex() +
1),
+ pFieldmark->GetOtherMarkPos().nContent.GetIndex());
}
CPPUNIT_TEST_FIXTURE(SwUiWriterTest2, testDropDownFormFieldInsertion)
diff --git a/sw/source/core/crsr/bookmark.cxx b/sw/source/core/crsr/bookmark.cxx
index 2945b018bb8b..417558aad130 100644
--- a/sw/source/core/crsr/bookmark.cxx
+++ b/sw/source/core/crsr/bookmark.cxx
@@ -154,6 +154,10 @@ namespace
SwPosition const sepPos(sw::mark::FindFieldSep(rField));
assert(sepPos.nNode.GetNode().GetTextNode()->GetText()[sepPos.nContent.GetIndex()]
== CH_TXT_ATR_FIELDSEP); (void) sepPos;
}
+ else
+ { // must be m_pPos1 < m_pPos2 because of asymmetric SplitNode update
+ assert(rField.GetMarkPos().nContent.GetIndex() + 1 ==
rField.GetOtherMarkPos().nContent.GetIndex());
+ }
SwPosition const& rEnd(rField.GetMarkEnd());
assert(rEnd.nNode.GetNode().GetTextNode()->GetText()[rEnd.nContent.GetIndex() -
1] == aEndMark); (void) rEnd;
}
@@ -207,7 +211,14 @@ namespace
{
SwPaM aEndPaM(rEnd);
io_rDoc.getIDocumentContentOperations().InsertString(aEndPaM,
OUString(aEndMark));
- ++rEnd.nContent;
+ if (aEndMark != CH_TXT_ATR_FORMELEMENT)
+ {
+ ++rEnd.nContent; // InsertString didn't move non-empty mark
+ }
+ else
+ { // InsertString moved the mark's end, not its start
+ assert(rField.GetMarkPos().nContent.GetIndex() + 1 ==
rField.GetOtherMarkPos().nContent.GetIndex());
+ }
}
lcl_AssertFieldMarksSet(rField, aStartMark, aEndMark);
@@ -590,12 +601,6 @@ namespace sw::mark
if (eMode == sw::mark::InsertMode::New)
{
lcl_SetFieldMarks(*this, io_rDoc, CH_TXT_ATR_FIELDSTART,
CH_TXT_ATR_FORMELEMENT, pSepPos);
-
- // For some reason the end mark is moved from 1 by the Insert:
- // we don't want this for checkboxes
- SwPosition aNewEndPos = GetMarkEnd();
- aNewEndPos.nContent--;
- SetMarkEndPos( aNewEndPos );
}
else
{
diff --git a/sw/source/core/doc/docbm.cxx b/sw/source/core/doc/docbm.cxx
index 5ed676719622..c15c1dd3fcea 100644
--- a/sw/source/core/doc/docbm.cxx
+++ b/sw/source/core/doc/docbm.cxx
@@ -1419,8 +1419,7 @@ namespace sw::mark
void MarkManager::deleteFieldmarkAt(const SwPosition& rPos)
{
auto const pFieldmark = dynamic_cast<Fieldmark*>(getFieldmarkAt(rPos));
- if (!pFieldmark)
- return;
+ assert(pFieldmark); // currently all callers require it to be there
deleteMark(lcl_FindMark(m_vAllMarks, pFieldmark));
}
@@ -1455,12 +1454,11 @@ namespace sw::mark
// Store attributes needed to create the new fieldmark
OUString sName = pFieldmark->GetName();
- SwPaM aPaM(pFieldmark->GetMarkPos());
+ SwPaM const aPaM(pFieldmark->GetMarkStart());
// Remove the old fieldmark and create a new one with the new type
- if(aPaM.GetPoint()->nContent > 0 && (rNewType == ODF_FORMDROPDOWN ||
rNewType == ODF_FORMCHECKBOX))
+ if (rNewType == ODF_FORMDROPDOWN || rNewType == ODF_FORMCHECKBOX)
{
- --aPaM.GetPoint()->nContent;
SwPosition aNewPos (aPaM.GetPoint()->nNode,
aPaM.GetPoint()->nContent);
deleteFieldmarkAt(aNewPos);
return makeNoTextFieldBookmark(aPaM, sName, rNewType);
diff --git a/sw/source/core/txtnode/modeltoviewhelper.cxx
b/sw/source/core/txtnode/modeltoviewhelper.cxx
index 846e4d1a51b1..1f35cd1ee614 100644
--- a/sw/source/core/txtnode/modeltoviewhelper.cxx
+++ b/sw/source/core/txtnode/modeltoviewhelper.cxx
@@ -286,7 +286,7 @@ ModelToViewHelper::ModelToViewHelper(const SwTextNode
&rNode,
for (sw::mark::IFieldmark *const pMark : aNoTextFieldmarks)
{
- const sal_Int32 nDummyCharPos =
pMark->GetMarkPos().nContent.GetIndex()-1;
+ const sal_Int32 nDummyCharPos =
pMark->GetMarkStart().nContent.GetIndex();
if (aHiddenMulti.IsSelected(nDummyCharPos))
continue;
std::vector<block>::iterator aFind =
std::find_if(aBlocks.begin(), aBlocks.end(),
diff --git a/sw/source/core/undo/rolbck.cxx b/sw/source/core/undo/rolbck.cxx
index 85f92be47670..49c4d9b1d946 100644
--- a/sw/source/core/undo/rolbck.cxx
+++ b/sw/source/core/undo/rolbck.cxx
@@ -729,8 +729,8 @@ bool SwHistoryBookmark::IsEqualBookmark(const
::sw::mark::IMark& rBkmk)
SwHistoryNoTextFieldmark::SwHistoryNoTextFieldmark(const
::sw::mark::IFieldmark& rFieldMark)
: SwHistoryHint(HSTRY_NOTEXTFIELDMARK)
, m_sType(rFieldMark.GetFieldname())
- , m_nNode(rFieldMark.GetMarkPos().nNode.GetIndex())
- , m_nContent(rFieldMark.GetMarkPos().nContent.GetIndex())
+ , m_nNode(rFieldMark.GetMarkStart().nNode.GetIndex())
+ , m_nContent(rFieldMark.GetMarkStart().nContent.GetIndex())
{
}
@@ -760,8 +760,8 @@ void SwHistoryNoTextFieldmark::ResetInDoc(SwDoc& rDoc)
std::optional<SwPaM> pPam;
const SwContentNode* pContentNd = rNds[m_nNode]->GetContentNode();
- if(pContentNd)
- pPam.emplace(*pContentNd, m_nContent-1);
+ assert(pContentNd);
+ pPam.emplace(*pContentNd, m_nContent);
if (pPam)
{
