tdf#100033: Frames with the same name are removed

Allow to have frames with the same name. For removing
real duplicated frames (generated by LO earlier)
check other things also next to the frame name:
position, size or whether the two frames are anchored
to the same position.

Reviewed-on: https://gerrit.libreoffice.org/37702
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Tamás Zolnai <tamas.zolnai@collabora.com>
(cherry picked from commit 6952d696439981962ad378aa28b0d16ea6e48f0e)

Change-Id: I191ae5128d0228eb85f78f065b44b1f0b3ba6dcf
Reviewed-on: https://gerrit.libreoffice.org/37706
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Miklos Vajna <vmiklos@collabora.co.uk>

6 files changed, 112 insertions, 9 deletions

diff --git a/include/xmloff/txtimp.hxx b/include/xmloff/txtimp.hxx
index cab9f53653e4..ceedf377d9a2 100644
--- a/include/xmloff/txtimp.hxx
+++ b/include/xmloff/txtimp.hxx
@@ -536,6 +536,11 @@ public:
         GetChapterNumbering() const;
 
     bool HasFrameByName( const OUString& rName ) const;
+
+    bool IsDuplicateFrame(const OUString& sName, sal_Int32 nX, sal_Int32 nY, sal_Int32 nWidth, sal_Int32 nHeight) const;
+    void StoreLastImportedFrameName(const OUString& rName);
+    void ClearLastImportedTextFrameName();
+
     void ConnectFrameChains( const OUString& rFrmName,
         const OUString& rNextFrmName,
         const css::uno::Reference< css::beans::XPropertySet >& rFrmPropSet );
diff --git a/sw/qa/extras/odfimport/data/tdf100033_1.odt b/sw/qa/extras/odfimport/data/tdf100033_1.odt
new file mode 100755
index 000000000000..b7f3ae7aaeb4
--- /dev/null
+++ b/sw/qa/extras/odfimport/data/tdf100033_1.odt
diff --git a/sw/qa/extras/odfimport/data/tdf100033_2.odt b/sw/qa/extras/odfimport/data/tdf100033_2.odt
new file mode 100755
index 000000000000..98ae7bd6b31d
--- /dev/null
+++ b/sw/qa/extras/odfimport/data/tdf100033_2.odt
diff --git a/sw/qa/extras/odfimport/odfimport.cxx b/sw/qa/extras/odfimport/odfimport.cxx
index 9c21cad377b7..c262f9cf72e9 100644
--- a/sw/qa/extras/odfimport/odfimport.cxx
+++ b/sw/qa/extras/odfimport/odfimport.cxx
@@ -761,5 +761,21 @@ DECLARE_ODFIMPORT_TEST(testTdf75221, "tdf75221.odt")
     CPPUNIT_ASSERT(top.toInt32() > 0);
 }
 
+DECLARE_ODFIMPORT_TEST(testTdf100033_1, "tdf100033_1.odt")
+{
+    // Test document have three duplicated frames with the same name and position/size -> import one frame
+    uno::Reference<text::XTextFramesSupplier> xTextFramesSupplier(mxComponent, uno::UNO_QUERY);
+    uno::Reference<container::XIndexAccess> xIndexAccess(xTextFramesSupplier->getTextFrames(), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), xIndexAccess->getCount());
+}
+
+DECLARE_ODFIMPORT_TEST(testTdf100033_2, "tdf100033_2.odt")
+{
+    // Test document have three different frames anchored to different paragraphs -> import all frames
+    uno::Reference<text::XTextFramesSupplier> xTextFramesSupplier(mxComponent, uno::UNO_QUERY);
+    uno::Reference<container::XIndexAccess> xIndexAccess(xTextFramesSupplier->getTextFrames(), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(3), xIndexAccess->getCount());
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/xmloff/source/text/XMLTextFrameContext.cxx b/xmloff/source/text/XMLTextFrameContext.cxx
index 2d72c1758092..136ed2db8998 100644
--- a/xmloff/source/text/XMLTextFrameContext.cxx
+++ b/xmloff/source/text/XMLTextFrameContext.cxx
@@ -410,6 +410,7 @@ class XMLTextFrameContext_Impl : public SvXMLImportContext
     bool    bSyncHeight : 1;
     bool    bCreateFailed : 1;
     bool    bOwnBase64Stream : 1;
+    bool    mbMultipleContent : 1; // This context is created based on a multiple content (image)
 
     void Create( bool bHRefOrBase64 );
 
@@ -425,7 +426,8 @@ public:
             const css::uno::Reference<css::xml::sax::XAttributeList > & rAttrList,
             css::text::TextContentAnchorType eAnchorType,
             sal_uInt16 nType,
-            const css::uno::Reference<css::xml::sax::XAttributeList > & rFrameAttrList );
+            const css::uno::Reference<css::xml::sax::XAttributeList > & rFrameAttrList,
+            bool bMultipleContent = false );
     virtual ~XMLTextFrameContext_Impl() override;
 
     virtual void EndElement() override;
