summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMiklos Vajna <vmiklos@collabora.com>2022-02-02 16:02:19 +0100
committerXisco Fauli <xiscofauli@libreoffice.org>2022-02-04 09:12:15 +0100
commit04b4c269fc2e4001eb37c4b2c34a023a44ca6ebe (patch)
tree6d2c868090f271e0610ee4ad09e48258dc4da04a
parent6694e3ea9c2f05a20245d94c5c1eda955cb3aacc (diff)
tdf#137920 sw: avoid layout loop when inserting at-char anchored large image
Regression from commit d250ca91c79f457102daf4da81446b7f130fb0ee (sw: insert image: set anchor to at-char by default, 2019-11-18), the problem was that the document has 2 pages, the first page would host the anchor of an inserted image and looped while creating/deleting a next page frame when updating the layout on inserting of an image. This problem was less visible before, as the at-para anchor is a different codepath, which was the previous default. The primary problem is that the loop in SwLayAction::Action() assumes that we make progress, so in case m_bAgain is set, then doing the layout again won't re-set m_bAgain, but it happened here. That happens because each iteration calls SwFrame::InsertPage(), followed by a SwPageFrame::DestroyImpl(). Examining the backtrace of the SwPageFrame ctor leads to SwTextFrame::FormatAdjust() which tries to split the text frame only in case it has either a number portion or a text portion, but the check there was incomplete: a "none" number format results in no number portion, leading to this loop. Fix the problem by checking for HasVisibleNumberingOrBullet() before assuming that we can split an empty text frame into two pieces where at least one of them is not empty. (cherry picked from commit 3e9975cf507e24e9c501575c501833164d217acc) Change-Id: I454e1ec275ad6c00202e65b97adb79647d11a0b7 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/129395 Tested-by: Jenkins Reviewed-by: Michael Stahl <michael.stahl@allotropia.de> Signed-off-by: Xisco Fauli <xiscofauli@libreoffice.org> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/129410
-rw-r--r--sw/qa/core/text/data/empty-numbering-page-split.fodt62
-rw-r--r--sw/qa/core/text/data/image.pngbin0 -> 3148 bytes
-rw-r--r--sw/qa/core/text/text.cxx16
-rw-r--r--sw/source/core/text/frmform.cxx10
4 files changed, 87 insertions, 1 deletions
diff --git a/sw/qa/core/text/data/empty-numbering-page-split.fodt b/sw/qa/core/text/data/empty-numbering-page-split.fodt
new file mode 100644
index 000000000000..517ad2e0fb26
--- /dev/null
+++ b/sw/qa/core/text/data/empty-numbering-page-split.fodt
@@ -0,0 +1,62 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<office:document xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0" xmlns:style="urn:oasis:names:tc:opendocument:xmlns:style:1.0" xmlns:fo="urn:oasis:names:tc:opendocument:xmlns:xsl-fo-compatible:1.0" xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0" office:version="1.3" office:mimetype="application/vnd.oasis.opendocument.text">
+ <office:styles>
+ <style:default-style style:family="paragraph">
+ <style:text-properties fo:font-size="12pt"/>
+ </style:default-style>
+ <style:style style:name="Standard" style:family="paragraph" style:class="text"/>
+ <style:style style:name="Heading" style:family="paragraph" style:parent-style-name="Standard" style:next-style-name="Text_20_body" style:class="text">
+ <style:paragraph-properties fo:margin-top="0.423cm" fo:margin-bottom="0.212cm" style:contextual-spacing="false" fo:keep-with-next="always"/>
+ <style:text-properties fo:font-size="14pt"/>
+ </style:style>
+ <style:style style:name="Text_20_body" style:display-name="Text body" style:family="paragraph" style:parent-style-name="Standard" style:class="text">
+ <style:paragraph-properties fo:margin-top="0cm" fo:margin-bottom="0.212cm"/>
+ </style:style>
+ <style:style style:name="Heading_20_1" style:display-name="Heading 1" style:family="paragraph" style:parent-style-name="Heading" style:class="text">
+ <style:paragraph-properties fo:margin-top="0.423cm" fo:margin-bottom="0.212cm" fo:break-before="page"/>
+ </style:style>
+ <text:outline-style style:name="Outline">
+ <text:outline-level-style text:level="1" style:num-format="">
+ <style:list-level-properties text:min-label-distance="0.381cm"/>
+ </text:outline-level-style>
+ </text:outline-style>
+ </office:styles>
+ <office:automatic-styles>
+ <style:style style:name="P1" style:family="paragraph" style:parent-style-name="Standard" style:master-page-name="Intro">
+ <style:paragraph-properties style:page-number="1"/>
+ </style:style>
+ <style:page-layout style:name="pm1">
+ <style:page-layout-properties fo:page-width="21.001cm" fo:page-height="29.7cm" fo:margin-top="2cm" fo:margin-bottom="2cm" fo:margin-left="2cm" fo:margin-right="2cm"/>
+ <style:header-style>
+ <style:header-footer-properties fo:min-height="0cm" fo:margin-left="0cm" fo:margin-right="0cm" fo:margin-bottom="0cm"/>
+ </style:header-style>
+ <style:footer-style>
+ <style:header-footer-properties fo:min-height="0.6cm" fo:margin-left="0cm" fo:margin-right="0cm" fo:margin-top="0.499cm" style:dynamic-spacing="false"/>
+ </style:footer-style>
+ </style:page-layout>
+ <style:page-layout style:name="pm2">
+ <style:page-layout-properties fo:page-width="21.001cm" fo:page-height="29.7cm" fo:margin-top="2cm" fo:margin-bottom="2cm" fo:margin-left="2cm" fo:margin-right="2cm"/>
+ <style:header-style>
+ <style:header-footer-properties fo:min-height="0cm" fo:margin-left="0cm" fo:margin-right="0cm" fo:margin-bottom="0.499cm"/>
+ </style:header-style>
+ <style:footer-style/>
+ </style:page-layout>
+ </office:automatic-styles>
+ <office:master-styles>
+ <style:master-page style:name="Standard" style:page-layout-name="pm1" style:next-style-name="Intro">
+ <style:header>
+ <text:p/>
+ </style:header>
+ <style:footer>
+ <text:p/>
+ </style:footer>
+ </style:master-page>
+ <style:master-page style:name="Intro" style:page-layout-name="pm2"/>
+ </office:master-styles>
+ <office:body>
+ <office:text text:use-soft-page-breaks="true">
+ <text:h text:style-name="Heading_20_1" text:outline-level="1"/>
+ <text:p text:style-name="P1"/>
+ </office:text>
+ </office:body>
+</office:document>
diff --git a/sw/qa/core/text/data/image.png b/sw/qa/core/text/data/image.png
new file mode 100644
index 000000000000..49e71b07ae8b
--- /dev/null
+++ b/sw/qa/core/text/data/image.png
Binary files differ
diff --git a/sw/qa/core/text/text.cxx b/sw/qa/core/text/text.cxx
index 4db880e687f4..5f1b89e34247 100644
--- a/sw/qa/core/text/text.cxx
+++ b/sw/qa/core/text/text.cxx
@@ -135,6 +135,22 @@ CPPUNIT_TEST_FIXTURE(SwCoreTextTest, testTabOverMarginSection)
CPPUNIT_ASSERT_LESS(static_cast<sal_Int32>(5000), nWidth);
}
+CPPUNIT_TEST_FIXTURE(SwCoreTextTest, testEmptyNumberingPageSplit)
+{
+ // Given a document with 2 pages: the only para on page 1 is a numbering without a number
+ // portion:
+ createSwDoc(DATA_DIRECTORY, "empty-numbering-page-split.fodt");
+
+ // When inserting an image that doesn't fit the body frame:
+ // Then make sure that the layout update after insertion finishes:
+ uno::Sequence<beans::PropertyValue> aArgs = {
+ comphelper::makePropertyValue("FileName",
+ m_directories.getURLFromSrc(DATA_DIRECTORY) + "image.png"),
+ };
+ // Without the accompanying fix in place, this never finished.
+ dispatchCommand(mxComponent, ".uno:InsertGraphic", aArgs);
+}
+
CPPUNIT_PLUGIN_IMPLEMENT();
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/text/frmform.cxx b/sw/source/core/text/frmform.cxx
index f9f6935e9a9d..bdd3f1d9d17c 100644
--- a/sw/source/core/text/frmform.cxx
+++ b/sw/source/core/text/frmform.cxx
@@ -1117,13 +1117,21 @@ void SwTextFrame::FormatAdjust( SwTextFormatter &rLine,
}
else
{
+ const SwTextNode* pTextNode = GetTextNodeForParaProps();
+ bool bHasVisibleNumRule = nStrLen == TextFrameIndex(0) && pTextNode->GetNumRule();
+
+ if (!pTextNode->HasVisibleNumberingOrBullet())
+ {
+ bHasVisibleNumRule = false;
+ }
+
// Only split frame, if the frame contains
// content or contains no content, but has a numbering.
// i#84870 - No split, if text frame only contains one
// as-character anchored object.
if ( !bOnlyContainsAsCharAnchoredObj &&
(nStrLen > TextFrameIndex(0) ||
- (nStrLen == TextFrameIndex(0) && GetTextNodeForParaProps()->GetNumRule()))
+ bHasVisibleNumRule )
)
{
SplitFrame( nEnd );