From c2726c29b233b8fa7fbd390aafe149bdf7e94b07 Mon Sep 17 00:00:00 2001
From: Miklos Vajna <vmiklos@collabora.com>
Date: Tue, 14 Dec 2021 13:51:14 +0100
Subject: [PATCH] sw ODF shape import: improve is-textbox check

This builds on top of commit 28d67b792724a23015dec32fb0278b729f676736
(tdf#107776 sw ODF shape import: make is-textbox check more strict,
2019-08-26). Before that commit, any non-empty parent style name on a
shape resulted in an sw textbox. After that commit, we filtered out Writer
images by checking for the Frame parent style name.

The problem with this approach is that in case two documents are to be
merged together, then it's reasonable to rename the Frame style, but
then the complex content inside the shape gets lost.

Fix the problem by going with a middle ground: require Frame as a
prefix, but allow other names as well. This gets e.g. FrameX to work
without breaking the "reject Graphics" use-case.

Change-Id: Id59ed28a64a9398f2f1620f69b5f148d65e6c95a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/126828
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins
---
 xmloff/qa/unit/data/table-in-shape.fodt | 22 ++++++++++++++++++++++
 xmloff/qa/unit/draw.cxx                 | 24 ++++++++++++++++++++++++
 xmloff/source/draw/ximpshap.cxx         |  4 +++-
 3 files changed, 49 insertions(+), 1 deletion(-)
 create mode 100644 xmloff/qa/unit/data/table-in-shape.fodt

diff --git a/xmloff/qa/unit/data/table-in-shape.fodt b/xmloff/qa/unit/data/table-in-shape.fodt
new file mode 100644
index 000000000000..44c2bcb05db2
--- /dev/null
+++ b/xmloff/qa/unit/data/table-in-shape.fodt
@@ -0,0 +1,22 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<office:document xmlns:svg="urn:oasis:names:tc:opendocument:xmlns:svg-compatible:1.0" xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0" xmlns:style="urn:oasis:names:tc:opendocument:xmlns:style:1.0" xmlns:draw="urn:oasis:names:tc:opendocument:xmlns:drawing: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" xmlns:table="urn:oasis:names:tc:opendocument:xmlns:table:1.0" xmlns:drawooo="http://openoffice.org/2010/draw" xmlns:loext="urn:org:documentfoundation:names:experimental:office:xmlns:loext:1.0" office:version="1.3" office:mimetype="application/vnd.oasis.opendocument.text">
+  <office:styles>
+    <style:style style:name="FrameX" style:family="graphic"/>
+  </office:styles>
+  <office:automatic-styles>
+    <style:style style:name="Table1" style:family="table">
+      <style:table-properties style:width="8.042cm" fo:margin-left="0.009cm" fo:margin-top="0cm" fo:margin-bottom="0cm" table:align="left" style:writing-mode="lr-tb"/>
+    </style:style>
+    <style:style style:name="Table1.A" style:family="table-column">
+      <style:table-column-properties style:column-width="8.042cm"/>
+    </style:style>
+    <style:style style:name="Table1.1" style:family="table-row"/>
+    <style:style style:name="Table1.A1" style:family="table-cell"/>
+    <style:style style:name="gr1" style:family="graphic" style:parent-style-name="FrameX"/>
+  </office:automatic-styles>
+  <office:body>
+    <office:text>
+      <text:p text:style-name="Standard"><draw:custom-shape text:anchor-type="char" draw:z-index="0" draw:name="Rectangle: Rounded Corners 1" draw:style-name="gr1" svg:width="8.997cm" svg:height="4.287cm" svg:x="-0.141cm" svg:y="0.035cm"><loext:table table:name="Table1" table:style-name="Table1"><loext:table-column table:style-name="Table1.A"/><loext:table-row table:style-name="Table1.1"><loext:table-cell table:style-name="Table1.A1" office:value-type="string"><text:p>A1</text:p></loext:table-cell></loext:table-row></loext:table><text:p/><draw:enhanced-geometry draw:mirror-horizontal="false" draw:mirror-vertical="false" svg:viewBox="0 0 0 0" draw:text-areas="?f5 ?f5 ?f6 ?f7" draw:type="ooxml-roundRect" draw:modifiers="16667" draw:enhanced-path="M 0 ?f2 L ?f3 0 L ?f11 ?f4 L ?f2 ?f10 Z N" drawooo:enhanced-path="M 0 ?f2 G ?f2 ?f2 ?f12 ?f13 L ?f3 0 G ?f2 ?f2 ?f14 ?f15 L ?f11 ?f4 G ?f2 ?f2 ?f16 ?f17 L ?f2 ?f10 G ?f2 ?f2 ?f18 ?f19 Z N"><draw:equation draw:name="f0" draw:formula="if(0-$0 ,0,if(50000-$0 ,$0 ,50000))"/><draw:equation draw:name="f1" draw:formula="min(logwidth,logheight)"/><draw:equation draw:name="f2" draw:formula="?f1 *?f0 /100000"/><draw:equation draw:name="f3" draw:formula="logwidth+0-?f2 "/><draw:equation draw:name="f4" draw:formula="logheight+0-?f2 "/><draw:equation draw:name="f5" draw:formula="?f2 *29289/100000"/><draw:equation draw:name="f6" draw:formula="logwidth+0-?f5 "/><draw:equation draw:name="f7" draw:formula="logheight+0-?f5 "/><draw:equation draw:name="f8" draw:formula="logwidth/2"/><draw:equation draw:name="f9" draw:formula="logheight/2"/><draw:equation draw:name="f10" draw:formula="logheight"/><draw:equation draw:name="f11" draw:formula="logwidth"/><draw:equation draw:name="f12" draw:formula="(10800000)/60000.0"/><draw:equation draw:name="f13" draw:formula="(5400000)/60000.0"/><draw:equation draw:name="f14" draw:formula="(16200000)/60000.0"/><draw:equation draw:name="f15" draw:formula="(5400000)/60000.0"/><draw:equation draw:name="f16" draw:formula="(0)/60000.0"/><draw:equation draw:name="f17" draw:formula="(5400000)/60000.0"/><draw:equation draw:name="f18" draw:formula="(5400000)/60000.0"/><draw:equation draw:name="f19" draw:formula="(5400000)/60000.0"/><draw:handle draw:handle-range-x-minimum="0" draw:handle-range-x-maximum="50000" draw:handle-position="?f2 0"/></draw:enhanced-geometry></draw:custom-shape></text:p>
+    </office:text>
+  </office:body>
+</office:document>
diff --git a/xmloff/qa/unit/draw.cxx b/xmloff/qa/unit/draw.cxx
index f13a60082d7d..fc07053cbacf 100644
--- a/xmloff/qa/unit/draw.cxx
+++ b/xmloff/qa/unit/draw.cxx
@@ -19,6 +19,8 @@
 #include <com/sun/star/drawing/XDrawPagesSupplier.hpp>
 #include <com/sun/star/drawing/XMasterPageTarget.hpp>
 #include <com/sun/star/util/Color.hpp>
