ODFstudio/odfstudio
2
0
Fork 0
Code Issues Packages Activity

tdf#162342: workaround out-of-order context stack pop

Browse Source 
Turns out, the context stack is populated in one order, but emptied in
another. Commit e63a75e00dcd797b0bd061add3c72dcb7b353007 (tdf#158586
writerfilter: RTF import: fix \page \sect \skbnone, 2024-02-01) added
yet another case, which resulted in m_pTopContext being empty at the
moment when section properties should have been applied (see "needed
for page properties" comment in DomainMapper::sprmWithProps; that code
wasn't reached because of (!rContext) check above).

But that isn't the only case; I tried to assert in non-RTF cases, and
it fired in `make check`, too.

So here is an UGLY HACK. I repair the stack when it's popped out of
order. This pleads for a proper fix, but the effort will be far from
trivial.

Change-Id: I1a5c646188f3da4842d103b8edeb11b801fea830
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/188449
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
This commit is contained in:
Mike Kaganski
2025-07-28 16:35:19 +02:00
parent d1fb9ff22f
commit 1567f48ec9
3 changed files with 101 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
sw/qa/extras/rtfexport/data/tdf162342.rtf
+7
View File
@@ -0,0 +1,7 @@
{\rtf1
\margl720 \margr360 \margt720 \margb720 \paperw6120 \paperh7920
First
\page
Second
\page
Third}
sw/qa/extras/rtfexport/rtfexport8.cxx
+64
View File
@@ -1211,6 +1211,70 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf167569_2)
}
}
CPPUNIT_TEST_FIXTURE(Test, testTdf162342)
{
// Given an RTF with some page settings, and two page breaks:
createSwDoc("tdf162342.rtf");
{
xmlDocUniquePtr pLayout = parseLayoutDump();
assertXPath(pLayout, "//page['pass 1']", 3);
// Without the fix, this failed with
// - Expected: 6120
// - Actual : 12240
// i.e., the page geometry was ignored.
assertXPath(pLayout, "//page[1]['pass 1']/infos/bounds", "width", u"6120");
assertXPath(pLayout, "//page[1]['pass 1']/infos/bounds", "height", u"7937"); // Not 7920?
assertXPath(pLayout, "//page[1]['pass 1']/infos/prtBounds", "left", u"720");
assertXPath(pLayout, "//page[1]['pass 1']/infos/prtBounds", "top", u"720");
assertXPath(pLayout, "//page[1]['pass 1']/infos/prtBounds", "width", u"5040");
assertXPath(pLayout, "//page[1]['pass 1']/infos/prtBounds", "height", u"6497");
assertXPath(pLayout, "//page[2]['pass 1']/infos/bounds", "width", u"6120");
assertXPath(pLayout, "//page[2]['pass 1']/infos/bounds", "height", u"7937");
assertXPath(pLayout, "//page[2]['pass 1']/infos/prtBounds", "left", u"720");
assertXPath(pLayout, "//page[2]['pass 1']/infos/prtBounds", "top", u"720");
assertXPath(pLayout, "//page[2]['pass 1']/infos/prtBounds", "width", u"5040");
assertXPath(pLayout, "//page[2]['pass 1']/infos/prtBounds", "height", u"6497");
assertXPath(pLayout, "//page[3]['pass 1']/infos/bounds", "width", u"6120");
assertXPath(pLayout, "//page[3]['pass 1']/infos/bounds", "height", u"7937");
assertXPath(pLayout, "//page[3]['pass 1']/infos/prtBounds", "left", u"720");
assertXPath(pLayout, "//page[3]['pass 1']/infos/prtBounds", "top", u"720");
assertXPath(pLayout, "//page[3]['pass 1']/infos/prtBounds", "width", u"5040");
assertXPath(pLayout, "//page[3]['pass 1']/infos/prtBounds", "height", u"6497");
}
saveAndReload(mpFilter);
{
xmlDocUniquePtr pLayout = parseLayoutDump();
assertXPath(pLayout, "//page['pass 2']", 3);
assertXPath(pLayout, "//page[1]['pass 2']/infos/bounds", "width", u"6120");
assertXPath(pLayout, "//page[1]['pass 2']/infos/bounds", "height", u"7937");
assertXPath(pLayout, "//page[1]['pass 2']/infos/prtBounds", "left", u"720");
assertXPath(pLayout, "//page[1]['pass 2']/infos/prtBounds", "top", u"720");
assertXPath(pLayout, "//page[1]['pass 2']/infos/prtBounds", "width", u"5040");
assertXPath(pLayout, "//page[1]['pass 2']/infos/prtBounds", "height", u"6497");
assertXPath(pLayout, "//page[2]['pass 2']/infos/bounds", "width", u"6120");
assertXPath(pLayout, "//page[2]['pass 2']/infos/bounds", "height", u"7937");
assertXPath(pLayout, "//page[2]['pass 2']/infos/prtBounds", "left", u"720");
assertXPath(pLayout, "//page[2]['pass 2']/infos/prtBounds", "top", u"720");
assertXPath(pLayout, "//page[2]['pass 2']/infos/prtBounds", "width", u"5040");
assertXPath(pLayout, "//page[2]['pass 2']/infos/prtBounds", "height", u"6497");
assertXPath(pLayout, "//page[3]['pass 2']/infos/bounds", "width", u"6120");
assertXPath(pLayout, "//page[3]['pass 2']/infos/bounds", "height", u"7937");
assertXPath(pLayout, "//page[3]['pass 2']/infos/prtBounds", "left", u"720");
assertXPath(pLayout, "//page[3]['pass 2']/infos/prtBounds", "top", u"720");
assertXPath(pLayout, "//page[3]['pass 2']/infos/prtBounds", "width", u"5040");
assertXPath(pLayout, "//page[3]['pass 2']/infos/prtBounds", "height", u"6497");
}
}
} // end of anonymous namespace
CPPUNIT_PLUGIN_IMPLEMENT();
sw/source/writerfilter/dmapper/DomainMapper_Impl.cxx
+30 -4
View File
@@ -1370,13 +1370,39 @@ void DomainMapper_Impl::PopProperties(ContextType eId)
}
m_aPropertyStacks[eId].pop();
{
// !!!FIXME!!!
// We sometimes pop out of order. Just try to comment out this fixing block, keeping the
// assert; and run `make check`. Sigh! In that case, popping from m_aContextStack would
// make it (and m_pTopContext) out-of-sync. This must be an abort; but let's try to fix
// the stack instead, as a temporary (?) measure. I don't replace the stack with a more
// easy-to-manipulate container: I don't optimize for invalid case.
// !!!FIXME!!!
SAL_WARN_IF(m_aContextStack.top() != eId, "writerfilter.dmapper", "pop out of order");
std::stack<ContextType> tmp;
while (m_aContextStack.top() != eId)
{
tmp.push(m_aContextStack.top());
m_aContextStack.pop();
}
m_aContextStack.pop();
while (!tmp.empty())
{
m_aContextStack.push(tmp.top());
tmp.pop();
}
m_aContextStack.push(eId); // to pop below
}
assert(m_aContextStack.top() == eId);
m_aContextStack.pop();
if(!m_aContextStack.empty() && !m_aPropertyStacks[m_aContextStack.top()].empty())
m_pTopContext = m_aPropertyStacks[m_aContextStack.top()].top();
if (!m_aContextStack.empty() && !m_aPropertyStacks[m_aContextStack.top()].empty())
{
m_pTopContext = m_aPropertyStacks[m_aContextStack.top()].top();
}
else
{
// OSL_ENSURE(eId == CONTEXT_SECTION, "this should happen at a section context end");
SAL_WARN_IF(eId != CONTEXT_SECTION, "writerfilter.dmapper",
"this should happen at a section context end");
m_pTopContext.clear();
}
}