@@ -448,6 +450,8 @@ public:
 
     void SetName();
 
+    const OUString& GetOrigName() const { return m_sOrigName; }
+
     css::text::TextContentAnchorType GetAnchorType() const { return eAnchorType; }
 
     const css::uno::Reference < css::beans::XPropertySet >& GetPropSet() const { return xPropSet; }
@@ -563,6 +567,14 @@ void XMLTextFrameContext_Impl::Create( bool /*bHRefOrBase64*/ )
 
     Reference< XPropertySetInfo > xPropSetInfo = xPropSet->getPropertySetInfo();
 
+    // Skip duplicated frames
+    if(!mbMultipleContent && // It's allowed to have multiple image for the same frame
+       xTextImportHelper->IsDuplicateFrame(sName, nX, nY, nWidth, nHeight))
+    {
+        bCreateFailed = true;
+        return;
+    }
+
     // set name
     Reference < XNamed > xNamed( xPropSet, UNO_QUERY );
     if( xNamed.is() )
@@ -582,14 +594,9 @@ void XMLTextFrameContext_Impl::Create( bool /*bHRefOrBase64*/ )
             xNamed->setName( sName );
             if( sName != sOldName )
             {
-                bool bSuccess = xTextImportHelper->GetRenameMap().Add( XML_TEXT_RENAME_TYPE_FRAME,
+                xTextImportHelper->GetRenameMap().Add( XML_TEXT_RENAME_TYPE_FRAME,
                                              sOldName, sName );
 
-                if (!bSuccess && !sOldName.isEmpty())
-                {
-                    bCreateFailed = true;
-                    return;
-                }
             }
         }
     }
@@ -825,7 +832,8 @@ XMLTextFrameContext_Impl::XMLTextFrameContext_Impl(
         const Reference< XAttributeList > & rAttrList,
         TextContentAnchorType eATyp,
         sal_uInt16 nNewType,
-        const Reference< XAttributeList > & rFrameAttrList )
+        const Reference< XAttributeList > & rFrameAttrList,
+        bool bMultipleContent )
 :   SvXMLImportContext( rImport, nPrfx, rLName )
 ,   mbListContextPushed( false )
 ,   sWidth("Width")
@@ -870,6 +878,7 @@ XMLTextFrameContext_Impl::XMLTextFrameContext_Impl(
     bSyncHeight = false;
     bCreateFailed = false;
     bOwnBase64Stream = false;
+    mbMultipleContent = bMultipleContent;
 
     rtl::Reference < XMLTextImportHelper > xTxtImport =
         GetImport().GetTextImport();
@@ -1427,6 +1436,8 @@ void XMLTextFrameContext::EndElement()
             m_pHyperlink.reset();
         }
 
+        GetImport().GetTextImport()->StoreLastImportedFrameName(pImpl->GetOrigName());
+
     }
 }
 
@@ -1530,7 +1541,7 @@ SvXMLImportContext *XMLTextFrameContext::CreateChildContext(
         // read another image
         pContext = new XMLTextFrameContext_Impl(
             GetImport(), p_nPrefix, rLocalName, xAttrList,
-            m_eDefaultAnchorType, XML_TEXT_FRAME_GRAPHIC, m_xAttrList);
+            m_eDefaultAnchorType, XML_TEXT_FRAME_GRAPHIC, m_xAttrList, true);
 
         m_xImplContext = pContext;
         addContent(*m_xImplContext.get());
