From 8401181bf0232959efb516802abcda42377ea06e Mon Sep 17 00:00:00 2001 From: Vasily Melenchuk Date: Sun, 17 May 2020 13:35:46 +0300 Subject: tdf#120394: DOCX list import: simplify zero width space hack Since introducion of list format string hack with creation of zero-width-space can be much more simple. It was being used to indicate existing, but empty list label suffix to avoid stripping down numbering. Change-Id: I9a0c6047f806b2c656ef5dbab0c6b38200818bd2 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/94383 Tested-by: Jenkins Reviewed-by: Thorsten Behrens --- sw/qa/extras/ooxmlexport/data/tdf120394.docx | Bin 0 -> 11032 bytes sw/qa/extras/ooxmlexport/ooxmlexport14.cxx | 25 +++++++++++++ sw/qa/extras/rtfimport/rtfimport.cxx | 5 ++- sw/source/core/doc/number.cxx | 7 ++++ sw/source/filter/ww8/wrtw8num.cxx | 30 ++++++++-------- writerfilter/source/dmapper/NumberingManager.cxx | 44 ++++------------------- 6 files changed, 56 insertions(+), 55 deletions(-) create mode 100644 sw/qa/extras/ooxmlexport/data/tdf120394.docx diff --git a/sw/qa/extras/ooxmlexport/data/tdf120394.docx b/sw/qa/extras/ooxmlexport/data/tdf120394.docx new file mode 100644 index 000000000000..39bd5886c0fe Binary files /dev/null and b/sw/qa/extras/ooxmlexport/data/tdf120394.docx differ diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx index b372f2e86dcd..b95d0afe4e8f 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx @@ -887,6 +887,31 @@ DECLARE_OOXMLEXPORT_EXPORTONLY_TEST(testTdf128290, "tdf128290.odt") assertXPath(pXml, "/w:document/w:body/w:tbl/w:tblPr/w:tblLayout", "type", "fixed"); } +DECLARE_OOXMLEXPORT_TEST(testTdf120394, "tdf120394.docx") +{ + CPPUNIT_ASSERT_EQUAL(1, getPages()); + { + uno::Reference xPara(getParagraph(1), uno::UNO_QUERY); + CPPUNIT_ASSERT_EQUAL(static_cast(0), getProperty(xPara, "NumberingLevel")); + CPPUNIT_ASSERT_EQUAL(OUString("1"), getProperty(xPara, "ListLabelString")); + } + { + uno::Reference xPara(getParagraph(2), uno::UNO_QUERY); + CPPUNIT_ASSERT_EQUAL(static_cast(1), getProperty(xPara, "NumberingLevel")); + CPPUNIT_ASSERT_EQUAL(OUString(CHAR_ZWSP), getProperty(xPara, "ListLabelString")); + } + { + uno::Reference xPara(getParagraph(3), uno::UNO_QUERY); + CPPUNIT_ASSERT_EQUAL(static_cast(1), getProperty(xPara, "NumberingLevel")); + CPPUNIT_ASSERT_EQUAL(OUString(CHAR_ZWSP), getProperty(xPara, "ListLabelString")); + } + { + uno::Reference xPara(getParagraph(5), uno::UNO_QUERY); + CPPUNIT_ASSERT_EQUAL(static_cast(2), getProperty(xPara, "NumberingLevel")); + CPPUNIT_ASSERT_EQUAL(OUString("1.2.1"), getProperty(xPara, "ListLabelString")); + } +} + DECLARE_OOXMLEXPORT_EXPORTONLY_TEST(testTdf123757, "tdf123757.docx") { xmlDocUniquePtr pXml = parseExport("word/document.xml"); diff --git a/sw/qa/extras/rtfimport/rtfimport.cxx b/sw/qa/extras/rtfimport/rtfimport.cxx index 085a71aedc86..cf9d5dd31309 100644 --- a/sw/qa/extras/rtfimport/rtfimport.cxx +++ b/sw/qa/extras/rtfimport/rtfimport.cxx @@ -343,8 +343,7 @@ CPPUNIT_TEST_FIXTURE(Test, testFdo49692) if (rProp.Name == "Suffix") { - OUString aExpected(u'\x200B'); - CPPUNIT_ASSERT_EQUAL(aExpected, rProp.Value.get()); + CPPUNIT_ASSERT(rProp.Value.get().isEmpty()); } } } @@ -1358,7 +1357,7 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf78506) if (rProp.Name == "Suffix") // This was '0', invalid \levelnumbers wasn't ignored. - CPPUNIT_ASSERT_EQUAL(CHAR_ZWSP, rProp.Value.get().toChar()); + CPPUNIT_ASSERT(rProp.Value.get().isEmpty()); } } diff --git a/sw/source/core/doc/number.cxx b/sw/source/core/doc/number.cxx index 02c2512e7872..1d2ae4f970d4 100644 --- a/sw/source/core/doc/number.cxx +++ b/sw/source/core/doc/number.cxx @@ -667,6 +667,13 @@ OUString SwNumRule::MakeNumString( const SwNumberTree::tNumberVector & rNumVecto if (nPosition >= 0) sLevelFormat = sLevelFormat.replaceAt(nPosition, sFind.getLength(), sReplacement); } + + // As a fallback: caller code expects nonempty string as a result. + // But if we have empty string (and had no errors before) this is valid result. + // So use classical hack with zero-width-space as a string filling. + if (sLevelFormat.isEmpty()) + sLevelFormat = OUStringChar(CHAR_ZWSP); + aStr = sLevelFormat; } else diff --git a/sw/source/filter/ww8/wrtw8num.cxx b/sw/source/filter/ww8/wrtw8num.cxx index e2aad33206d3..1774c37239e7 100644 --- a/sw/source/filter/ww8/wrtw8num.cxx +++ b/sw/source/filter/ww8/wrtw8num.cxx @@ -494,12 +494,7 @@ void MSWordExportBase::NumberingLevel( const vcl::Font* pBulletFont=nullptr; rtl_TextEncoding eChrSet=0; FontFamily eFamily=FAMILY_DECORATIVE; - if (rRule.Get(nLvl).HasListFormat()) - { - // Nothing to construct: we have it already - sNumStr = rRule.Get(nLvl).GetListFormat(); - } - else if (SVX_NUM_CHAR_SPECIAL == rFormat.GetNumberingType() || + if (SVX_NUM_CHAR_SPECIAL == rFormat.GetNumberingType() || SVX_NUM_BITMAP == rFormat.GetNumberingType()) { // Use bullet @@ -507,7 +502,9 @@ void MSWordExportBase::NumberingLevel( } else { - // Construct list format string from prefix, level numbers and suffix + // Create level string + // For docx it is not the best way: we can just take it from rRule.Get(nLvl).GetListFormat() + // But for compatibility with doc we follow same routine if (SVX_NUM_NUMBER_NONE != rFormat.GetNumberingType()) { sal_uInt8* pLvlPos = aNumLvlPos; @@ -518,20 +515,23 @@ void MSWordExportBase::NumberingLevel( // now search the nums in the string for (sal_uInt8 i = 0; i <= nLvl; ++i) { - OUString sSrch( OUString::number( i )); - sal_Int32 nFnd = sNumStr.indexOf( sSrch ); - if( -1 != nFnd ) + OUString sSrch(OUString::number(i)); + sal_Int32 nFnd = sNumStr.indexOf(sSrch); + if (-1 != nFnd) { - *pLvlPos = static_cast(nFnd + rFormat.GetPrefix().getLength() + 1 ); + *pLvlPos = static_cast(nFnd + rFormat.GetPrefix().getLength() + 1); ++pLvlPos; - sNumStr = sNumStr.replaceAt( nFnd, 1, OUString(static_cast(i)) ); + sNumStr = sNumStr.replaceAt(nFnd, 1, OUString(static_cast(i))); } } } - if (!rFormat.GetPrefix().isEmpty()) - sNumStr = rFormat.GetPrefix() + sNumStr; - sNumStr += rFormat.GetSuffix(); + if (!rRule.Get(nLvl).HasListFormat()) + { + if (!rFormat.GetPrefix().isEmpty()) + sNumStr = rFormat.GetPrefix() + sNumStr; + sNumStr += rFormat.GetSuffix(); + } } if (SVX_NUM_CHAR_SPECIAL == rFormat.GetNumberingType() || diff --git a/writerfilter/source/dmapper/NumberingManager.cxx b/writerfilter/source/dmapper/NumberingManager.cxx index 59fb4f8c2123..047d29a065ca 100644 --- a/writerfilter/source/dmapper/NumberingManager.cxx +++ b/writerfilter/source/dmapper/NumberingManager.cxx @@ -207,10 +207,6 @@ uno::Sequence ListLevel::GetLevelProperties(bool bDefaults { if (m_xGraphicBitmap.is()) nNumberFormat = style::NumberingType::BITMAP; - else if (m_sBulletChar.isEmpty() && nNumberFormat != style::NumberingType::CHAR_SPECIAL) - // w:lvlText is empty, that means no numbering in Word. - // CHAR_SPECIAL is handled separately below. - nNumberFormat = style::NumberingType::NUMBER_NONE; aNumberingProperties.push_back(lcl_makePropVal(PROP_NUMBERING_TYPE, nNumberFormat)); } @@ -548,40 +544,14 @@ void ListDef::CreateNumberingRules( DomainMapper& rDMapper, if (pLevel && !pLevel->GetBulletChar().isEmpty()) sText = pLevel->GetBulletChar( ); - if (sText.isEmpty()) - { - // Empty ? Then put a Unicode "zero width space" as a suffix, so LabelFollowedBy is still shown, as in Word. - // With empty suffix, Writer does not show LabelFollowedBy, either. - OUString sSuffix; - auto it = std::find_if(aLvlProps.begin(), aLvlProps.end(), [](const beans::PropertyValue& rValue) { return rValue.Name == "NumberingType"; }); - if (it != aLvlProps.end()) - { - sal_Int16 nNumberFormat = it->Value.get(); - - // No need for a zero width space without a real LabelFollowedBy. - bool bLabelFollowedBy = true; - it = std::find_if(aLvlProps.begin(), aLvlProps.end(), [](const beans::PropertyValue& rValue) { return rValue.Name == "LabelFollowedBy"; }); - if (it != aLvlProps.end()) - { - sal_Int16 nValue; - if (it->Value >>= nValue) - bLabelFollowedBy = nValue != SvxNumberFormat::NOTHING; - } + aLvlProps.push_back(comphelper::makePropertyValue(getPropertyName(PROP_PREFIX), OUString(""))); + aLvlProps.push_back(comphelper::makePropertyValue(getPropertyName(PROP_SUFFIX), OUString(""))); + aLvlProps.push_back(comphelper::makePropertyValue(getPropertyName(PROP_LIST_FORMAT), sText)); - if (bLabelFollowedBy && nNumberFormat == style::NumberingType::NUMBER_NONE) - sSuffix = OUString(u'\x200B'); - } - aLvlProps.push_back(comphelper::makePropertyValue(getPropertyName(PROP_SUFFIX), sSuffix)); - } - else - { - aLvlProps.push_back(comphelper::makePropertyValue(getPropertyName(PROP_LIST_FORMAT), sText)); - - // Total count of replacement holders is determining amount of required parent numbering to include - // TODO: not sure how "%" symbol is escaped. This is not supported yet - sal_Int16 nParentNum = comphelper::string::getTokenCount(sText, '%'); - aLvlProps.push_back(comphelper::makePropertyValue(getPropertyName(PROP_PARENT_NUMBERING), nParentNum)); - } + // Total count of replacement holders is determining amount of required parent numbering to include + // TODO: not sure how "%" symbol is escaped. This is not supported yet + sal_Int16 nParentNum = comphelper::string::getTokenCount(sText, '%'); + aLvlProps.push_back(comphelper::makePropertyValue(getPropertyName(PROP_PARENT_NUMBERING), nParentNum)); aLvlProps.push_back(comphelper::makePropertyValue(getPropertyName(PROP_POSITION_AND_SPACE_MODE), sal_Int16(text::PositionAndSpaceMode::LABEL_ALIGNMENT))); -- cgit