[Libreoffice-commits] core.git: Branch 'libreoffice-6-4' - 2 commits - sw/qa sw/source
Michael Stahl (via logerrit)
logerrit at kemper.freedesktop.org
Wed Nov 20 11:01:33 UTC 2019
sw/qa/core/data/ww8/pass/ofz18554-1.doc |binary
sw/source/core/doc/DocumentContentOperationsManager.cxx | 32 +++++++++++-----
sw/source/core/doc/docbm.cxx | 10 +++++
sw/source/core/inc/bookmrk.hxx | 3 +
sw/source/core/inc/rolbck.hxx | 4 +-
sw/source/core/undo/rolbck.cxx | 20 ++++++----
sw/source/core/undo/undobj.cxx | 10 ++---
sw/source/core/undo/untbl.cxx | 2 -
8 files changed, 56 insertions(+), 25 deletions(-)
New commits:
commit bccaf21242f7326b04c7914a6b527cc6f21d5932
Author: Michael Stahl <Michael.Stahl at cib.de>
AuthorDate: Tue Nov 19 10:54:57 2019 +0100
Commit: Michael Stahl <michael.stahl at cib.de>
CommitDate: Wed Nov 20 12:00:33 2019 +0100
sw: fix invalid downcast of SwDrawFrameFormat in DelContentIndex()
failure in UITest_writer_tests6:
sw/source/core/undo/undobj.cxx:1021:47: runtime error: downcast of address 0x61300019d3c0 which does not point to an object of type 'SwFlyFrameFormat'
0x61300019d3c0:m note: object is of type 'SwDrawFrameFormat'
a2 01 80 19 50 ac d1 9e 14 7f 00 00 00 af 19 00 30 61 00 00 e8 4f 0b 00 b0 61 00 00 40 dd 40 00
^~~~~~~~~~~~~~~~~~~~~~~
vptr for 'SwDrawFrameFormat'
in SwUndoSaveContent::DelContentIndex(SwPosition const&, SwPosition const&, DelContentType) at sw/source/core/undo/undobj.cxx:1021:47 (instdir/program/../program/libswlo.so +0xf8e5833)
Also lose the ridiculous overloading across derived classes.
Change-Id: I19439d0d511c5f8c36b00d8dd02c31f802a44e00
Reviewed-on: https://gerrit.libreoffice.org/83175
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl at cib.de>
(cherry picked from commit 8f7274682661baec4c9b160ee2165972bbfa9881)
Reviewed-on: https://gerrit.libreoffice.org/83220
diff --git a/sw/source/core/inc/rolbck.hxx b/sw/source/core/inc/rolbck.hxx
index 91e9d85574b1..2abe1d590b88 100644
--- a/sw/source/core/inc/rolbck.hxx
+++ b/sw/source/core/inc/rolbck.hxx
@@ -364,8 +364,8 @@ public:
void Add( SwTextAttr* pTextHt, sal_uLong nNodeIdx, bool bNewAttr );
void Add( SwFormatColl*, sal_uLong nNodeIdx, SwNodeType nWhichNd );
void Add( const ::sw::mark::IMark&, bool bSavePos, bool bSaveOtherPos );
- void Add( SwFrameFormat& rFormat );
- void Add( SwFlyFrameFormat&, sal_uInt16& rSetPos );
+ void AddChangeFlyAnchor( SwFrameFormat& rFormat );
+ void AddDeleteFly( SwFrameFormat&, sal_uInt16& rSetPos );
void Add( const SwTextFootnote& );
void Add( const SfxItemSet & rSet, const SwCharFormat & rCharFormat);
diff --git a/sw/source/core/undo/rolbck.cxx b/sw/source/core/undo/rolbck.cxx
index e768b0a1451b..ef4815a1cff4 100644
--- a/sw/source/core/undo/rolbck.cxx
+++ b/sw/source/core/undo/rolbck.cxx
@@ -1095,18 +1095,21 @@ void SwHistory::Add(const ::sw::mark::IMark& rBkmk, bool bSavePos, bool bSaveOth
m_SwpHstry.push_back( std::move(pHt) );
}
-void SwHistory::Add( SwFrameFormat& rFormat )
+void SwHistory::AddChangeFlyAnchor(SwFrameFormat& rFormat)
{
std::unique_ptr<SwHistoryHint> pHt(new SwHistoryChangeFlyAnchor( rFormat ));
m_SwpHstry.push_back( std::move(pHt) );
}
-void SwHistory::Add( SwFlyFrameFormat& rFormat, sal_uInt16& rSetPos )
+void SwHistory::AddDeleteFly(SwFrameFormat& rFormat, sal_uInt16& rSetPos)
{
OSL_ENSURE( !m_nEndDiff, "History was not deleted after REDO" );
const sal_uInt16 nWh = rFormat.Which();
- if( RES_FLYFRMFMT == nWh || RES_DRAWFRMFMT == nWh )
+ (void) nWh;
+ // only Flys!
+ assert((RES_FLYFRMFMT == nWh && dynamic_cast<SwFlyFrameFormat*>(&rFormat))
+ || (RES_DRAWFRMFMT == nWh && dynamic_cast<SwDrawFrameFormat*>(&rFormat)));
{
std::unique_ptr<SwHistoryHint> pHint(new SwHistoryTextFlyCnt( &rFormat ));
m_SwpHstry.push_back( std::move(pHint) );
@@ -1115,10 +1118,11 @@ void SwHistory::Add( SwFlyFrameFormat& rFormat, sal_uInt16& rSetPos )
if( SfxItemState::SET == rFormat.GetItemState( RES_CHAIN, false,
reinterpret_cast<const SfxPoolItem**>(&pChainItem) ))
{
+ assert(RES_FLYFRMFMT == nWh); // not supported on SdrObjects
if( pChainItem->GetNext() || pChainItem->GetPrev() )
{
std::unique_ptr<SwHistoryHint> pHt(
- new SwHistoryChangeFlyChain( rFormat, *pChainItem ));
+ new SwHistoryChangeFlyChain(static_cast<SwFlyFrameFormat&>(rFormat), *pChainItem));
m_SwpHstry.insert( m_SwpHstry.begin() + rSetPos++, std::move(pHt) );
if ( pChainItem->GetNext() )
{
diff --git a/sw/source/core/undo/undobj.cxx b/sw/source/core/undo/undobj.cxx
index bb5683853a51..e54290e941b0 100644
--- a/sw/source/core/undo/undobj.cxx
+++ b/sw/source/core/undo/undobj.cxx
@@ -988,7 +988,7 @@ void SwUndoSaveContent::DelContentIndex( const SwPosition& rMark,
// Do not try to move the anchor to a table!
&& rMark.nNode.GetNode().IsTextNode())
{
- m_pHistory->Add( *pFormat );
+ m_pHistory->AddChangeFlyAnchor(*pFormat);
SwFormatAnchor aAnch( *pAnchor );
SwPosition aPos( rMark.nNode );
aAnch.SetAnchor( &aPos );
@@ -996,7 +996,7 @@ void SwUndoSaveContent::DelContentIndex( const SwPosition& rMark,
}
else
{
- m_pHistory->Add( *static_cast<SwFlyFrameFormat *>(pFormat), nChainInsPos );
+ m_pHistory->AddDeleteFly(*pFormat, nChainInsPos );
// reset n so that no Format is skipped
n = n >= rSpzArr.size() ?
rSpzArr.size() : n+1;
@@ -1014,7 +1014,7 @@ void SwUndoSaveContent::DelContentIndex( const SwPosition& rMark,
if (IsDestroyFrameAnchoredAtChar(
*pAPos, *pStt, *pEnd, nDelContentType))
{
- m_pHistory->Add( *static_cast<SwFlyFrameFormat *>(pFormat), nChainInsPos );
+ m_pHistory->AddDeleteFly(*pFormat, nChainInsPos);
n = n >= rSpzArr.size() ? rSpzArr.size() : n+1;
}
else if (!((DelContentType::CheckNoCntnt |
@@ -1028,7 +1028,7 @@ void SwUndoSaveContent::DelContentIndex( const SwPosition& rMark,
// Do not try to move the anchor to a table!
if( rMark.nNode.GetNode().GetTextNode() )
{
- m_pHistory->Add( *pFormat );
+ m_pHistory->AddChangeFlyAnchor(*pFormat);
SwFormatAnchor aAnch( *pAnchor );
aAnch.SetAnchor( &rMark );
pFormat->SetFormatAttr( aAnch );
@@ -1045,7 +1045,7 @@ void SwUndoSaveContent::DelContentIndex( const SwPosition& rMark,
if( !m_pHistory )
m_pHistory.reset( new SwHistory );
- m_pHistory->Add( *static_cast<SwFlyFrameFormat *>(pFormat), nChainInsPos );
+ m_pHistory->AddDeleteFly(*pFormat, nChainInsPos);
// reset n so that no Format is skipped
n = n >= rSpzArr.size() ? rSpzArr.size() : n+1;
diff --git a/sw/source/core/undo/untbl.cxx b/sw/source/core/undo/untbl.cxx
index fb617fd6e2aa..9d1675c0f304 100644
--- a/sw/source/core/undo/untbl.cxx
+++ b/sw/source/core/undo/untbl.cxx
@@ -424,7 +424,7 @@ SwUndoTableToText::SwUndoTableToText( const SwTable& rTable, sal_Unicode cCh )
nTableStt <= pAPos->nNode.GetIndex() &&
pAPos->nNode.GetIndex() < nTableEnd )
{
- pHistory->Add( *pFormat );
+ pHistory->AddChangeFlyAnchor(*pFormat);
}
}
commit 60c3571a8a4ee2d5ecc018465a273cdae63ab825
Author: Michael Stahl <Michael.Stahl at cib.de>
AuthorDate: Tue Nov 19 15:39:33 2019 +0100
Commit: Michael Stahl <michael.stahl at cib.de>
CommitDate: Wed Nov 20 12:00:04 2019 +0100
ofz#18554 sw: fix Null-dereference due to overlapping fieldmarks
The problem is that the WW8 import wants to set a fieldmark on a range
that contains only the CH_TXT_ATR_FIELDEND of another fieldmark:
(rr) p io_pDoc->GetNodes()[12]->m_Text.copy(33,10)
$30 = "\bÿÿÿ\001ÿÿÿ\001 "
MarkManager::makeMark() must check that a new fieldmark never overlaps
existing fieldmarks or meta-fields.
While at it, it looks like the test in
DocumentContentOperationsManager::DelFullPara() can't necessarily use
the passed rPam, because it obviously deletes entire nodes, but at
least SwRangeRedline::DelCopyOfSection() doesn't even set nContent
on rPam.
Also, the check in makeMark() triggers an assert in
CppunitTest_sw_uiwriter testTextFormFieldInsertion because
SwHistoryTextFieldmark::SetInDoc() was neglecting to subtract 1
from the end position for the CH_TXT_ATR_FIELDEND.
Change-Id: I46c1955dd8dd422a41dcbb9bc68dbe09075b4922
Reviewed-on: https://gerrit.libreoffice.org/83000
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolanm at redhat.com>
Tested-by: Caolán McNamara <caolanm at redhat.com>
(cherry picked from commit bd2ada701aad2c4e85d03cd8db68eaeae081d91c)
Reviewed-on: https://gerrit.libreoffice.org/83268
Reviewed-by: Michael Stahl <michael.stahl at cib.de>
diff --git a/sw/qa/core/data/ww8/pass/ofz18554-1.doc b/sw/qa/core/data/ww8/pass/ofz18554-1.doc
new file mode 100644
index 000000000000..0a6b81d78b43
Binary files /dev/null and b/sw/qa/core/data/ww8/pass/ofz18554-1.doc differ
diff --git a/sw/source/core/doc/DocumentContentOperationsManager.cxx b/sw/source/core/doc/DocumentContentOperationsManager.cxx
index 995e3dcfa482..3b9433619ae6 100644
--- a/sw/source/core/doc/DocumentContentOperationsManager.cxx
+++ b/sw/source/core/doc/DocumentContentOperationsManager.cxx
@@ -60,6 +60,7 @@
#include <UndoAttribute.hxx>
#include <rolbck.hxx>
#include <acorrect.hxx>
+#include <bookmrk.hxx>
#include <ftnidx.hxx>
#include <txtftn.hxx>
#include <hints.hxx>
@@ -1796,6 +1797,16 @@ namespace //local functions originally from docfmt.cxx
namespace sw
{
+namespace mark
+{
+ bool IsFieldmarkOverlap(SwPaM const& rPaM)
+ {
+ std::vector<std::pair<sal_uLong, sal_Int32>> Breaks;
+ lcl_CalcBreaks(Breaks, rPaM);
+ return !Breaks.empty();
+ }
+}
+
DocumentContentOperationsManager::DocumentContentOperationsManager( SwDoc& i_rSwdoc ) : m_rDoc( i_rSwdoc )
{
}
@@ -1947,9 +1958,16 @@ bool DocumentContentOperationsManager::DelFullPara( SwPaM& rPam )
}
{
- std::vector<std::pair<sal_uLong, sal_Int32>> Breaks;
- lcl_CalcBreaks(Breaks, rPam);
- if (!Breaks.empty())
+ SwPaM temp(rPam, nullptr);
+ if (SwTextNode *const pNode = temp.Start()->nNode.GetNode().GetTextNode())
+ { // rPam may not have nContent set but IsFieldmarkOverlap requires it
+ pNode->MakeStartIndex(&temp.Start()->nContent);
+ }
+ if (SwTextNode *const pNode = temp.End()->nNode.GetNode().GetTextNode())
+ {
+ pNode->MakeEndIndex(&temp.End()->nContent);
+ }
+ if (sw::mark::IsFieldmarkOverlap(temp))
{ // a bit of a problem: we want to completely remove the nodes
// but then how can the CH_TXT_ATR survive?
return false;
@@ -2100,13 +2118,7 @@ bool DocumentContentOperationsManager::MoveRange( SwPaM& rPaM, SwPosition& rPos,
if( !rPaM.HasMark() || *pStt >= *pEnd || (*pStt <= rPos && rPos < *pEnd))
return false;
-#ifndef NDEBUG
- {
- std::vector<std::pair<sal_uLong, sal_Int32>> Breaks;
- lcl_CalcBreaks(Breaks, rPaM);
- assert(Breaks.empty()); // probably an invalid redline was created?
- }
-#endif
+ assert(!sw::mark::IsFieldmarkOverlap(rPaM)); // probably an invalid redline was created?
// Save the paragraph anchored Flys, so that they can be moved.
SaveFlyArr aSaveFlyArr;
diff --git a/sw/source/core/doc/docbm.cxx b/sw/source/core/doc/docbm.cxx
index c2118fd444f2..c0c973904bf5 100644
--- a/sw/source/core/doc/docbm.cxx
+++ b/sw/source/core/doc/docbm.cxx
@@ -582,6 +582,16 @@ namespace sw { namespace mark
return nullptr;
}
+ if ((eType == MarkType::TEXT_FIELDMARK || eType == MarkType::DATE_FIELDMARK)
+ // can't check for Copy - it asserts - but it's also obviously unnecessary
+ && eMode == InsertMode::New
+ && sw::mark::IsFieldmarkOverlap(rPaM))
+ {
+ SAL_WARN("sw.core", "MarkManager::makeMark(..)"
+ " - invalid range on fieldmark, overlaps existing fieldmark or meta-field");
+ return nullptr;
+ }
+
// create mark
std::unique_ptr<::sw::mark::MarkBase> pMark;
switch(eType)
diff --git a/sw/source/core/inc/bookmrk.hxx b/sw/source/core/inc/bookmrk.hxx
index cd0e154185db..3960ca4b3d8b 100644
--- a/sw/source/core/inc/bookmrk.hxx
+++ b/sw/source/core/inc/bookmrk.hxx
@@ -339,6 +339,9 @@ namespace sw {
/// return position of the CH_TXT_ATR_FIELDSEP for rMark
SwPosition FindFieldSep(IFieldmark const& rMark);
+
+ /// check if rPaM is valid range of new fieldmark
+ bool IsFieldmarkOverlap(SwPaM const& rPaM);
}
}
#endif
diff --git a/sw/source/core/undo/rolbck.cxx b/sw/source/core/undo/rolbck.cxx
index 353ce708fe8a..e768b0a1451b 100644
--- a/sw/source/core/undo/rolbck.cxx
+++ b/sw/source/core/undo/rolbck.cxx
@@ -744,11 +744,13 @@ void SwHistoryTextFieldmark::SetInDoc(SwDoc* pDoc, bool)
SwPaM const pam(*rNds[m_nStartNode]->GetContentNode(), m_nStartContent,
*rNds[m_nEndNode]->GetContentNode(),
+ // subtract 1 for the CH_TXT_ATR_FIELDEND itself,
+ // plus more if same node as other CH_TXT_ATR
m_nStartNode == m_nEndNode
- ? (m_nEndContent - 2)
+ ? (m_nEndContent - 3)
: m_nSepNode == m_nEndNode
- ? (m_nEndContent - 1)
- : m_nEndContent);
+ ? (m_nEndContent - 2)
+ : (m_nEndContent - 1));
SwPosition const sepPos(*rNds[m_nSepNode]->GetContentNode(),
m_nStartNode == m_nSepNode ? (m_nSepContent - 1) : m_nSepContent);
More information about the Libreoffice-commits
mailing list