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(); };