offapi/com/sun/star/text/ContentControl.idl | 2 schema/libreoffice/OpenDocument-v1.3+libreoffice-schema.rng | 5 + sw/qa/core/unocore/unocore.cxx | 2 sw/qa/extras/ooxmlexport/ooxmlexport17.cxx | 2 sw/source/filter/ww8/docxattributeoutput.cxx | 26 ++++------ sw/source/filter/ww8/docxattributeoutput.hxx | 4 - writerfilter/qa/cppunittests/dmapper/SdtHelper.cxx | 4 + writerfilter/qa/cppunittests/dmapper/data/sdt-run-rich-text.docx |binary writerfilter/source/dmapper/DomainMapper.cxx | 4 + xmloff/qa/unit/data/content-control-alias.fodt | 2 xmloff/qa/unit/text.cxx | 5 + xmloff/source/text/txtparae.cxx | 7 ++ xmloff/source/text/xmlcontentcontrolcontext.cxx | 13 +++++ xmloff/source/text/xmlcontentcontrolcontext.hxx | 1 14 files changed, 59 insertions(+), 18 deletions(-)
New commits: commit 7b279ffa22a5c2080e31e6cac534ccc048229e6e Author: Justin Luth <justin.l...@collabora.com> AuthorDate: Fri Dec 2 14:59:08 2022 -0500 Commit: Miklos Vajna <vmik...@collabora.com> CommitDate: Wed Dec 21 07:57:08 2022 +0000 sw content controls: enhance preserve id Squashed commit including NFC cleanup docxattributeoutput: use existing function to clear fd14dd0e4c6a7ea0866a686f653db90301d664b6 Follow-up to 100c914d44ae8f362924fe567d7d41d0033ae8ad which added the initial id preservation for DOCX. adding DOCX block SDT grabbaging, ODF import/export [content controls can't exist in DOC format] The ID field is CRITICAL to preserve for VBA purposes. This patch adjusts BlockSDT to also round-trip the id instead of just creating a random one. m_nRunSdtPrToken <never equals> FSNS(XML_w, XML_id) since 2021 with 5f3af56b2c0ef6c628a7cfe5ce6e86f8e1765f5f, so I removed that part of the clause. I had thought about changing the ID to use a string instead of an int, but then the integer version was adopted to fix a regression in the commit mentioned earlier. I think it is AWFUL to have a number as the identifier when it will be used in StarBASIC. The VBA guys have to deal with it, but it would be nice to do something reasonable for LO native access to content controls. However, the concern would be if we allow VBA macro content created in LO to be exported to DOCX format - that would cause problems converting from a string ID to a number ID. VBA editing already is happening to some extent, and mmeeks seems interested in enabling it. So over-all it seems best to just stick with an integer ID. I used the commits for <w:alias> and <w:tag> to compose this patch. XML_ID already existed in include/xmloff/xmltoken.hxx and "id" already exists in xmloff/source/token/tokens.txt The ID can be used in VBA to select a specific control. The id (which is a positive or negative integer in DOCX) specifies a unique control - either by passing the number as a string (of the UNSIGNED value) or by passing as a float (specified with #). For example: msgbox ActiveDocument.ContentControls(2587720202#).ID msgbox ActiveDocument.ContentControls("2587720202").ID but not as an integer since that is used for index access. dim index as integer index = 1 msgbox ActiveDocument.ContentControls(index).ID make CppunitTest_writerfilter_dmapper CPPUNIT_TEST_NAME=testSdtRunRichText make CppunitTest_sw_ooxmlexport17 CPPUNIT_TEST_NAME=testDateContentControlExport make CppunitTest_sw_core_unocore CPPUNIT_TEST_NAME=testContentControlDate make CppunitTest_xmloff_text CPPUNIT_TEST_NAME=testAliasContentControlExport make CppunitTest_xmloff_text CPPUNIT_TEST_NAME=testAliasContentControlImport Change-Id: I5c4022dc932d941fad9da6d75ce899ee1ff66ff5 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/142818 Tested-by: Jenkins Reviewed-by: Justin Luth <jl...@mail.com> Reviewed-by: Miklos Vajna <vmik...@collabora.com> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144625 Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> diff --git a/offapi/com/sun/star/text/ContentControl.idl b/offapi/com/sun/star/text/ContentControl.idl index ce741d9b2926..cb980a24b0ff 100644 --- a/offapi/com/sun/star/text/ContentControl.idl +++ b/offapi/com/sun/star/text/ContentControl.idl @@ -105,12 +105,14 @@ service ContentControl [optional, property] boolean DropDown; /** The alias: kind of a human-readable title / description, show up on the UI. + -also used by VBA to group controls into a smaller, indexed collection @since LibreOffice 7.5 */ [optional, property] string Alias; /** The tag: similar to Alias, but is meant to be machine-readable. + -also used by VBA to group controls into a smaller, indexed collection @since LibreOffice 7.5 */ diff --git a/schema/libreoffice/OpenDocument-v1.3+libreoffice-schema.rng b/schema/libreoffice/OpenDocument-v1.3+libreoffice-schema.rng index ff0019c1bc50..f2853d8e2bb9 100644 --- a/schema/libreoffice/OpenDocument-v1.3+libreoffice-schema.rng +++ b/schema/libreoffice/OpenDocument-v1.3+libreoffice-schema.rng @@ -2944,6 +2944,11 @@ xmlns:loext="urn:org:documentfoundation:names:experimental:office:xmlns:loext:1. <rng:ref name="string"/> </rng:attribute> </rng:optional> + <rng:optional> + <rng:attribute name="loext:id"> + <rng:ref name="integer"/> + </rng:attribute> + </rng:optional> <rng:optional> <rng:attribute name="loext:tab-index"> <rng:ref name="nonNegativeInteger"/> diff --git a/sw/qa/core/unocore/unocore.cxx b/sw/qa/core/unocore/unocore.cxx index 2cb6c995e08b..e26e227b059b 100644 --- a/sw/qa/core/unocore/unocore.cxx +++ b/sw/qa/core/unocore/unocore.cxx @@ -574,6 +574,7 @@ CPPUNIT_TEST_FIXTURE(SwCoreUnocoreTest, testContentControlDate) xContentControlProps->setPropertyValue("Color", uno::Any(OUString("008000"))); xContentControlProps->setPropertyValue("Alias", uno::Any(OUString("myalias"))); xContentControlProps->setPropertyValue("Tag", uno::Any(OUString("mytag"))); + xContentControlProps->setPropertyValue("Id", uno::Any(static_cast<sal_Int32>(123))); xContentControlProps->setPropertyValue("TabIndex", uno::Any(sal_uInt32(1))); xContentControlProps->setPropertyValue("Lock", uno::Any(OUString("sdtContentLocked"))); xText->insertTextContent(xCursor, xContentControl, /*bAbsorb=*/true); @@ -601,6 +602,7 @@ CPPUNIT_TEST_FIXTURE(SwCoreUnocoreTest, testContentControlDate) CPPUNIT_ASSERT_EQUAL(OUString("008000"), pContentControl->GetColor()); CPPUNIT_ASSERT_EQUAL(OUString("myalias"), pContentControl->GetAlias()); CPPUNIT_ASSERT_EQUAL(OUString("mytag"), pContentControl->GetTag()); + CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(123), pContentControl->GetId()); CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt32>(1), pContentControl->GetTabIndex()); CPPUNIT_ASSERT_EQUAL(OUString("sdtContentLocked"), pContentControl->GetLock()); } diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport17.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport17.cxx index 72c78cebe79c..a9daa8042f15 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport17.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport17.cxx @@ -300,6 +300,7 @@ CPPUNIT_TEST_FIXTURE(Test, testDateContentControlExport) xContentControlProps->setPropertyValue("Color", uno::Any(OUString("008000"))); xContentControlProps->setPropertyValue("Alias", uno::Any(OUString("myalias"))); xContentControlProps->setPropertyValue("Tag", uno::Any(OUString("mytag"))); + xContentControlProps->setPropertyValue("Id", uno::Any(static_cast<sal_Int32>(123))); xContentControlProps->setPropertyValue("TabIndex", uno::Any(sal_uInt32(2))); xContentControlProps->setPropertyValue("Lock", uno::Any(OUString("sdtLocked"))); @@ -326,6 +327,7 @@ CPPUNIT_TEST_FIXTURE(Test, testDateContentControlExport) assertXPath(pXmlDoc, "//w:sdt/w:sdtPr/w15:color", "val", "008000"); assertXPath(pXmlDoc, "//w:sdt/w:sdtPr/w:alias", "val", "myalias"); assertXPath(pXmlDoc, "//w:sdt/w:sdtPr/w:tag", "val", "mytag"); + assertXPath(pXmlDoc, "//w:sdt/w:sdtPr/w:id", "val", "123"); assertXPath(pXmlDoc, "//w:sdt/w:sdtPr/w:tabIndex", "val", "2"); assertXPath(pXmlDoc, "//w:sdt/w:sdtPr/w:lock", "val", "sdtLocked"); } diff --git a/sw/source/filter/ww8/docxattributeoutput.cxx b/sw/source/filter/ww8/docxattributeoutput.cxx index 20dd8cf8738f..b190bf38e581 100644 --- a/sw/source/filter/ww8/docxattributeoutput.cxx +++ b/sw/source/filter/ww8/docxattributeoutput.cxx @@ -624,13 +624,13 @@ void SdtBlockHelper::DeleteAndResetTheLists() if (!m_aColor.isEmpty()) m_aColor.clear(); m_bShowingPlaceHolder = false; + m_nId = 0; m_nTabIndex = 0; - m_bHasId = false; } void SdtBlockHelper::WriteSdtBlock(::sax_fastparser::FSHelperPtr& pSerializer, bool bRunTextIsOn, bool bParagraphHasDrawing) { - if (m_nSdtPrToken <= 0 && !m_pDataBindingAttrs.is() && !m_bHasId) + if (m_nSdtPrToken <= 0 && !m_pDataBindingAttrs.is() && !m_nId) return; // sdt start mark @@ -685,19 +685,15 @@ void SdtBlockHelper::WriteSdtBlock(::sax_fastparser::FSHelperPtr& pSerializer, b // clear sdt status m_nSdtPrToken = 0; - m_pTokenChildren.clear(); - m_pDataBindingAttrs.clear(); - m_pTextAttrs.clear(); - m_aAlias.clear(); - m_bHasId = false; + DeleteAndResetTheLists(); } void SdtBlockHelper::WriteExtraParams(::sax_fastparser::FSHelperPtr& pSerializer) { - if (m_nSdtPrToken == FSNS(XML_w, XML_id) || m_bHasId) - //Word won't open a document with an empty id tag, we fill it with a random number - pSerializer->singleElementNS(XML_w, XML_id, FSNS(XML_w, XML_val), - OString::number(comphelper::rng::uniform_int_distribution(0, std::numeric_limits<int>::max()))); + if (m_nId) + { + pSerializer->singleElementNS(XML_w, XML_id, FSNS(XML_w, XML_val), OString::number(m_nId)); + } if (m_pDataBindingAttrs.is()) { @@ -852,6 +848,11 @@ void SdtBlockHelper::GetSdtParamsFromGrabBag(const uno::Sequence<beans::Property if (!(aPropertyValue.Value >>= m_aTag)) SAL_WARN("sw.ww8", "DocxAttributeOutput::GrabBag: unexpected sdt tag value"); } + else if (aPropertyValue.Name == "ooxml:CT_SdtPr_id") + { + if (!(aPropertyValue.Value >>= m_nId)) + SAL_WARN("sw.ww8", "DocxAttributeOutput::GrabBag: unexpected sdt id value"); + } else if (aPropertyValue.Name == "ooxml:CT_SdtPr_tabIndex" && !m_nTabIndex) { if (!(aPropertyValue.Value >>= m_nTabIndex)) @@ -862,8 +863,6 @@ void SdtBlockHelper::GetSdtParamsFromGrabBag(const uno::Sequence<beans::Property if (!(aPropertyValue.Value >>= m_aLock)) SAL_WARN("sw.ww8", "DocxAttributeOutput::GrabBag: unexpected sdt lock value"); } - else if (aPropertyValue.Name == "ooxml:CT_SdtPr_id") - m_bHasId = true; else if (aPropertyValue.Name == "ooxml:CT_SdtPr_citation") m_nSdtPrToken = FSNS(XML_w, XML_citation); else if (aPropertyValue.Name == "ooxml:CT_SdtPr_docPartObj" || @@ -1125,7 +1124,6 @@ void DocxAttributeOutput::EndParagraph( ww8::WW8TableNodeInfoInner::Pointer_t pT //These should be written out to the actual Node and not to the anchor. //Clear them as they will be repopulated when the node is processed. m_aParagraphSdt.m_nSdtPrToken = 0; - m_aParagraphSdt.m_bHasId = false; m_aParagraphSdt.DeleteAndResetTheLists(); } diff --git a/sw/source/filter/ww8/docxattributeoutput.hxx b/sw/source/filter/ww8/docxattributeoutput.hxx index e5b00eb4c050..9628eb3942cc 100644 --- a/sw/source/filter/ww8/docxattributeoutput.hxx +++ b/sw/source/filter/ww8/docxattributeoutput.hxx @@ -128,14 +128,14 @@ class SdtBlockHelper { public: SdtBlockHelper() - : m_bHasId(false) + : m_nId(0) , m_bStartedSdt(false) , m_bShowingPlaceHolder(false) , m_nTabIndex(0) , m_nSdtPrToken(0) {} - bool m_bHasId; + sal_Int32 m_nId; bool m_bStartedSdt; rtl::Reference<sax_fastparser::FastAttributeList> m_pTokenChildren; rtl::Reference<sax_fastparser::FastAttributeList> m_pTokenAttributes; diff --git a/writerfilter/qa/cppunittests/dmapper/SdtHelper.cxx b/writerfilter/qa/cppunittests/dmapper/SdtHelper.cxx index 3595eb3d22f0..bde82531b327 100644 --- a/writerfilter/qa/cppunittests/dmapper/SdtHelper.cxx +++ b/writerfilter/qa/cppunittests/dmapper/SdtHelper.cxx @@ -95,6 +95,10 @@ CPPUNIT_TEST_FIXTURE(Test, testSdtRunRichText) xContentControlProps->getPropertyValue("Tag") >>= aTag; // This was empty. CPPUNIT_ASSERT_EQUAL(OUString("mytag"), aTag); + sal_Int32 nId = 0; + xContentControlProps->getPropertyValue("Id") >>= nId; + // This was 0. + CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(2147483647), nId); sal_uInt32 nTabIndex = 0; xContentControlProps->getPropertyValue("TabIndex") >>= nTabIndex; // This was 0 diff --git a/writerfilter/qa/cppunittests/dmapper/data/sdt-run-rich-text.docx b/writerfilter/qa/cppunittests/dmapper/data/sdt-run-rich-text.docx index ca980abb0356..d5644f33e9d9 100644 Binary files a/writerfilter/qa/cppunittests/dmapper/data/sdt-run-rich-text.docx and b/writerfilter/qa/cppunittests/dmapper/data/sdt-run-rich-text.docx differ diff --git a/writerfilter/source/dmapper/DomainMapper.cxx b/writerfilter/source/dmapper/DomainMapper.cxx index 3301658eaefb..a3007aeb1b3a 100644 --- a/writerfilter/source/dmapper/DomainMapper.cxx +++ b/writerfilter/source/dmapper/DomainMapper.cxx @@ -2940,13 +2940,15 @@ void DomainMapper::sprmWithProps( Sprm& rSprm, const PropertyMapPtr& rContext ) } else if (nSprmId == NS_ooxml::LN_CT_SdtPr_showingPlcHdr) { + // Grabbag boolean values beans::PropertyValue aValue; aValue.Name = sName; aValue.Value <<= bool(nIntValue); m_pImpl->m_pSdtHelper->appendToInteropGrabBag(aValue); } - else if (nSprmId == NS_ooxml::LN_CT_SdtPr_tabIndex) + else if (nSprmId == NS_ooxml::LN_CT_SdtPr_id || nSprmId == NS_ooxml::LN_CT_SdtPr_tabIndex) { + // Grabbag integer values beans::PropertyValue aValue; aValue.Name = sName; aValue.Value <<= nIntValue; diff --git a/xmloff/qa/unit/data/content-control-alias.fodt b/xmloff/qa/unit/data/content-control-alias.fodt index 48fdc7b9436c..0742610ca63a 100644 --- a/xmloff/qa/unit/data/content-control-alias.fodt +++ b/xmloff/qa/unit/data/content-control-alias.fodt @@ -2,7 +2,7 @@ <office:document xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0" xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0" xmlns:loext="urn:org:documentfoundation:names:experimental:office:xmlns:loext:1.0" office:version="1.3" office:mimetype="application/vnd.oasis.opendocument.text"> <office:body> <office:text> - <text:p><loext:content-control loext:alias="my alias" loext:tag="my tag" loext:tab-index="4" loext:lock="sdtContentLocked">test</loext:content-control></text:p> + <text:p><loext:content-control loext:alias="my alias" loext:tag="my tag" loext:id="2147483647" loext:tab-index="4" loext:lock="sdtContentLocked">test</loext:content-control></text:p> </office:text> </office:body> </office:document> diff --git a/xmloff/qa/unit/text.cxx b/xmloff/qa/unit/text.cxx index 9d6be54c5e70..c8dc35c28d00 100644 --- a/xmloff/qa/unit/text.cxx +++ b/xmloff/qa/unit/text.cxx @@ -849,6 +849,7 @@ CPPUNIT_TEST_FIXTURE(XmloffStyleTest, testAliasContentControlExport) uno::Reference<beans::XPropertySet> xContentControlProps(xContentControl, uno::UNO_QUERY); xContentControlProps->setPropertyValue("Alias", uno::Any(OUString("my alias"))); xContentControlProps->setPropertyValue("Tag", uno::Any(OUString("my tag"))); + xContentControlProps->setPropertyValue("Id", uno::Any(static_cast<sal_Int32>(-2147483648))); xContentControlProps->setPropertyValue("TabIndex", uno::Any(sal_uInt32(3))); xContentControlProps->setPropertyValue("Lock", uno::Any(OUString("unlocked"))); xText->insertTextContent(xCursor, xContentControl, /*bAbsorb=*/true); @@ -872,6 +873,7 @@ CPPUNIT_TEST_FIXTURE(XmloffStyleTest, testAliasContentControlExport) // i.e. alias was lost on export. assertXPath(pXmlDoc, "//loext:content-control", "alias", "my alias"); assertXPath(pXmlDoc, "//loext:content-control", "tag", "my tag"); + assertXPath(pXmlDoc, "//loext:content-control", "id", "-2147483648"); assertXPath(pXmlDoc, "//loext:content-control", "tab-index", "3"); assertXPath(pXmlDoc, "//loext:content-control", "lock", "unlocked"); } @@ -939,6 +941,9 @@ CPPUNIT_TEST_FIXTURE(XmloffStyleTest, testAliasContentControlImport) OUString aTag; xContentControlProps->getPropertyValue("Tag") >>= aTag; CPPUNIT_ASSERT_EQUAL(OUString("my tag"), aTag); + sal_Int32 nId = 0; + xContentControlProps->getPropertyValue("Id") >>= nId; + CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(2147483647), nId); sal_uInt32 nTabIndex; xContentControlProps->getPropertyValue("TabIndex") >>= nTabIndex; CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt32>(4), nTabIndex); diff --git a/xmloff/source/text/txtparae.cxx b/xmloff/source/text/txtparae.cxx index b90b7318a07f..957c64a25bb3 100644 --- a/xmloff/source/text/txtparae.cxx +++ b/xmloff/source/text/txtparae.cxx @@ -3989,6 +3989,13 @@ void XMLTextParagraphExport::ExportContentControl( GetExport().AddAttribute(XML_NAMESPACE_LO_EXT, XML_TAG, aTag); } + sal_Int32 nId = 0; + xPropertySet->getPropertyValue("Id") >>= nId; + if (nId) + { + GetExport().AddAttribute(XML_NAMESPACE_LO_EXT, XML_ID, OUString::number(nId)); + } + sal_uInt32 nTabIndex; xPropertySet->getPropertyValue("TabIndex") >>= nTabIndex; if (nTabIndex) diff --git a/xmloff/source/text/xmlcontentcontrolcontext.cxx b/xmloff/source/text/xmlcontentcontrolcontext.cxx index 40ea1157b43d..d0371ed72220 100644 --- a/xmloff/source/text/xmlcontentcontrolcontext.cxx +++ b/xmloff/source/text/xmlcontentcontrolcontext.cxx @@ -152,6 +152,14 @@ void XMLContentControlContext::startFastElement( m_aTag = rIter.toString(); break; } + case XML_ELEMENT(LO_EXT, XML_ID): + { + if (sax::Converter::convertNumber(nTmp, rIter.toView())) + { + m_nId = nTmp; + } + break; + } case XML_ELEMENT(LO_EXT, XML_TAB_INDEX): { if (sax::Converter::convertNumber(nTmp, rIter.toView())) @@ -276,6 +284,11 @@ void XMLContentControlContext::endFastElement(sal_Int32) xPropertySet->setPropertyValue("Tag", uno::Any(m_aTag)); } + if (m_nId) + { + xPropertySet->setPropertyValue("Id", uno::Any(m_nId)); + } + if (m_nTabIndex) { xPropertySet->setPropertyValue("TabIndex", uno::Any(m_nTabIndex)); diff --git a/xmloff/source/text/xmlcontentcontrolcontext.hxx b/xmloff/source/text/xmlcontentcontrolcontext.hxx index 44abe71d6a08..13c1e50f23fa 100644 --- a/xmloff/source/text/xmlcontentcontrolcontext.hxx +++ b/xmloff/source/text/xmlcontentcontrolcontext.hxx @@ -53,6 +53,7 @@ class XMLContentControlContext : public SvXMLImportContext bool m_bDropDown = false; OUString m_aAlias; OUString m_aTag; + sal_Int32 m_nId = 0; sal_uInt32 m_nTabIndex = 0; OUString m_aLock;