Skip to content

Commit

Permalink
tdf#136929 docx export: keep frame with paragraph
Browse files Browse the repository at this point in the history
EndParagraph was checking if any frames were to be
attached. Well, startParagraph can occur multiple
times before endParagraph, so the frames can be
attached to the wrong paragraph. In this case,
it was moving the text-body frame into the footer.

So make a stack of these things, so that each
paragraph can keep track of it's own setting.

RTF can have endParagraph without startParagraph.
Although that doesn't seem to apply
in this DOCX-only context, just to be safe
I'm assuming that it could in theory happen
as well with a DOCX, and so never assume
that the stack exists.

Based on a code read, (and then a
confirming unit test,) things seem to be
complicated by multi-levels of textboxes
that need to be squished.

The for loop with a changing upper end
really threw me for a loop, so I'm
clearly documenting that.

Change-Id: I1060736c0a2174af125d853ff7d72265e000c8de
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/107516
Tested-by: Jenkins
Reviewed-by: Justin Luth <justin_luth@sil.org>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
  • Loading branch information
Justin Luth authored and vmiklos committed Dec 11, 2020
1 parent a4eec60 commit 3fd1564
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 7 deletions.
Binary file not shown.
7 changes: 7 additions & 0 deletions sw/qa/extras/ooxmlexport/ooxmlexport15.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,13 @@ DECLARE_OOXMLEXPORT_TEST(testTdf135216_evenOddFooter, "tdf135216_evenOddFooter.o
getParagraph(2, "2");
}

DECLARE_OOXMLEXPORT_TEST(testTdf136929_framesOfParagraph, "tdf136929_framesOfParagraph.odt")
{
// Before this fix, the image was placed in the footer instead of in the text body - messing everything up.
CPPUNIT_ASSERT_EQUAL_MESSAGE( "Number of Pages", 5, getPages() );
CPPUNIT_ASSERT_EQUAL_MESSAGE("Header2 text", OUString("* | *"), parseDump("/root/page[4]/footer/txt"));
}

DECLARE_OOXMLEXPORT_TEST(testTdf136589_paraHadField, "tdf136589_paraHadField.docx")
{
// The section break should not add an additional CR - which equals an empty page two.
Expand Down
27 changes: 21 additions & 6 deletions sw/source/filter/ww8/docxattributeoutput.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,12 @@ static void checkAndWriteFloatingTables(DocxAttributeOutput& rDocxAttributeOutpu

void DocxAttributeOutput::StartParagraph( ww8::WW8TableNodeInfo::Pointer_t pTextNodeInfo )
{
// Paragraphs (in headers/footers/comments/frames etc) can start before another finishes.
// So a stack is needed to keep track of each paragraph's status separately.
// Complication: Word can't handle nested text boxes, so those need to be collected together.
if ( !m_aFramesOfParagraph.size() || !m_nTextFrameLevel )
m_aFramesOfParagraph.push(std::vector<ww8::Frame>());

// look ahead for floating tables that were put into a frame during import
// floating tables in shapes are not supported: exclude this case
if (!pTextNodeInfo && !m_rExport.SdrExporter().IsDMLAndVMLDrawingOpen())
Expand Down Expand Up @@ -644,10 +650,13 @@ void DocxAttributeOutput::EndParagraph( ww8::WW8TableNodeInfoInner::Pointer_t pT

assert(!m_pPostponedCustomShape);
m_pPostponedCustomShape.reset(new std::vector<PostponedDrawing>);
for (size_t nIndex = 0; nIndex < m_aFramesOfParagraph.size(); ++nIndex)

// The for loop can change the size of m_aFramesOfParagraph, so the max size cannot be set in stone before the loop.
size_t nFrames = m_aFramesOfParagraph.size() ? m_aFramesOfParagraph.top().size() : 0;
for (size_t nIndex = 0; nIndex < nFrames; ++nIndex)
{
m_bParagraphFrameOpen = true;
ww8::Frame aFrame = m_aFramesOfParagraph[nIndex];
ww8::Frame aFrame = m_aFramesOfParagraph.top()[nIndex];
const SwFrameFormat& rFrameFormat = aFrame.GetFrameFormat();

if (!TextBoxIsFramePr(rFrameFormat) || m_bWritingHeaderFooter)
Expand Down Expand Up @@ -711,6 +720,8 @@ void DocxAttributeOutput::EndParagraph( ww8::WW8TableNodeInfoInner::Pointer_t pT
std::shared_ptr<ww8::Frame> pFramePr = std::make_shared<ww8::Frame>(aFrame);
aFramePrTextbox.push_back(pFramePr);
}

nFrames = m_aFramesOfParagraph.size() ? m_aFramesOfParagraph.top().size() : 0;
}
if (!m_pPostponedCustomShape->empty())
{
Expand All @@ -720,7 +731,8 @@ void DocxAttributeOutput::EndParagraph( ww8::WW8TableNodeInfoInner::Pointer_t pT
}
m_pPostponedCustomShape.reset();

m_aFramesOfParagraph.clear();
if ( m_aFramesOfParagraph.size() )
m_aFramesOfParagraph.top().clear();

if (!pTextNodeInfoInner)
{
Expand All @@ -730,6 +742,8 @@ void DocxAttributeOutput::EndParagraph( ww8::WW8TableNodeInfoInner::Pointer_t pT
}

--m_nTextFrameLevel;
if ( m_aFramesOfParagraph.size() && !m_nTextFrameLevel )
m_aFramesOfParagraph.pop();

/* If m_nHyperLinkCount > 0 that means hyperlink tag is not yet closed.
* This is due to nested hyperlink tags. So close it before end of paragraph.
Expand Down Expand Up @@ -6037,10 +6051,10 @@ void DocxAttributeOutput::OutputFlyFrame_Impl( const ww8::Frame &rFrame, const P
// The frame output is postponed to the end of the anchor paragraph
bool bDuplicate = false;
const OUString& rName = rFrame.GetFrameFormat().GetName();
unsigned nSize = m_aFramesOfParagraph.size();
unsigned nSize = m_aFramesOfParagraph.size() ? m_aFramesOfParagraph.top().size() : 0;
for( unsigned nIndex = 0; nIndex < nSize; ++nIndex )
{
const OUString& rNameExisting = m_aFramesOfParagraph[nIndex].GetFrameFormat().GetName();
const OUString& rNameExisting = m_aFramesOfParagraph.top()[nIndex].GetFrameFormat().GetName();

if (!rName.isEmpty() && !rNameExisting.isEmpty())
{
Expand All @@ -6052,7 +6066,8 @@ void DocxAttributeOutput::OutputFlyFrame_Impl( const ww8::Frame &rFrame, const P
if( !bDuplicate )
{
m_bPostponedProcessingFly = true ;
m_aFramesOfParagraph.emplace_back(rFrame);
if ( m_aFramesOfParagraph.size() )
m_aFramesOfParagraph.top().emplace_back(rFrame);
}
}
break;
Expand Down
2 changes: 1 addition & 1 deletion sw/source/filter/ww8/docxattributeoutput.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -854,7 +854,7 @@ private:
// This paragraph must end with page break
bool m_bPageBreakAfter = false;

std::vector<ww8::Frame> m_aFramesOfParagraph;
std::stack< std::vector<ww8::Frame> > m_aFramesOfParagraph;
o3tl::sorted_vector<const SwFrameFormat*> m_aFloatingTablesOfParagraph;
sal_Int32 m_nTextFrameLevel;

Expand Down

0 comments on commit 3fd1564

Please sign in to comment.