sw: improve duplicated images in copied header/footer text

DOCX import currently maps linked headers to 2 Writer headers, the
second header has a copy of the first header's content. The image in the
first header is named 'Picture 1', the copied header names the image as
'Image1'.

This is similar to what commit 41403fbff8140ad0ca7cf8f81d37cddcfbd19197
(sw: improve duplicated bookmarks in copied header/footer text,
2022-10-13) fixed for bookmarks, what happens is that
sw::DocumentLayoutManager::CopyLayoutFormat() clears the name of the
image, and then these are filled in at the end of the import in one
shot, to improve performance. The downside is that it's not possible for
an API user to know which was the original image and which is the copy.

Fix the problem by tweaking the in-header-footer && not-in-mail-merge
case to generate a name like 'Picture 1 Copy 1': this is meant to
preserve the lost connection between the original image and its copy,
while maintaining performance for the mail merge and body text cases
where we expect lots of images.

In the long run it would probably make sense to rather support linked
headers/footers in Writer core so we don't have to create such a copy in
the first place.

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

5 files changed, 47 insertions, 8 deletions

diff --git a/sw/inc/doc.hxx b/sw/inc/doc.hxx
index f5e868494cf6..f3662cb02b17 100644
--- a/sw/inc/doc.hxx
+++ b/sw/inc/doc.hxx
@@ -684,7 +684,7 @@ public:
     SwDBData const & GetDBData();
 
     // Some helper functions
-    OUString GetUniqueGrfName() const;
+    OUString GetUniqueGrfName(std::u16string_view rPrefix = std::u16string_view()) const;
     OUString GetUniqueOLEName() const;
     OUString GetUniqueFrameName() const;
     OUString GetUniqueShapeName() const;
diff --git a/sw/qa/core/doc/data/copy-bookmarks.docx b/sw/qa/core/doc/data/copy-bookmarks.docx
index 3fb27b430a17..a9bedb487946 100644
--- a/sw/qa/core/doc/data/copy-bookmarks.docx
+++ b/sw/qa/core/doc/data/copy-bookmarks.docx
diff --git a/sw/qa/core/doc/doc.cxx b/sw/qa/core/doc/doc.cxx
index 98548befa9ee..90e69ddc7f73 100644
--- a/sw/qa/core/doc/doc.cxx
+++ b/sw/qa/core/doc/doc.cxx
@@ -278,6 +278,24 @@ CPPUNIT_TEST_FIXTURE(SwCoreDocTest, testCopyBookmarks)
     // - Actual  : 2
     // i.e. the 2nd header had a duplicated bookmark without "Copy" in its name.
     CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(1), nActual);
+
+    // Also, when checking the # of non-copy images in the resulting doc model:
+    nActual = 0;
+    SwFrameFormats& rFrameFormats = *pDoc->GetSpzFrameFormats();
+    for (size_t i = 0; i < rFrameFormats.size(); ++i)
+    {
+        if (rFrameFormats[i]->GetName().indexOf("Copy") == -1)
+        {
+            ++nActual;
+        }
+    }
+
+    // Then make sure we have a single non-copy image, with no duplications:
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected: 1
+    // - Actual  : 2
+    // i.e. the 2nd header had a duplicated image without "Copy" in its name.
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(1), nActual);
 }
 
 CPPUNIT_TEST_FIXTURE(SwCoreDocTest, testLinkedStyleDelete)
