From 100c914d44ae8f362924fe567d7d41d0033ae8ad Mon Sep 17 00:00:00 2001
From: Miklos Vajna <vmiklos@collabora.com>
Date: Tue, 22 Nov 2022 14:30:08 +0100
Subject: DOCX filter: fix <w:id> creating both grab-bag and content control
 for <w:sdt>

Exporting the DOCX bugdoc back to DOCX resulted in a document that can't
be opened in Word.

Examining the output, the problem is that the document had 2 inline
<w:sdt> elements with <w:id>, and we mapped such <w:sdt> elements to
both grab-bags and content controls, leading to duplicate <w:sdt>
elements on export. This is schema-valid, but it goes against the
intention of the spec and Word can't open it.

The initial fix was just a writerfilter/ tweak to avoid grab-bagging
<w:id> for inline <w:sdt>, but CppunitTest_sw_ooxmlexport4's
testSimpleSdts points out that in other cases we already require
preserving <w:id>. Fix the problem by storing <w:id> in the content
control, which is essentially a subset of
<https://gerrit.libreoffice.org/c/core/+/142818>.

Thanks to Justin Luth! - he prototyped the DOCX filter for <w:id>.

Change-Id: I9f002b770049ce8be30253d0b39410d9a58981dd
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/143117
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins
---
 writerfilter/source/dmapper/DomainMapper.cxx      | 6 ++++++
 writerfilter/source/dmapper/DomainMapper_Impl.cxx | 5 +++++
 writerfilter/source/dmapper/SdtHelper.cxx         | 5 +++++
 writerfilter/source/dmapper/SdtHelper.hxx         | 6 ++++++
 4 files changed, 22 insertions(+)

(limited to 'writerfilter/source')

diff --git a/writerfilter/source/dmapper/DomainMapper.cxx b/writerfilter/source/dmapper/DomainMapper.cxx
index b5e45b7c2734..94422d8a6a98 100644
--- a/writerfilter/source/dmapper/DomainMapper.cxx
+++ b/writerfilter/source/dmapper/DomainMapper.cxx
@@ -2865,6 +2865,12 @@ void DomainMapper::sprmWithProps( Sprm& rSprm, const PropertyMapPtr& rContext )
                 break;
             }
 
+            if (nSprmId == NS_ooxml::LN_CT_SdtPr_id)
+            {
+                m_pImpl->m_pSdtHelper->SetId(nIntValue);
+                break;
+            }
+
             if (nSprmId == NS_ooxml::LN_CT_SdtPr_checkbox)
             {
                 m_pImpl->m_pSdtHelper->setControlType(SdtControlType::checkBox);
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index 3cae3962811e..6477e58bbdaf 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -950,6 +950,11 @@ void DomainMapper_Impl::PopSdt()
                                                uno::Any(m_pSdtHelper->GetTag()));
     }
 
+    if (m_pSdtHelper->GetId())
+    {
+        xContentControlProps->setPropertyValue("Id", uno::Any(m_pSdtHelper->GetId()));
+    }
+
     if (m_pSdtHelper->getControlType() == SdtControlType::checkBox)
     {
         xContentControlProps->setPropertyValue("Checkbox", uno::Any(true));
diff --git a/writerfilter/source/dmapper/SdtHelper.cxx b/writerfilter/source/dmapper/SdtHelper.cxx
index 6f08c81559cf..a6a6b6cb08f8 100644
--- a/writerfilter/source/dmapper/SdtHelper.cxx
+++ b/writerfilter/source/dmapper/SdtHelper.cxx
@@ -520,6 +520,7 @@ void SdtHelper::clear()
     m_aColor.clear();
     m_aAlias.clear();
     m_aTag.clear();
+    m_nId = 0;
 }
 
 void SdtHelper::SetPlaceholderDocPart(const OUString& rPlaceholderDocPart)
@@ -541,6 +542,10 @@ void SdtHelper::SetTag(const OUString& rTag) { m_aTag = rTag; }
 
 const OUString& SdtHelper::GetTag() const { return m_aTag; }
 
+void SdtHelper::SetId(sal_Int32 nId) { m_nId = nId; }
+
+sal_Int32 SdtHelper::GetId() const { return m_nId; }
+
 } // namespace writerfilter::dmapper
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/writerfilter/source/dmapper/SdtHelper.hxx b/writerfilter/source/dmapper/SdtHelper.hxx
index dc1a3172e364..736476e63c2f 100644
--- a/writerfilter/source/dmapper/SdtHelper.hxx
+++ b/writerfilter/source/dmapper/SdtHelper.hxx
@@ -132,6 +132,9 @@ class SdtHelper final : public virtual SvRefBase
     /// <w:sdtPr>'s <w:tag w:val="...">.
     OUString m_aTag;
 
+    /// <w:sdtPr>'s <w:id w:val="...">.
+    sal_Int32 m_nId = 0;
+
 public:
     explicit SdtHelper(DomainMapper_Impl& rDM_Impl,
                        css::uno::Reference<css::uno::XComponentContext> xContext);
@@ -217,6 +220,9 @@ public:
     void SetTag(const OUString& rTag);
     const OUString& GetTag() const;
 
+    void SetId(sal_Int32 nId);
+    sal_Int32 GetId() const;
+
     std::optional<OUString> getValueFromDataBinding();
 };
 
-- 
cgit