sw XHTML import: improve handling of <object> with images

The reqif/xhtml export can map images to a pair of <object> elements:
the outer one contains the real image and the inner one contains a PNG
fallback.  We used to import this back to OLE objects, except for PNG,
which has a single <object> element.

Improve this by allowing the pair of <object> elements as well: if the
outer type is GIF, then map this to an image and ignore the inner PNG.

Change-Id: I5c56720cf6a054208cd0478a54bdac15ffece9aa
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/119221
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins

5 files changed, 53 insertions, 2 deletions

diff --git a/sw/qa/extras/htmlimport/data/ole-img.xhtml b/sw/qa/extras/htmlimport/data/ole-img.xhtml
new file mode 100644
index 000000000000..798787290c18
--- /dev/null
+++ b/sw/qa/extras/htmlimport/data/ole-img.xhtml
@@ -0,0 +1,7 @@
+<reqif-xhtml:div>
+<reqif-xhtml:p>
+<reqif-xhtml:object data="ole2.gif" type="image/gif">
+    <reqif-xhtml:object data="ole2.png" type="image/png"></reqif-xhtml:object>
+</reqif-xhtml:object>
+</reqif-xhtml:p>
+</reqif-xhtml:div>
diff --git a/sw/qa/extras/htmlimport/data/ole2.gif b/sw/qa/extras/htmlimport/data/ole2.gif
new file mode 100644
index 000000000000..19e9785e527c
--- /dev/null
+++ b/sw/qa/extras/htmlimport/data/ole2.gif
diff --git a/sw/qa/extras/htmlimport/htmlimport.cxx b/sw/qa/extras/htmlimport/htmlimport.cxx
index 066d41b921aa..033d53e8629b 100644
--- a/sw/qa/extras/htmlimport/htmlimport.cxx
+++ b/sw/qa/extras/htmlimport/htmlimport.cxx
@@ -461,6 +461,30 @@ CPPUNIT_TEST_FIXTURE(SwHtmlOptionsImportTest, testHiddenTextframe)
     CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(0), xDrawPage->getCount());
 }
 
+CPPUNIT_TEST_FIXTURE(SwModelTestBase, testOleImg)
+{
+    // Given an XHTML with an <object> (containing GIF) and an inner <object> (containing PNG, to be
+    // ignored):
+    uno::Sequence<beans::PropertyValue> aLoadProperties = {
+        comphelper::makePropertyValue("FilterName", OUString("HTML (StarWriter)")),
+        comphelper::makePropertyValue("FilterOptions", OUString("xhtmlns=reqif-xhtml")),
+    };
+    OUString aURL = m_directories.getURLFromSrc(DATA_DIRECTORY) + "ole-img.xhtml";
+
+    // When loading the document:
+    mxComponent = loadFromDesktop(aURL, "com.sun.star.text.TextDocument", aLoadProperties);
+
+    // Then make sure the result is a single Writer image:
+    uno::Reference<text::XTextGraphicObjectsSupplier> xSupplier(mxComponent, uno::UNO_QUERY);
+    uno::Reference<container::XIndexAccess> xObjects(xSupplier->getGraphicObjects(),
+                                                     uno::UNO_QUERY);
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected: 0
+    // - Actual  : 1
+    // i.e. the image was not imported as a Writer image (but as an OLE object).
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(1), xObjects->getCount());
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/filter/html/htmlplug.cxx b/sw/source/filter/html/htmlplug.cxx
index 96ec2efad939..e09de84671e6 100644
--- a/sw/source/filter/html/htmlplug.cxx
+++ b/sw/source/filter/html/htmlplug.cxx
@@ -434,9 +434,17 @@ bool SwHTMLParser::InsertEmbed()
         aCmdLst.Append( rOption.GetTokenString(), rOption.GetString() );
     }
 
-    if (aType == "image/png" && m_aEmbeds.empty())
-        // Toplevel <object> for PNG -> that's an image, not an OLE object.
+    static const std::set<std::u16string_view> vAllowlist = {
+        u"image/png",
+        u"image/gif",
+    };
+
+    if (vAllowlist.find(aType) != vAllowlist.end() && m_aEmbeds.empty())
+    {
+        // Toplevel <object> for an image format -> that's an image, not an OLE object.
+        m_aEmbeds.push(nullptr);
         return false;
+    }
 
     SfxItemSet aItemSet( m_xDoc->GetAttrPool(), m_pCSS1Parser->GetWhichMap() );
     SvxCSS1PropertyInfo aPropInfo;
@@ -488,6 +496,12 @@ bool SwHTMLParser::InsertEmbed()
     {
         // Nested XHTML <object> element: points to replacement graphic.
         SwOLENode* pOLENode = m_aEmbeds.top();
+        if (!pOLENode)
+        {
+            // <object> is mapped to an image -> ignore replacement graphic.
+            return true;
+        }
+
         svt::EmbeddedObjectRef& rObj = pOLENode->GetOLEObj().GetObject();
         Graphic aGraphic;
         if (GraphicFilter::GetGraphicFilter().ImportGraphic(aGraphic, aURLObj) != ERRCODE_NONE)
diff --git a/sw/source/filter/html/swhtml.cxx b/sw/source/filter/html/swhtml.cxx
index f12fba16bb54..70b68e4e1df1 100644
--- a/sw/source/filter/html/swhtml.cxx
+++ b/sw/source/filter/html/swhtml.cxx
@@ -1532,6 +1532,12 @@ void SwHTMLParser::NextToken( HtmlTokenId nToken )
                 // The text token is inside an OLE object, which means
                 // alternate text.
                 SwOLENode* pOLENode = m_aEmbeds.top();
+                if (!pOLENode)
+                {
+                    // <object> is mapped to an image -> ignore.
+                    break;
+                }
+
                 if (SwFlyFrameFormat* pFormat
                     = dynamic_cast<SwFlyFrameFormat*>(pOLENode->GetFlyFormat()))
                 {