+#include <com/sun/star/text/XTextRange.hpp>
+#include <com/sun/star/text/XTextTable.hpp>
 
 #include <unotools/mediadescriptor.hxx>
 #include <unotools/tempfile.hxx>
@@ -218,6 +220,28 @@ CPPUNIT_TEST_FIXTURE(XmloffDrawTest, testReferToTheme)
                            "color-lum-off");
 }
 
+CPPUNIT_TEST_FIXTURE(XmloffDrawTest, testTableInShape)
+{
+    // Given a document with a shape with a "FrameX" parent style (starts with Frame, but is not
+    // Frame):
+    OUString aURL = m_directories.getURLFromSrc(DATA_DIRECTORY) + "table-in-shape.fodt";
+
+    // When loading that document:
+    getComponent() = loadFromDesktop(aURL);
+
+    // Then make sure the table inside the shape is not lost:
+    uno::Reference<drawing::XDrawPageSupplier> xDrawPageSupplier(getComponent(), uno::UNO_QUERY);
+    uno::Reference<drawing::XDrawPage> xDrawPage = xDrawPageSupplier->getDrawPage();
+    uno::Reference<text::XTextRange> xShape(xDrawPage->getByIndex(0), uno::UNO_QUERY);
+    uno::Reference<container::XEnumerationAccess> xText(xShape->getText(), uno::UNO_QUERY);
+    uno::Reference<container::XEnumeration> xEnum = xText->createEnumeration();
+    uno::Reference<text::XTextTable> xTable(xEnum->nextElement(), uno::UNO_QUERY);
+    // Without the accompanying fix in place, this test would have crashed, as xTable was an empty
+    // reference, i.e. the table inside the shape was lost.
+    uno::Reference<text::XTextRange> xCell(xTable->getCellByName("A1"), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(OUString("A1"), xCell->getString());
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/xmloff/source/draw/ximpshap.cxx b/xmloff/source/draw/ximpshap.cxx
index c1f93885c9c7..4d2dec38a3c8 100644
--- a/xmloff/source/draw/ximpshap.cxx
+++ b/xmloff/source/draw/ximpshap.cxx
@@ -3503,7 +3503,9 @@ SdXMLCustomShapeContext::SdXMLCustomShapeContext(
                 rtl::Reference<XMLTextImportHelper> xTxtImport = GetImport().GetTextImport();
                 XMLPropStyleContext* pStyle = xTxtImport->FindAutoFrameStyle(aStyleName);
                 // Note that this an API name, so intentionally not localized.
-                if (pStyle && pStyle->GetParentName() == "Frame")
+                // Also allow other Frame styles with the same prefix, we just want to reject
+                // Graphics after all.
+                if (pStyle && pStyle->GetParentName().startsWith("Frame"))
                 {
                     mbTextBox = true;
                     break;