diff --git a/xmloff/source/text/txtimp.cxx b/xmloff/source/text/txtimp.cxx
index f4b66c854518..8029ddfd075b 100644
--- a/xmloff/source/text/txtimp.cxx
+++ b/xmloff/source/text/txtimp.cxx
@@ -537,6 +537,9 @@ struct XMLTextImportHelper::Impl
     /// name of the last 'open' redline that started between paragraphs
     OUString m_sOpenRedlineIdentifier;
 
+    // Used for frame deduplication, the name of the last frame imported directly before the current one
+    OUString msLastImportedFrameName;
+
     uno::Reference<text::XText> m_xText;
     uno::Reference<text::XTextCursor> m_xCursor;
     uno::Reference<text::XTextRange> m_xCursorAsRange;
@@ -1108,6 +1111,72 @@ bool XMLTextImportHelper::HasFrameByName( const OUString& rName ) const
             m_xImpl->m_xObjects->hasByName(rName));
 }
 
+bool XMLTextImportHelper::IsDuplicateFrame(const OUString& sName, sal_Int32 nX, sal_Int32 nY, sal_Int32 nWidth, sal_Int32 nHeight) const
+{
+    if (HasFrameByName(sName))
+    {
+        uno::Reference<beans::XPropertySet> xOtherFrame;
+        if(m_xImpl->m_xTextFrames.is() && m_xImpl->m_xTextFrames->hasByName(sName))
+            xOtherFrame.set(m_xImpl->m_xTextFrames->getByName(sName), uno::UNO_QUERY);
+        else if(m_xImpl->m_xGraphics.is() && m_xImpl->m_xGraphics->hasByName(sName))
+            xOtherFrame.set(m_xImpl->m_xGraphics->getByName(sName), uno::UNO_QUERY);
+        else if (m_xImpl->m_xObjects.is() && m_xImpl->m_xObjects->hasByName(sName))
+            xOtherFrame.set(m_xImpl->m_xObjects->getByName(sName), uno::UNO_QUERY);
+
+        Reference< XPropertySetInfo > xPropSetInfo = xOtherFrame->getPropertySetInfo();
+        if(xPropSetInfo->hasPropertyByName("Width"))
+        {
+            sal_Int32 nOtherWidth = 0;
+            xOtherFrame->getPropertyValue("Width") >>= nOtherWidth;
+            if(nWidth != nOtherWidth)
+                return false;
+        }
+
+        if (xPropSetInfo->hasPropertyByName("Height"))
+        {
+            sal_Int32 nOtherHeight = 0;
+            xOtherFrame->getPropertyValue("Height") >>= nOtherHeight;
+            if (nHeight != nOtherHeight)
+                return false;
+        }
+
+        if (xPropSetInfo->hasPropertyByName("HoriOrientPosition"))
+        {
+            sal_Int32 nOtherX = 0;
+            xOtherFrame->getPropertyValue("HoriOrientPosition") >>= nOtherX;
+            if (nX != nOtherX)
+                return false;
+        }
+
+        if (xPropSetInfo->hasPropertyByName("VertOrientPosition"))
+        {
+            sal_Int32 nOtherY = 0;
+            xOtherFrame->getPropertyValue("VertOrientPosition") >>= nOtherY;
+            if (nY != nOtherY)
+                return false;
+        }
+
+        // In some case, position is not defined for frames, so check whether the two frames follow each other (are anchored to the same position)
+        if (m_xImpl->msLastImportedFrameName != sName)
+        {
+            return false;
+        }
+
+        return true;
+    }
+    return false;
+}
+
+void XMLTextImportHelper::StoreLastImportedFrameName(const OUString& rName)
+{
+    m_xImpl->msLastImportedFrameName = rName;
+}
+
+void XMLTextImportHelper::ClearLastImportedTextFrameName()
+{
+    m_xImpl->msLastImportedFrameName.clear();
+}
+
 void XMLTextImportHelper::InsertString( const OUString& rChars )
 {
     assert(m_xImpl->m_xText.is());
@@ -2342,6 +2411,8 @@ SvXMLImportContext *XMLTextImportHelper::CreateTextChildContext(
         m_xImpl->m_bBodyContentStarted = false;
     }
 
+    if( nToken != XML_TOK_TEXT_FRAME_PAGE )
+        ClearLastImportedTextFrameName();
     return pContext;
 }