offapi/com/sun/star/text/ContentControl.idl          |    6 +++
 sw/inc/formatcontentcontrol.hxx                      |    7 ++++
 sw/inc/unoprnms.hxx                                  |    1 
 sw/qa/extras/ooxmlexport/data/sdt-duplicated-id.docx |binary
 sw/qa/extras/ooxmlexport/ooxmlexport18.cxx           |   15 +++++++++
 sw/source/core/txtnode/attrcontentcontrol.cxx        |    2 +
 sw/source/core/unocore/unocontentcontrol.cxx         |   29 +++++++++++++++++++
 sw/source/core/unocore/unomap1.cxx                   |    1 
 sw/source/filter/ww8/docxattributeoutput.cxx         |    6 +++
 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 +++
 13 files changed, 89 insertions(+)

New commits:
commit 7baaec9e1683f8683498da7e9153e737fe111671
Author:     Miklos Vajna <vmik...@collabora.com>
AuthorDate: Tue Nov 22 14:30:08 2022 +0100
Commit:     Miklos Vajna <vmik...@collabora.com>
CommitDate: Thu Nov 24 08:10:08 2022 +0100

    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>.
    
    (cherry picked from commit 100c914d44ae8f362924fe567d7d41d0033ae8ad)
    
    Conflicts:
            sw/qa/extras/ooxmlexport/ooxmlexport18.cxx
    
    Change-Id: I9f002b770049ce8be30253d0b39410d9a58981dd
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/143139
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
    Reviewed-by: Miklos Vajna <vmik...@collabora.com>

diff --git a/offapi/com/sun/star/text/ContentControl.idl 
b/offapi/com/sun/star/text/ContentControl.idl
index 7bdb39fa5101..6abcc79fd204 100644
--- a/offapi/com/sun/star/text/ContentControl.idl
+++ b/offapi/com/sun/star/text/ContentControl.idl
@@ -121,6 +121,12 @@ service ContentControl
         @since LibreOffice 7.5
     */
     [optional, property, readonly] string DateString;
+
+    /** A unique numeric id, used by macros to identify a specific control.
+
+        @since LibreOffice 7.5
+    */
+    [optional, property] long Id;
 };
 
 
diff --git a/sw/inc/formatcontentcontrol.hxx b/sw/inc/formatcontentcontrol.hxx
index a44c8b9b5a44..9ae6205e87b3 100644
--- a/sw/inc/formatcontentcontrol.hxx
+++ b/sw/inc/formatcontentcontrol.hxx
@@ -175,6 +175,9 @@ class SW_DLLPUBLIC SwContentControl : public 
sw::BroadcastingModify
     /// The tag: just remembered.
     OUString m_aTag;
 
+    /// The id: just remembered.
+    sal_Int32 m_nId = 0;
+
     /// Stores a list item index, in case the doc model is not yet updated.
     std::optional<size_t> m_oSelectedListItem;
 
@@ -344,6 +347,10 @@ public:
 
     const OUString& GetTag() const { return m_aTag; }
 
+    void SetId(sal_Int32 nId) { m_nId = nId; }
+
+    sal_Int32 GetId() const { return m_nId; }
+
     void SetReadWrite(bool bReadWrite) { m_bReadWrite = bReadWrite; }
 
     bool GetReadWrite() const { return m_bReadWrite; }
diff --git a/sw/inc/unoprnms.hxx b/sw/inc/unoprnms.hxx
index 37069b07925f..2dffbeb739af 100644
--- a/sw/inc/unoprnms.hxx
+++ b/sw/inc/unoprnms.hxx
@@ -890,6 +890,7 @@
 #define UNO_NAME_COLOR "Color"
 #define UNO_NAME_ALIAS "Alias"
 #define UNO_NAME_TAG "Tag"
+#define UNO_NAME_ID "Id"
 #define UNO_NAME_DATE_STRING "DateString"
 #endif
 
diff --git a/sw/qa/extras/ooxmlexport/data/sdt-duplicated-id.docx 
b/sw/qa/extras/ooxmlexport/data/sdt-duplicated-id.docx
new file mode 100644
index 000000000000..d21894df3007
Binary files /dev/null and 
b/sw/qa/extras/ooxmlexport/data/sdt-duplicated-id.docx differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx 
b/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx
index 8d647e9cf234..bc48cc158975 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx
@@ -62,6 +62,21 @@ CPPUNIT_TEST_FIXTURE(Test, testInlineSdtHeader)
     loadAndSave("inline-sdt-header.docx");
 }
 
