[Libreoffice-commits] core.git: 5 commits - sw/qa sw/source
Michael Stahl
mstahl at redhat.com
Wed Sep 28 09:12:07 UTC 2016
sw/qa/extras/ooxmlexport/ooxmlfieldexport.cxx | 2
sw/source/core/crsr/bookmrk.cxx | 39 +++++---------
sw/source/core/doc/docbm.cxx | 70 ++++++++++++--------------
3 files changed, 50 insertions(+), 61 deletions(-)
New commits:
commit bb069fe7b8b6a24f9ff4df4c7052961e17ea3a8c
Author: Michael Stahl <mstahl at redhat.com>
Date: Wed Sep 28 10:41:07 2016 +0200
sw: remove defensive programming bullshit in lcl_AssureFieldMarksSet
In CppunitTest_sw_ooxmlfieldexport testFdo81492 a TextFieldmark is
inserted at a position where already another TextFieldmark starts. The
defensively programmed lcl_AssureFieldMarksSet notices there is aleady a
dummy character at the start position, and does not insert another one,
but then the dummy character for the end position is inserted, moving
the start position index, which puts the start position behind another
bookmark.
So we end up with a field mark that has a end character but not a start
character and an un-sorted m_vAllMarks.
Change-Id: Icd15e83471e18f607eb41b2f7b0c2ce61c94ff9f
diff --git a/sw/qa/extras/ooxmlexport/ooxmlfieldexport.cxx b/sw/qa/extras/ooxmlexport/ooxmlfieldexport.cxx
index 9af064c..39a2274 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlfieldexport.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlfieldexport.cxx
@@ -565,7 +565,7 @@ DECLARE_OOXMLEXPORT_TEST(testSdtDateDuplicate, "sdt-date-duplicate.docx")
DECLARE_OOXMLEXPORT_TEST(testFdo81492, "fdo81492.docx")
{
if (xmlDocPtr pXmlDoc = parseExport())
- assertXPathContent(pXmlDoc, "/w:document/w:body/w:p[1]/w:r[5]/w:instrText", "ADDIN EN.CITE.DATA");
+ assertXPathContent(pXmlDoc, "/w:document/w:body/w:p[1]/w:r[9]/w:instrText", "ADDIN EN.CITE.DATA");
}
DECLARE_OOXMLEXPORT_TEST(testEditTime, "fdo81341.docx")
diff --git a/sw/source/core/crsr/bookmrk.cxx b/sw/source/core/crsr/bookmrk.cxx
index b99edeb..936e48f 100644
--- a/sw/source/core/crsr/bookmrk.cxx
+++ b/sw/source/core/crsr/bookmrk.cxx
@@ -73,11 +73,7 @@ namespace
io_pDoc->GetIDocumentUndoRedo().StartUndo(UNDO_UI_REPLACE, nullptr);
SwPosition start = pField->GetMarkStart();
- SwTextNode const*const pStartTextNode = start.nNode.GetNode().GetTextNode();
- sal_Unicode ch_start = 0;
- if (pStartTextNode && (start.nContent.GetIndex() < pStartTextNode->GetText().getLength()))
- ch_start = pStartTextNode->GetText()[start.nContent.GetIndex()];
- if( ( ch_start != aStartMark ) && ( aEndMark != CH_TXT_ATR_FORMELEMENT ) )
+ if (aEndMark != CH_TXT_ATR_FORMELEMENT)
{
SwPaM aStartPaM(start);
io_pDoc->getIDocumentContentOperations().InsertString(aStartPaM, OUString(aStartMark));
@@ -88,14 +84,7 @@ namespace
}
SwPosition& rEnd = pField->GetMarkEnd();
- SwTextNode const*const pEndTextNode = rEnd.nNode.GetNode().GetTextNode();
- const sal_Int32 nEndPos = (rEnd == start || rEnd.nContent.GetIndex() == 0)
- ? rEnd.nContent.GetIndex()
- : rEnd.nContent.GetIndex() - 1;
- sal_Unicode ch_end = 0;
- if ( pEndTextNode && ( nEndPos < pEndTextNode->GetText().getLength() ) )
- ch_end = pEndTextNode->GetText()[nEndPos];
- if ( aEndMark && ( ch_end != aEndMark ) )
+ if (aEndMark)
{
SwPaM aEndPaM(rEnd);
io_pDoc->getIDocumentContentOperations().InsertString(aEndPaM, OUString(aEndMark));
@@ -118,8 +107,9 @@ namespace
if( pStartTextNode )
ch_start = pStartTextNode->GetText()[rStart.nContent.GetIndex()];
- if( ch_start == aStartMark )
+ if (aEndMark != CH_TXT_ATR_FORMELEMENT)
{
+ assert(ch_start == aStartMark);
SwPaM aStart(rStart, rStart);
++aStart.End()->nContent;
io_pDoc->getIDocumentContentOperations().DeleteRange(aStart);
@@ -133,13 +123,11 @@ namespace
sal_Unicode ch_end = 0;
if ( pEndTextNode )
ch_end = pEndTextNode->GetText()[nEndPos];
- if ( ch_end == aEndMark )
- {
- SwPaM aEnd(rEnd, rEnd);
- if (aEnd.Start()->nContent > 0)
- --aEnd.Start()->nContent;
- io_pDoc->getIDocumentContentOperations().DeleteRange(aEnd);
- }
+ assert(ch_end == aEndMark);
+ SwPaM aEnd(rEnd, rEnd);
+ if (aEnd.Start()->nContent > 0)
+ --aEnd.Start()->nContent;
+ io_pDoc->getIDocumentContentOperations().DeleteRange(aEnd);
io_pDoc->GetIDocumentUndoRedo().EndUndo(UNDO_UI_REPLACE, nullptr);
};
commit 2bd8be10e231314e757f29a37177f529e6f1df47
Author: Michael Stahl <mstahl at redhat.com>
Date: Tue Sep 27 23:49:04 2016 +0200
sw: rStart variable isn't a reference
Change-Id: I48c402e3dcd9606d7f40ee2096fa8803d5499d73
diff --git a/sw/source/core/crsr/bookmrk.cxx b/sw/source/core/crsr/bookmrk.cxx
index 26ec9f9..b99edeb 100644
--- a/sw/source/core/crsr/bookmrk.cxx
+++ b/sw/source/core/crsr/bookmrk.cxx
@@ -72,23 +72,26 @@ namespace
{
io_pDoc->GetIDocumentUndoRedo().StartUndo(UNDO_UI_REPLACE, nullptr);
- SwPosition rStart = pField->GetMarkStart();
- SwTextNode const*const pStartTextNode = rStart.nNode.GetNode().GetTextNode();
+ SwPosition start = pField->GetMarkStart();
+ SwTextNode const*const pStartTextNode = start.nNode.GetNode().GetTextNode();
sal_Unicode ch_start = 0;
- if( pStartTextNode && ( rStart.nContent.GetIndex() < pStartTextNode->GetText().getLength() ) )
- ch_start = pStartTextNode->GetText()[rStart.nContent.GetIndex()];
+ if (pStartTextNode && (start.nContent.GetIndex() < pStartTextNode->GetText().getLength()))
+ ch_start = pStartTextNode->GetText()[start.nContent.GetIndex()];
if( ( ch_start != aStartMark ) && ( aEndMark != CH_TXT_ATR_FORMELEMENT ) )
{
- SwPaM aStartPaM(rStart);
+ SwPaM aStartPaM(start);
io_pDoc->getIDocumentContentOperations().InsertString(aStartPaM, OUString(aStartMark));
- --rStart.nContent;
- pField->SetMarkStartPos( rStart );
+ --start.nContent; // restore, it was moved by InsertString
+ // do not manipulate via reference directly but call SetMarkStartPos
+ // which works even if start and end pos were the same
+ pField->SetMarkStartPos( start );
}
SwPosition& rEnd = pField->GetMarkEnd();
SwTextNode const*const pEndTextNode = rEnd.nNode.GetNode().GetTextNode();
- const sal_Int32 nEndPos = ( rEnd == rStart || rEnd.nContent.GetIndex() == 0 ) ?
- rEnd.nContent.GetIndex() : rEnd.nContent.GetIndex() - 1;
+ const sal_Int32 nEndPos = (rEnd == start || rEnd.nContent.GetIndex() == 0)
+ ? rEnd.nContent.GetIndex()
+ : rEnd.nContent.GetIndex() - 1;
sal_Unicode ch_end = 0;
if ( pEndTextNode && ( nEndPos < pEndTextNode->GetText().getLength() ) )
ch_end = pEndTextNode->GetText()[nEndPos];
commit bcc3509cdbffef6a3f371434161efbfc4fbd8e5c
Author: Michael Stahl <mstahl at redhat.com>
Date: Tue Sep 27 15:16:49 2016 +0200
sw: make the debug code in docbm.cxx usable
Change-Id: I45e412e107601947221d679992356b47d29ad4a7
diff --git a/sw/source/core/doc/docbm.cxx b/sw/source/core/doc/docbm.cxx
index 6a10076..d8bebf4 100644
--- a/sw/source/core/doc/docbm.cxx
+++ b/sw/source/core/doc/docbm.cxx
@@ -270,28 +270,28 @@ namespace
[&rName] (IDocumentMarkAccess::pMark_t const& rpMark) { return rpMark->GetName() == rName; } );
}
-#if 0
static void lcl_DebugMarks(IDocumentMarkAccess::container_t vMarks)
{
- OSL_TRACE("%d Marks", vMarks.size());
+#if OSL_DEBUG_LEVEL > 0
+ SAL_INFO("sw.core", vMarks.size() << " Marks");
for(IDocumentMarkAccess::iterator_t ppMark = vMarks.begin();
ppMark != vMarks.end();
ppMark++)
{
IMark* pMark = ppMark->get();
- OString sName = OUStringToOString(pMark->GetName(), RTL_TEXTENCODING_UTF8);
const SwPosition* const pStPos = &pMark->GetMarkStart();
const SwPosition* const pEndPos = &pMark->GetMarkEnd();
- OSL_TRACE("%s %s %d,%d %d,%d",
- typeid(*pMark).name(),
- sName.getStr(),
- pStPos->nNode.GetIndex(),
- pStPos->nContent.GetIndex(),
- pEndPos->nNode.GetIndex(),
+ SAL_INFO("sw.core",
+ typeid(*pMark).name() << " " <<
+ pMark->GetName() << " " <<
+ pStPos->nNode.GetIndex() << "," <<
+ pStPos->nContent.GetIndex() << " " <<
+ pEndPos->nNode.GetIndex() << "," <<
pEndPos->nContent.GetIndex());
}
- };
#endif
+ assert(std::is_sorted(vMarks.begin(), vMarks.end(), lcl_MarkOrderingByStart));
+ };
}
IDocumentMarkAccess::MarkType IDocumentMarkAccess::GetType(const IMark& rBkmk)
@@ -352,18 +352,17 @@ namespace sw { namespace mark
const OUString& rName,
const IDocumentMarkAccess::MarkType eType)
{
-#if 0
+#if OSL_DEBUG_LEVEL > 0
{
- OString sName = OUStringToOString(rName, RTL_TEXTENCODING_UTF8);
const SwPosition* const pPos1 = rPaM.GetPoint();
const SwPosition* pPos2 = pPos1;
if(rPaM.HasMark())
pPos2 = rPaM.GetMark();
- OSL_TRACE("%s %d,%d %d,%d",
- sName.getStr(),
- pPos1->nNode.GetIndex(),
- pPos1->nContent.GetIndex(),
- pPos2->nNode.GetIndex(),
+ SAL_INFO("sw.core",
+ rName << " " <<
+ pPos1->nNode.GetIndex() << "," <<
+ pPos1->nContent.GetIndex() << " " <<
+ pPos2->nNode.GetIndex() << "," <<
pPos2->nContent.GetIndex());
}
#endif
@@ -451,15 +450,13 @@ namespace sw { namespace mark
break;
}
pMarkBase->InitDoc(m_pDoc);
-#if 0
- OSL_TRACE("--- makeType ---");
- OSL_TRACE("Marks");
+ SAL_INFO("sw.core", "--- makeType ---");
+ SAL_INFO("sw.core", "Marks");
lcl_DebugMarks(m_vAllMarks);
- OSL_TRACE("Bookmarks");
+ SAL_INFO("sw.core", "Bookmarks");
lcl_DebugMarks(m_vBookmarks);
- OSL_TRACE("Fieldmarks");
+ SAL_INFO("sw.core", "Fieldmarks");
lcl_DebugMarks(m_vFieldmarks);
-#endif
return pMark.get();
}
@@ -571,6 +568,9 @@ namespace sw { namespace mark
const SwPosition& rNewPos,
const sal_Int32 nOffset)
{
+ SAL_INFO("sw.core", "correctMarksAbsolute entry");
+ lcl_DebugMarks(m_vAllMarks);
+
const SwNode* const pOldNode = &rOldNode.GetNode();
SwPosition aNewPos(rNewPos);
aNewPos.nContent += nOffset;
@@ -607,10 +607,9 @@ namespace sw { namespace mark
// restore sorting if needed
if(isSortingNeeded)
sortMarks();
-#if 0
- OSL_TRACE("correctMarksAbsolute");
+
+ SAL_INFO("sw.core", "correctMarksAbsolute");
lcl_DebugMarks(m_vAllMarks);
-#endif
}
void MarkManager::correctMarksRelative(const SwNodeIndex& rOldNode, const SwPosition& rNewPos, const sal_Int32 nOffset)
@@ -657,10 +656,9 @@ namespace sw { namespace mark
// restore sorting if needed
if(isSortingNeeded)
sortMarks();
-#if 0
- OSL_TRACE("correctMarksRelative");
+
+ SAL_INFO("sw.core", "correctMarksRelative");
lcl_DebugMarks(m_vAllMarks);
-#endif
}
void MarkManager::deleteMarks(
@@ -832,10 +830,8 @@ namespace sw { namespace mark
sortMarks();
}
-#if 0
- OSL_TRACE("deleteMarks");
+ SAL_INFO("sw.core", "deleteMarks");
lcl_DebugMarks(m_vAllMarks);
-#endif
}
struct LazyFieldmarkDeleter : public IDocumentMarkAccess::ILazyDeleter
commit 39ac3aed06c65184e4c387234d045aee8f684e04
Author: Michael Stahl <mstahl at redhat.com>
Date: Tue Sep 27 14:55:02 2016 +0200
sw: assert that IMarks are MarkBase
Change-Id: Id9e61dc624a34012dc7a53e262c76f8d25c4a455
diff --git a/sw/source/core/doc/docbm.cxx b/sw/source/core/doc/docbm.cxx
index 0ecc743..6a10076 100644
--- a/sw/source/core/doc/docbm.cxx
+++ b/sw/source/core/doc/docbm.cxx
@@ -581,8 +581,8 @@ namespace sw { namespace mark
++ppMark)
{
::sw::mark::MarkBase* pMark = dynamic_cast< ::sw::mark::MarkBase* >(ppMark->get());
- if (!pMark)
- continue;
+ // correction of non-existent non-MarkBase instances cannot be done
+ assert(pMark);
// is on position ??
bool bChangedPos = false;
if(&pMark->GetMarkPos().nNode.GetNode() == pOldNode)
@@ -627,8 +627,8 @@ namespace sw { namespace mark
// is on position ??
bool bChangedPos = false, bChangedOPos = false;
::sw::mark::MarkBase* const pMark = dynamic_cast< ::sw::mark::MarkBase* >(ppMark->get());
- if (!pMark)
- continue;
+ // correction of non-existent non-MarkBase instances cannot be done
+ assert(pMark);
if(&pMark->GetMarkPos().nNode.GetNode() == pOldNode)
{
SwPosition aNewPosRel(aNewPos);
commit 6c673a29947924148b9550517b98d8953680a2ca
Author: Michael Stahl <mstahl at redhat.com>
Date: Tue Sep 27 14:46:24 2016 +0200
sw: fix sorting in MarkManager::correctMarksAbsolute()
The recently added tdf90697.rtf caused a libstdc++ assertion about
non-sorted m_vAllMarks. This is because a SwXParagraph was disposed, a
bookmark on it was moved by correctMarksAbsolute onto the first
paragraph in a table, and there was a SwXTextRange with a UnoMark
pointing to the SwTableNode, so the sort order was not maintained.
So if at least one mark was moved, re-sort the array.
Change-Id: I0efc7827c826e696f29f8480bab1044b31ccfe39
diff --git a/sw/source/core/doc/docbm.cxx b/sw/source/core/doc/docbm.cxx
index 5e86714..0ecc743 100644
--- a/sw/source/core/doc/docbm.cxx
+++ b/sw/source/core/doc/docbm.cxx
@@ -589,6 +589,7 @@ namespace sw { namespace mark
{
pMark->SetMarkPos(aNewPos);
bChangedPos = true;
+ isSortingNeeded = true;
}
bool bChangedOPos = false;
if (pMark->IsExpanded() &&
@@ -597,6 +598,7 @@ namespace sw { namespace mark
// shift the OtherMark to aNewPos
pMark->SetOtherMarkPos(aNewPos);
bChangedOPos= true;
+ isSortingNeeded = true;
}
// illegal selection? collapse the mark and restore sorting later
isSortingNeeded |= lcl_FixCorrectedMark(bChangedPos, bChangedOPos, pMark);
More information about the Libreoffice-commits
mailing list