diff --git a/sw/source/core/doc/DocumentLayoutManager.cxx b/sw/source/core/doc/DocumentLayoutManager.cxx
index bbe90610e8b9..5bd14376a0ec 100644
--- a/sw/source/core/doc/DocumentLayoutManager.cxx
+++ b/sw/source/core/doc/DocumentLayoutManager.cxx
@@ -335,15 +335,16 @@ SwFrameFormat *DocumentLayoutManager::CopyLayoutFormat(
     //                     2) anchored in a header/footer
     //                     3) anchored (to paragraph?)
     bool bMayNotCopy = false;
+    const auto pCAnchor = rNewAnchor.GetContentAnchor();
+    bool bInHeaderFooter = pCAnchor && m_rDoc.IsInHeaderFooter(pCAnchor->GetNode());
     if(bDraw)
     {
-        const auto pCAnchor = rNewAnchor.GetContentAnchor();
         bool bCheckControlLayer = false;
         rSource.CallSwClientNotify(sw::CheckDrawFrameFormatLayerHint(&bCheckControlLayer));
         bMayNotCopy =
             bCheckControlLayer &&
             ((RndStdIds::FLY_AT_PARA == rNewAnchor.GetAnchorId()) || (RndStdIds::FLY_AT_FLY  == rNewAnchor.GetAnchorId()) || (RndStdIds::FLY_AT_CHAR == rNewAnchor.GetAnchorId())) &&
-            pCAnchor && m_rDoc.IsInHeaderFooter(pCAnchor->GetNode());
+            bInHeaderFooter;
     }
 
     // just return if we can't copy this
@@ -393,7 +394,7 @@ SwFrameFormat *DocumentLayoutManager::CopyLayoutFormat(
 
         if( !m_rDoc.IsCopyIsMove() || &m_rDoc != pSrcDoc )
         {
-            if( m_rDoc.IsInReading() || m_rDoc.IsInMailMerge() )
+            if( (m_rDoc.IsInReading() && !bInHeaderFooter) || m_rDoc.IsInMailMerge() )
                 pDest->SetFormatName( OUString() );
             else
             {
@@ -405,7 +406,7 @@ SwFrameFormat *DocumentLayoutManager::CopyLayoutFormat(
                 if( m_rDoc.FindFlyByName( sOld, nNdTyp ) )     // found one
                     switch( nNdTyp )
                     {
-                    case SwNodeType::Grf:    sOld = m_rDoc.GetUniqueGrfName();      break;
+                    case SwNodeType::Grf:    sOld = m_rDoc.GetUniqueGrfName(sOld);  break;
                     case SwNodeType::Ole:    sOld = m_rDoc.GetUniqueOLEName();      break;
                     default:                 sOld = m_rDoc.GetUniqueFrameName();    break;
                     }
diff --git a/sw/source/core/doc/doclay.cxx b/sw/source/core/doc/doclay.cxx
index e83e6173b4d1..b357bfed522c 100644
--- a/sw/source/core/doc/doclay.cxx
+++ b/sw/source/core/doc/doclay.cxx
@@ -1320,7 +1320,7 @@ namespace
     }
 }
 
-static OUString lcl_GetUniqueFlyName(const SwDoc& rDoc, TranslateId pDefStrId, sal_uInt16 eType)
+static OUString lcl_GetUniqueFlyName(const SwDoc& rDoc, TranslateId pDefStrId, sal_uInt16 eType, std::u16string_view rPrefix = std::u16string_view(), SwNodeType nNdTyp = SwNodeType::NONE)
 {
     assert(eType >= RES_FMT_BEGIN && eType < RES_FMT_END);
     if (rDoc.IsInMailMerge())
@@ -1331,6 +1331,26 @@ static OUString lcl_GetUniqueFlyName(const SwDoc& rDoc, TranslateId pDefStrId, s
         return newName;
     }
 
+    if (!rPrefix.empty())
+    {
+        // Generate a name that makes it possible to know this is a copy of which original name,
+        // e.g. 'Picture 1 Copy 1'.
+        assert(nNdTyp != SwNodeType::NONE);
+        sal_Int32 nCnt = 1;
+        OUString aPrefix = SwResId(STR_MARK_COPY).replaceFirst("%1", rPrefix);
+        OUString aTmp;
+        while(nCnt < SAL_MAX_INT32)
+        {
+            aTmp = aPrefix + OUString::number(nCnt);
+            ++nCnt;
+            if (!rDoc.FindFlyByName(aTmp, nNdTyp))
+            {
+                break;
+            }
+        }
+        return aTmp;
+    }
+
     OUString aName(SwResId(pDefStrId));
     sal_Int32 nNmLen = aName.getLength();
 
@@ -1360,9 +1380,9 @@ static OUString lcl_GetUniqueFlyName(const SwDoc& rDoc, TranslateId pDefStrId, s
     return aName + OUString::number(nNum);
 }
 
-OUString SwDoc::GetUniqueGrfName() const
+OUString SwDoc::GetUniqueGrfName(std::u16string_view rPrefix) const
 {
-    return lcl_GetUniqueFlyName(*this, STR_GRAPHIC_DEFNAME, RES_FLYFRMFMT);
+    return lcl_GetUniqueFlyName(*this, STR_GRAPHIC_DEFNAME, RES_FLYFRMFMT, rPrefix, SwNodeType::Grf);
 }
 
 OUString SwDoc::GetUniqueOLEName() const