[Libreoffice-commits] core.git: 2 commits - sw/qa sw/source
Michael Stahl (via logerrit)
logerrit at kemper.freedesktop.org
Tue Nov 19 21:19:32 UTC 2019
sw/qa/core/data/ww8/pass/ofz18554-1.doc |binary
sw/qa/extras/ooxmlexport/data/tdf128889.fodt | 15 +++++++
sw/qa/extras/ooxmlexport/ooxmlexport14.cxx | 11 +++++
sw/source/core/doc/DocumentContentOperationsManager.cxx | 32 +++++++++++-----
sw/source/core/doc/docbm.cxx | 10 +++++
sw/source/core/inc/bookmrk.hxx | 3 +
sw/source/core/undo/rolbck.cxx | 8 ++--
sw/source/filter/ww8/attributeoutputbase.hxx | 3 +
sw/source/filter/ww8/docxattributeoutput.cxx | 21 ++++++++--
sw/source/filter/ww8/docxattributeoutput.hxx | 6 ++-
sw/source/filter/ww8/docxexport.cxx | 4 +-
sw/source/filter/ww8/rtfattributeoutput.cxx | 3 +
sw/source/filter/ww8/rtfattributeoutput.hxx | 3 +
sw/source/filter/ww8/rtfexport.cxx | 4 +-
sw/source/filter/ww8/ww8atr.cxx | 8 ++--
sw/source/filter/ww8/ww8attributeoutput.hxx | 2 -
16 files changed, 103 insertions(+), 30 deletions(-)
New commits:
commit bd2ada701aad2c4e85d03cd8db68eaeae081d91c
Author: Michael Stahl <Michael.Stahl at cib.de>
AuthorDate: Tue Nov 19 15:39:33 2019 +0100
Commit: Caolán McNamara <caolanm at redhat.com>
CommitDate: Tue Nov 19 22:18:30 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>
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 6f0c1de92bb1..ef4815a1cff4 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);
commit b0e7e494b6bc69d3833c0a6c256ff8106a4a24cb
Author: Mike Kaganski <mike.kaganski at collabora.com>
AuthorDate: Tue Nov 19 22:41:52 2019 +0300
Commit: Mike Kaganski <mike.kaganski at collabora.com>
CommitDate: Tue Nov 19 22:18:27 2019 +0100
tdf#128889: don't write "page break after" into w:pPr
This produced invalid OOXML, which Word considers as "page before",
and LibreOffice ignores when re-importing.
Make sure to write it as *trailing* w:r with w:br, as Word also does
when imports ODT with this atribute, and saves as DOCX.
Change-Id: Ifc4f45d65d4455ecb5cd62aed1ef6a03375c8aa4
Reviewed-on: https://gerrit.libreoffice.org/83232
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski at collabora.com>
diff --git a/sw/qa/extras/ooxmlexport/data/tdf128889.fodt b/sw/qa/extras/ooxmlexport/data/tdf128889.fodt
new file mode 100644
index 000000000000..6dc1c4202696
--- /dev/null
+++ b/sw/qa/extras/ooxmlexport/data/tdf128889.fodt
@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<office:document xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0" xmlns:fo="urn:oasis:names:tc:opendocument:xmlns:xsl-fo-compatible:1.0" xmlns:style="urn:oasis:names:tc:opendocument:xmlns:style:1.0" xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0" office:version="1.2" office:mimetype="application/vnd.oasis.opendocument.text">
+ <office:automatic-styles>
+ <style:style style:name="P1" style:family="paragraph" style:parent-style-name="Standard">
+ <style:paragraph-properties fo:break-after="page"/>
+ </style:style>
+ </office:automatic-styles>
+ <office:body>
+ <office:text>
+ <text:p text:style-name="P1">para1</text:p>
+ <text:p>para2</text:p>
+ </office:text>
+ </office:body>
+</office:document>
\ No newline at end of file
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx
index 8996ba0edad7..3bafcab32d6b 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx
@@ -168,6 +168,17 @@ DECLARE_OOXMLEXPORT_EXPORTONLY_TEST(testTdf128820, "tdf128820.fodt")
"a:graphic/a:graphicData/wpg:wgp/wps:wsp");
}
+DECLARE_OOXMLEXPORT_EXPORTONLY_TEST(testTdf128889, "tdf128889.fodt")
+{
+ xmlDocPtr pXml = parseExport("word/document.xml");
+ CPPUNIT_ASSERT(pXml);
+ // There was an w:r (with w:br) as an invalid child of first paragraph's w:pPr
+ assertXPath(pXml, "/w:document/w:body/w:p[1]/w:pPr/w:r", 0);
+ assertXPath(pXml, "/w:document/w:body/w:p[1]/w:r", 2);
+ // Check that the break is in proper - last - position
+ assertXPath(pXml, "/w:document/w:body/w:p[1]/w:r[2]/w:br", "type", "page");
+}
+
CPPUNIT_PLUGIN_IMPLEMENT();
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/filter/ww8/attributeoutputbase.hxx b/sw/source/filter/ww8/attributeoutputbase.hxx
index 74084f625590..70509ed47806 100644
--- a/sw/source/filter/ww8/attributeoutputbase.hxx
+++ b/sw/source/filter/ww8/attributeoutputbase.hxx
@@ -299,7 +299,8 @@ public:
/// Write a section break
/// msword::ColumnBreak or msword::PageBreak
- virtual void SectionBreak( sal_uInt8 nC, const WW8_SepInfo* pSectionInfo = nullptr ) = 0;
+ /// bBreakAfter: the break must be scheduled for insertion in the end of current paragraph
+ virtual void SectionBreak( sal_uInt8 nC, bool bBreakAfter, const WW8_SepInfo* pSectionInfo = nullptr ) = 0;
// preserve page vertical alignment
virtual void TextVerticalAdjustment( const css::drawing::TextVerticalAdjust) {};
diff --git a/sw/source/filter/ww8/docxattributeoutput.cxx b/sw/source/filter/ww8/docxattributeoutput.cxx
index 3c2c614bf096..4f18582b0d7f 100644
--- a/sw/source/filter/ww8/docxattributeoutput.cxx
+++ b/sw/source/filter/ww8/docxattributeoutput.cxx
@@ -745,6 +745,13 @@ void DocxAttributeOutput::EndParagraph( ww8::WW8TableNodeInfoInner::Pointer_t pT
m_bStartedCharSdt = false;
}
+ if (m_bPageBreakAfter)
+ {
+ // tdf#128889 Trailing page break
+ SectionBreak(msword::PageBreak, false);
+ m_bPageBreakAfter = false;
+ }
+
m_pSerializer->endElementNS( XML_w, XML_p );
// on export sdt blocks are never nested ATM
if( !m_bAnchorLinkedToNode && !m_bStartedParaSdt )
@@ -5981,7 +5988,7 @@ void DocxAttributeOutput::PageBreakBefore( bool bBreak )
FSNS( XML_w, XML_val ), "false" );
}
-void DocxAttributeOutput::SectionBreak( sal_uInt8 nC, const WW8_SepInfo* pSectionInfo )
+void DocxAttributeOutput::SectionBreak( sal_uInt8 nC, bool bBreakAfter, const WW8_SepInfo* pSectionInfo )
{
switch ( nC )
{
@@ -6043,9 +6050,15 @@ void DocxAttributeOutput::SectionBreak( sal_uInt8 nC, const WW8_SepInfo* pSectio
}
else if ( m_bParagraphOpened )
{
- m_pSerializer->startElementNS(XML_w, XML_r);
- m_pSerializer->singleElementNS(XML_w, XML_br, FSNS(XML_w, XML_type), "page");
- m_pSerializer->endElementNS( XML_w, XML_r );
+ if (bBreakAfter)
+ // tdf#128889
+ m_bPageBreakAfter = true;
+ else
+ {
+ m_pSerializer->startElementNS(XML_w, XML_r);
+ m_pSerializer->singleElementNS(XML_w, XML_br, FSNS(XML_w, XML_type), "page");
+ m_pSerializer->endElementNS(XML_w, XML_r);
+ }
}
else
m_bPostponedPageBreak = true;
diff --git a/sw/source/filter/ww8/docxattributeoutput.hxx b/sw/source/filter/ww8/docxattributeoutput.hxx
index 2a8a43a76a79..67561087ceb3 100644
--- a/sw/source/filter/ww8/docxattributeoutput.hxx
+++ b/sw/source/filter/ww8/docxattributeoutput.hxx
@@ -270,7 +270,8 @@ public:
/// Write a section break
/// msword::ColumnBreak or msword::PageBreak
- virtual void SectionBreak( sal_uInt8 nC, const WW8_SepInfo* pSectionInfo = nullptr ) override;
+ /// bBreakAfter: the break must be scheduled for insertion in the end of current paragraph
+ virtual void SectionBreak( sal_uInt8 nC, bool bBreakAfter, const WW8_SepInfo* pSectionInfo = nullptr ) override;
// preserve DOCX page vertical alignment
virtual void TextVerticalAdjustment( const css::drawing::TextVerticalAdjust ) override;
@@ -841,6 +842,9 @@ private:
// beginning of the next paragraph
bool m_bPostponedPageBreak;
+ // This paragraph must end with page break
+ bool m_bPageBreakAfter = false;
+
std::vector<ww8::Frame> m_aFramesOfParagraph;
std::set<const SwFrameFormat*> m_aFloatingTablesOfParagraph;
sal_Int32 m_nTextFrameLevel;
diff --git a/sw/source/filter/ww8/docxexport.cxx b/sw/source/filter/ww8/docxexport.cxx
index c6226fd130ab..3e9c4bc5fd1b 100644
--- a/sw/source/filter/ww8/docxexport.cxx
+++ b/sw/source/filter/ww8/docxexport.cxx
@@ -547,7 +547,7 @@ ErrCode DocxExport::ExportDocument_Impl()
void DocxExport::AppendSection( const SwPageDesc *pPageDesc, const SwSectionFormat* pFormat, sal_uLong nLnNum )
{
- AttrOutput().SectionBreak( msword::PageBreak, m_pSections->CurrentSectionInfo() );
+ AttrOutput().SectionBreak( msword::PageBreak, false, m_pSections->CurrentSectionInfo() );
m_pSections->AppendSection( pPageDesc, pFormat, nLnNum, m_pAttrOutput->IsFirstParagraph() );
}
@@ -622,7 +622,7 @@ void DocxExport::PrepareNewPageDesc( const SfxItemSet* pSet,
{
// tell the attribute output that we are ready to write the section
// break [has to be output inside paragraph properties]
- AttrOutput().SectionBreak( msword::PageBreak, m_pSections->CurrentSectionInfo() );
+ AttrOutput().SectionBreak( msword::PageBreak, false, m_pSections->CurrentSectionInfo() );
const SwSectionFormat* pFormat = GetSectionFormat( rNd );
const sal_uLong nLnNm = GetSectionLineNo( pSet, rNd );
diff --git a/sw/source/filter/ww8/rtfattributeoutput.cxx b/sw/source/filter/ww8/rtfattributeoutput.cxx
index 67622810d0db..6a04e707a706 100644
--- a/sw/source/filter/ww8/rtfattributeoutput.cxx
+++ b/sw/source/filter/ww8/rtfattributeoutput.cxx
@@ -1200,7 +1200,8 @@ void RtfAttributeOutput::PageBreakBefore(bool bBreak)
}
}
-void RtfAttributeOutput::SectionBreak(sal_uInt8 nC, const WW8_SepInfo* pSectionInfo)
+void RtfAttributeOutput::SectionBreak(sal_uInt8 nC, bool /*bBreakAfter*/,
+ const WW8_SepInfo* pSectionInfo)
{
switch (nC)
{
diff --git a/sw/source/filter/ww8/rtfattributeoutput.hxx b/sw/source/filter/ww8/rtfattributeoutput.hxx
index fe0d093ae0a3..4ea8b3845bcd 100644
--- a/sw/source/filter/ww8/rtfattributeoutput.hxx
+++ b/sw/source/filter/ww8/rtfattributeoutput.hxx
@@ -165,7 +165,8 @@ public:
/// Write a section break
/// msword::ColumnBreak or msword::PageBreak
- void SectionBreak(sal_uInt8 nC, const WW8_SepInfo* pSectionInfo = nullptr) override;
+ void SectionBreak(sal_uInt8 nC, bool bBreakAfter,
+ const WW8_SepInfo* pSectionInfo = nullptr) override;
/// Start of the section properties.
void StartSection() override;
diff --git a/sw/source/filter/ww8/rtfexport.cxx b/sw/source/filter/ww8/rtfexport.cxx
index 787833bbac71..f29268032ed0 100644
--- a/sw/source/filter/ww8/rtfexport.cxx
+++ b/sw/source/filter/ww8/rtfexport.cxx
@@ -970,7 +970,7 @@ void RtfExport::PrepareNewPageDesc(const SfxItemSet* pSet, const SwNode& rNd,
// Don't insert a page break, when we're changing page style just because the next page has to be a different one.
if (!m_pAttrOutput->GetPrevPageDesc()
|| m_pAttrOutput->GetPrevPageDesc()->GetFollow() != pNewPgDesc)
- AttrOutput().SectionBreak(msword::PageBreak, m_pSections->CurrentSectionInfo());
+ AttrOutput().SectionBreak(msword::PageBreak, false, m_pSections->CurrentSectionInfo());
}
bool RtfExport::DisallowInheritingOutlineNumbering(const SwFormat& rFormat)
@@ -1026,7 +1026,7 @@ void RtfExport::AppendSection(const SwPageDesc* pPageDesc, const SwSectionFormat
sal_uLong nLnNum)
{
m_pSections->AppendSection(pPageDesc, pFormat, nLnNum);
- AttrOutput().SectionBreak(msword::PageBreak, m_pSections->CurrentSectionInfo());
+ AttrOutput().SectionBreak(msword::PageBreak, false, m_pSections->CurrentSectionInfo());
}
RtfExport::RtfExport(RtfExportFilter* pFilter, SwDoc* pDocument,
diff --git a/sw/source/filter/ww8/ww8atr.cxx b/sw/source/filter/ww8/ww8atr.cxx
index b72a0246bf21..f4525e09b663 100644
--- a/sw/source/filter/ww8/ww8atr.cxx
+++ b/sw/source/filter/ww8/ww8atr.cxx
@@ -2191,7 +2191,7 @@ void AttributeOutputBase::StartTOX( const SwSection& rSect )
SwSection *pParent = rSect.GetParent();
WW8_SepInfo rInfo(&GetExport( ).m_pDoc->GetPageDesc(0),
pParent ? pParent->GetFormat() : nullptr, 0/*nRstLnNum*/);
- GetExport( ).AttrOutput().SectionBreak( msword::PageBreak, &rInfo );
+ GetExport( ).AttrOutput().SectionBreak( msword::PageBreak, false, &rInfo );
}
sStr += "\\c \"" + OUString::number( nCol ) + "\"";
@@ -2499,7 +2499,7 @@ void AttributeOutputBase::EndTOX( const SwSection& rSect,bool bCareEnd )
if ( 0 < nCol )
{
WW8_SepInfo rInfo( &GetExport( ).m_pDoc->GetPageDesc( 0 ), rSect.GetFormat(), 0/*nRstLnNum*/ );
- GetExport( ).AttrOutput().SectionBreak( msword::PageBreak, &rInfo );
+ GetExport( ).AttrOutput().SectionBreak( msword::PageBreak, false, &rInfo );
}
}
}
@@ -3880,13 +3880,13 @@ void AttributeOutputBase::FormatBreak( const SvxFormatBreakItem& rBreak )
}
if ( !bFollowPageDescWritten )
{
- SectionBreak( nC );
+ SectionBreak(nC, !bBefore);
}
}
}
}
-void WW8AttributeOutput::SectionBreak( sal_uInt8 nC, const WW8_SepInfo* /*pSectionInfo*/ )
+void WW8AttributeOutput::SectionBreak( sal_uInt8 nC, bool /*bBreakAfter*/, const WW8_SepInfo* /*pSectionInfo*/ )
{
m_rWW8Export.ReplaceCr( nC );
}
diff --git a/sw/source/filter/ww8/ww8attributeoutput.hxx b/sw/source/filter/ww8/ww8attributeoutput.hxx
index 35d8db7dfa5e..7e3f2a31ff20 100644
--- a/sw/source/filter/ww8/ww8attributeoutput.hxx
+++ b/sw/source/filter/ww8/ww8attributeoutput.hxx
@@ -147,7 +147,7 @@ public:
/// Write a section break
/// msword::ColumnBreak or msword::PageBreak
- virtual void SectionBreak( sal_uInt8 nC, const WW8_SepInfo* pSectionInfo = nullptr ) override;
+ virtual void SectionBreak( sal_uInt8 nC, bool bBreakAfter, const WW8_SepInfo* pSectionInfo = nullptr ) override;
// preserve DOC page vertical alignment
virtual void TextVerticalAdjustment( const css::drawing::TextVerticalAdjust ) override;
More information about the Libreoffice-commits
mailing list