+CPPUNIT_TEST_FIXTURE(Test, testSdtDuplicatedId)
+{
+    // Given a document with 2 inline <w:sdt>, with each a <w:id>:
+    // When exporting that back to DOCX:
+    loadAndSave("sdt-duplicated-id.docx");
+
+    // Then make sure we write 2 <w:sdt> and no duplicates:
+    xmlDocUniquePtr pXmlDoc = parseExport("word/document.xml");
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected: 2
+    // - Actual  : 4
+    // i.e. grab-bags introduced 2 unwanted duplicates.
+    assertXPath(pXmlDoc, "//w:sdt", 2);
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/txtnode/attrcontentcontrol.cxx 
b/sw/source/core/txtnode/attrcontentcontrol.cxx
index 7300ecfdf265..836b9a638c8f 100644
--- a/sw/source/core/txtnode/attrcontentcontrol.cxx
+++ b/sw/source/core/txtnode/attrcontentcontrol.cxx
@@ -429,6 +429,8 @@ void SwContentControl::dumpAsXml(xmlTextWriterPtr pWriter) 
const
     (void)xmlTextWriterWriteAttribute(pWriter, BAD_CAST("alias"),
                                       BAD_CAST(m_aAlias.toUtf8().getStr()));
     (void)xmlTextWriterWriteAttribute(pWriter, BAD_CAST("tag"), 
BAD_CAST(m_aTag.toUtf8().getStr()));
+    (void)xmlTextWriterWriteAttribute(pWriter, BAD_CAST("id"),
+                                      
BAD_CAST(OString::number(m_nId).getStr()));
 
     if (!m_aListItems.empty())
     {
diff --git a/sw/source/core/unocore/unocontentcontrol.cxx 
b/sw/source/core/unocore/unocontentcontrol.cxx
index 827a6c4568ae..61c7ac66c9fd 100644
--- a/sw/source/core/unocore/unocontentcontrol.cxx
+++ b/sw/source/core/unocore/unocontentcontrol.cxx
@@ -177,6 +177,7 @@ public:
     OUString m_aColor;
     OUString m_aAlias;
     OUString m_aTag;
+    sal_Int32 m_nId;
 
     Impl(SwXContentControl& rThis, SwDoc& rDoc, SwContentControl* 
pContentControl,
          const uno::Reference<text::XText>& xParentText,
@@ -195,6 +196,7 @@ public:
         , m_bPlainText(false)
         , m_bComboBox(false)
         , m_bDropDown(false)
+        , m_nId(0)
     {
         if (m_pContentControl)
         {
@@ -553,6 +555,7 @@ void SwXContentControl::AttachImpl(const 
uno::Reference<text::XTextRange>& xText
     pContentControl->SetColor(m_pImpl->m_aColor);
     pContentControl->SetAlias(m_pImpl->m_aAlias);
     pContentControl->SetTag(m_pImpl->m_aTag);
+    pContentControl->SetId(m_pImpl->m_nId);
 
     SwFormatContentControl aContentControl(pContentControl, nWhich);
     bool bSuccess
@@ -1028,6 +1031,21 @@ void SAL_CALL SwXContentControl::setPropertyValue(const 
OUString& rPropertyName,
             }
         }
     }
+    else if (rPropertyName == UNO_NAME_ID)
+    {
+        sal_Int32 nValue = 0;
+        if (rValue >>= nValue)
+        {
+            if (m_pImpl->m_bIsDescriptor)
+            {
+                m_pImpl->m_nId = nValue;
+            }
+            else
+            {
+                m_pImpl->m_pContentControl->SetId(nValue);
+            }
+        }
+    }
     else
     {
         throw beans::UnknownPropertyException();
@@ -1279,6 +1297,17 @@ uno::Any SAL_CALL 
SwXContentControl::getPropertyValue(const OUString& rPropertyN
             aRet <<= m_pImpl->m_pContentControl->GetDateString();
         }
     }
+    else if (rPropertyName == UNO_NAME_ID)
+    {
+        if (m_pImpl->m_bIsDescriptor)
+        {
+            aRet <<= m_pImpl->m_nId;
+        }
+        else
+        {
+            aRet <<= m_pImpl->m_pContentControl->GetId();
+        }
+    }
     else
     {
         throw beans::UnknownPropertyException();
diff --git a/sw/source/core/unocore/unomap1.cxx 
b/sw/source/core/unocore/unomap1.cxx
index a584d1f59952..5d4d0c0b1a77 100644
--- a/sw/source/core/unocore/unomap1.cxx
+++ b/sw/source/core/unocore/unomap1.cxx
@@ -1047,6 +1047,7 @@ const SfxItemPropertyMapEntry* 
SwUnoPropertyMapProvider::GetContentControlProper
         { u"" UNO_NAME_COLOR, 0, cppu::UnoType<OUString>::get(), 
PROPERTY_NONE, 0 },
         { u"" UNO_NAME_ALIAS, 0, cppu::UnoType<OUString>::get(), 
PROPERTY_NONE, 0 },
         { u"" UNO_NAME_TAG, 0, cppu::UnoType<OUString>::get(), PROPERTY_NONE, 
0 },
+        { u"" UNO_NAME_ID, 0, cppu::UnoType<sal_Int32>::get(), PROPERTY_NONE, 
0 },
         { u"" UNO_NAME_DATE_STRING, 0, cppu::UnoType<OUString>::get(), 
PropertyAttribute::READONLY, 0 },
         { u"", 0, css::uno::Type(), 0, 0 }
     };
diff --git a/sw/source/filter/ww8/docxattributeoutput.cxx 
b/sw/source/filter/ww8/docxattributeoutput.cxx
index 1fc10fdbd332..506f62328947 100644
--- a/sw/source/filter/ww8/docxattributeoutput.cxx
+++ b/sw/source/filter/ww8/docxattributeoutput.cxx
@@ -2375,6 +2375,12 @@ void DocxAttributeOutput::WriteContentControlStart()
                                        m_pContentControl->GetTag());
     }
 
+    if (m_pContentControl->GetId())
+    {
+        m_pSerializer->singleElementNS(XML_w, XML_id, FSNS(XML_w, XML_val),
+                                       
OString::number(m_pContentControl->GetId()));
+    }
+
     if (m_pContentControl->GetShowingPlaceHolder())
     {
         m_pSerializer->singleElementNS(XML_w, XML_showingPlcHdr);
diff --git a/writerfilter/source/dmapper/DomainMapper.cxx 
b/writerfilter/source/dmapper/DomainMapper.cxx
index ba24f246d87b..82a6264d4b8b 100644
--- a/writerfilter/source/dmapper/DomainMapper.cxx
+++ b/writerfilter/source/dmapper/DomainMapper.cxx
@@ -2837,6 +2837,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 fe4b6c51b6ac..c97e7229e829 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -943,6 +943,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::makeAny(true));
diff --git a/writerfilter/source/dmapper/SdtHelper.cxx 
b/writerfilter/source/dmapper/SdtHelper.cxx
index 540969b1e933..3e8811c69e92 100644
--- a/writerfilter/source/dmapper/SdtHelper.cxx
+++ b/writerfilter/source/dmapper/SdtHelper.cxx
@@ -459,6 +459,7 @@ void SdtHelper::clear()
     m_aColor.clear();
     m_aAlias.clear();
     m_aTag.clear();
+    m_nId = 0;
 }
 
 void SdtHelper::SetPlaceholderDocPart(const OUString& rPlaceholderDocPart)
@@ -480,6 +481,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 73504c7024d7..201441547208 100644
--- a/writerfilter/source/dmapper/SdtHelper.hxx
+++ b/writerfilter/source/dmapper/SdtHelper.hxx
@@ -131,6 +131,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> const& 
xContext);
@@ -216,6 +219,9 @@ public:
     void SetTag(const OUString& rTag);
     const OUString& GetTag() const;
 
+    void SetId(sal_Int32 nId);
+    sal_Int32 GetId() const;
+
     std::optional<OUString> getValueFromDataBinding();
 };
 

Reply via email to