[Libreoffice-commits] core.git: sw/qa writerfilter/source
Justin Luth (via logerrit)
logerrit at kemper.freedesktop.org
Tue Jul 6 06:53:30 UTC 2021
sw/qa/extras/ooxmlexport/data/tdf141966_chapterNumbering.docx |binary
sw/qa/extras/ooxmlexport/ooxmlexport16.cxx | 13 +
writerfilter/source/dmapper/DomainMapper_Impl.cxx | 7
writerfilter/source/dmapper/NumberingManager.cxx | 90 ++++++----
writerfilter/source/dmapper/NumberingManager.hxx | 13 -
5 files changed, 79 insertions(+), 44 deletions(-)
New commits:
commit 3e09e0784ad7669d3e0a7655f5e604a2387b1b5d
Author: Justin Luth <justin_luth at sil.org>
AuthorDate: Thu May 13 13:50:56 2021 +0200
Commit: Miklos Vajna <vmiklos at collabora.com>
CommitDate: Tue Jul 6 08:52:55 2021 +0200
tdf#141966 writerfilter CN: fix chapter number identification
This totally up-ends the current way of doing things,
but it was so totally broken, and confused us for a very long time.
Chapter Numbering has nothing to do with "Heading X" styles!!!!!!!!
Thus, we can clean up a lot of nonsense clauses.
The existing code makes it sound like the constraints of
"outline numbering" are an MS format thing, but actually it is
just an implementation detail of how LO contrains styles
and listLevels.
Yes, it is still nice to be able to round-trip the
Chapter Numbering settings, so we won't ignore it completely,
but really, all the complexity comes from trying to cram
the normal flexibility of inheritable, style-assignable
listLevels into a single special-purpose numbering rule.
One thing that has always killed us is that direct formatting
could not share in this unique style. But it needs to in DOCX.
So, the key here is to NAME-ASSOCIATE the "Outline" style,
so that ALL references to that numId end up pointing to the
same rule, even if it is the super-special oddball one.
P.S. Chapter Numbering should have NO IMPACT on the
import process. It should only affect UI interaction
when a user assigns a paragraph style: whether LO can
apply a numbering listLevel or not.
THIS IS EXTREMELY DANGEROUS TERRITORY! WAIT FOR 7.3!
existing unit tests that mapped to a numId other than 1:
ooxmlexport3: tdf95495 - numId 4
ooxmlexport4: tdf81345 - numId 3
ooxmlexport5: tdf123262 - numId 48
ooxmlexport8: fdo74745 - numId 6
ooxmlexport10: tdf89165 - numId 12
ooxmlexport14: tdf133605 - numId 2
ooxmlexport16: tdf132752 - numId 2
and one example which didn't take the first available choice:
ooxmlexport13: tdf95848 - preferred numId 3 over numId 2
Change-Id: I561cd1594f198ad402893c77c1c983f206f53c57
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/115614
Tested-by: Jenkins
Reviewed-by: Justin Luth <justin_luth at sil.org>
Reviewed-by: Miklos Vajna <vmiklos at collabora.com>
diff --git a/sw/qa/extras/ooxmlexport/data/tdf141966_chapterNumbering.docx b/sw/qa/extras/ooxmlexport/data/tdf141966_chapterNumbering.docx
new file mode 100644
index 000000000000..3957fadf43c9
Binary files /dev/null and b/sw/qa/extras/ooxmlexport/data/tdf141966_chapterNumbering.docx differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport16.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport16.cxx
index b670c72333ff..8ec840b3b479 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport16.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport16.cxx
@@ -31,10 +31,12 @@
#include <com/sun/star/view/XViewCursor.hpp>
#include <comphelper/configuration.hxx>
#include <comphelper/propertysequence.hxx>
+#include <comphelper/sequenceashashmap.hxx>
#include <editeng/escapementitem.hxx>
#include <unotools/fltrcfg.hxx>
#include <textboxhelper.hxx>
#include <unoprnms.hxx>
+#include <unotxdoc.hxx> //XChapterNumberingSupplier
constexpr OUStringLiteral DATA_DIRECTORY = u"/sw/qa/extras/ooxmlexport/data/";
@@ -137,6 +139,17 @@ DECLARE_OOXMLEXPORT_TEST(testTdf141231_arabicHebrewNumbering, "tdf141231_arabicH
CPPUNIT_ASSERT_EQUAL(style::NumberingType::CHARS_ARABIC_ABJAD, nActual);
}
+DECLARE_OOXMLEXPORT_TEST(testTdf141966_chapterNumbering, "tdf141966_chapterNumbering.docx")
+{
+ uno::Reference<text::XChapterNumberingSupplier> xNumberingSupplier(mxComponent, uno::UNO_QUERY);
+ uno::Reference<container::XIndexAccess> xNumberingRules = xNumberingSupplier->getChapterNumberingRules();
+ comphelper::SequenceAsHashMap hashMap(xNumberingRules->getByIndex(0));
+
+ CPPUNIT_ASSERT(hashMap["HeadingStyleName"].get<OUString>().match("CN1"));
+
+ uno::Reference<beans::XPropertySet> xPara(getParagraph(7, "Direct formatting with \"Outline\" numbering."), uno::UNO_QUERY);
+ CPPUNIT_ASSERT_EQUAL(OUString("2nd"), getProperty<OUString>(xPara, "ListLabelString"));
+}
DECLARE_OOXMLEXPORT_TEST(testTdf132752, "tdf132752.docx")
{
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index 881c512358d8..30928027007c 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -1516,13 +1516,10 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con
isNumberingViaStyle = true;
// Since LO7.0/tdf#131321 fixed the loss of numbering in styles, this OUGHT to be obsolete,
// but now other new/critical LO7.0 code expects it, and perhaps some corner cases still need it as well.
- // So we skip it only for default outline styles, which are recognized by NumberingManager.
- if (!GetCurrentParaStyleName().startsWith("Heading ") || nListLevel >= pList->GetDefaultParentLevels())
- pParaContext->Insert( PROP_NUMBERING_STYLE_NAME, uno::makeAny(pList->GetStyleName()), true );
+ pParaContext->Insert(PROP_NUMBERING_STYLE_NAME, uno::makeAny(pList->GetStyleName()), true);
}
- else if ( !pList->isOutlineNumbering(nListLevel) )
+ else
{
- // After ignoring anything related to the special Outline levels,
// we have direct numbering, as well as paragraph-style numbering.
// Apply the style if it uses the same list as the direct numbering,
// otherwise the directly-applied-to-paragraph status will be lost,
diff --git a/writerfilter/source/dmapper/NumberingManager.cxx b/writerfilter/source/dmapper/NumberingManager.cxx
index 764aace41808..b8a511f83a6c 100644
--- a/writerfilter/source/dmapper/NumberingManager.cxx
+++ b/writerfilter/source/dmapper/NumberingManager.cxx
@@ -133,6 +133,11 @@ void ListLevel::SetValue( Id nId, sal_Int32 nValue )
void ListLevel::SetCustomNumberFormat(const OUString& rValue) { m_aCustomNumberFormat = rValue; }
+sal_Int16 ListLevel::GetNumberingType(sal_Int16 nDefault) const
+{
+ return ConversionHelper::ConvertNumberingType(m_nNFC, nDefault);
+}
+
bool ListLevel::HasValues() const
{
return m_bHasValues;
@@ -143,14 +148,6 @@ void ListLevel::SetParaStyle( const tools::SvRef< StyleSheetEntry >& pStyle )
if (!pStyle)
return;
m_pParaStyle = pStyle;
- // AFAICT .docx spec does not identify which numberings or paragraph
- // styles are actually the ones to be used for outlines (chapter numbering),
- // it only kind of says somewhere that they should be named Heading1 to Heading9.
- const OUString styleId= pStyle->sConvertedStyleName;
- m_outline = ( styleId.getLength() == RTL_CONSTASCII_LENGTH( "Heading 1" )
- && styleId.match( "Heading ", 0 )
- && styleId[ RTL_CONSTASCII_LENGTH( "Heading " ) ] >= '1'
- && styleId[ RTL_CONSTASCII_LENGTH( "Heading " ) ] <= '9' );
}
uno::Sequence<beans::PropertyValue> ListLevel::GetProperties(bool bDefaults)
@@ -407,7 +404,6 @@ const OUString& AbstractListDef::MapListId(OUString const& rId)
ListDef::ListDef( ) : AbstractListDef( )
{
- m_nDefaultParentLevels = WW_OUTLINE_MAX + 1;
}
ListDef::~ListDef( )
@@ -484,8 +480,40 @@ static uno::Reference< container::XNameContainer > lcl_getUnoNumberingStyles(
return xStyles;
}
+/// Rank the list in terms of suitability for becoming the Outline numbering rule in LO.
+sal_uInt16 ListDef::GetChapterNumberingWeight() const
+{
+ sal_Int16 nWeight = 0;
+ const sal_Int8 nAbstLevels = m_pAbstractDef ? m_pAbstractDef->Size() : 0;
+ for (sal_Int8 nLevel = 0; nLevel < nAbstLevels; ++nLevel)
+ {
+ const ListLevel::Pointer pAbsLevel = m_pAbstractDef->GetLevel(nLevel);
+ const StyleSheetEntryPtr pParaStyle = pAbsLevel->GetParaStyle();
+ if (!pParaStyle)
+ continue;
+ const StyleSheetPropertyMap& rProps =
+ *static_cast<StyleSheetPropertyMap*>(pParaStyle->pProperties.get());
+ // In LO, the level's paraStyle outlineLevel always matches this listLevel.
+ // An undefined listLevel is treated as the first level.
+ sal_Int8 nListLevel = std::clamp<sal_Int8>(rProps.GetListLevel(), 0, 9);
+ if (nListLevel != nLevel || rProps.GetOutlineLevel() != nLevel)
+ return 0;
+ else if (pAbsLevel->GetNumberingType(style::NumberingType::NUMBER_NONE)
+ != style::NumberingType::NUMBER_NONE)
+ {
+ // Arbitrarily chosen weighting factors - trying to round-trip LO choices if possible.
+ // LibreOffice always saves Outline rule (usually containing heading styles) as numId 1.
+ sal_uInt16 nWeightingFactor = GetId() == 1 ? 8 : 1;
+ if (pParaStyle->sStyleIdentifierD.startsWith("Heading") )
+ ++nWeightingFactor;
+ nWeight += nWeightingFactor;
+ }
+ }
+ return nWeight;
+}
+
void ListDef::CreateNumberingRules( DomainMapper& rDMapper,
- uno::Reference<lang::XMultiServiceFactory> const& xFactory)
+ uno::Reference<lang::XMultiServiceFactory> const& xFactory, sal_Int16 nOutline)
{
// Get the UNO Numbering styles
uno::Reference< container::XNameContainer > xStyles = lcl_getUnoNumberingStyles( xFactory );
@@ -501,11 +529,12 @@ void ListDef::CreateNumberingRules( DomainMapper& rDMapper,
xFactory->createInstance("com.sun.star.style.NumberingStyle"),
uno::UNO_QUERY_THROW );
- OUString sStyleName = GetStyleName(GetId(), xStyles);
-
- xStyles->insertByName( sStyleName, makeAny( xStyle ) );
+ if (GetId() == nOutline)
+ m_StyleName = "Outline"; //SwNumRule.GetOutlineRuleName()
+ else
+ xStyles->insertByName(GetStyleName(GetId(), xStyles), makeAny(xStyle));
- uno::Any oStyle = xStyles->getByName( sStyleName );
+ uno::Any oStyle = xStyles->getByName(GetStyleName());
xStyle.set( oStyle, uno::UNO_QUERY_THROW );
// Get the default OOo Numbering style rules
@@ -572,7 +601,7 @@ void ListDef::CreateNumberingRules( DomainMapper& rDMapper,
m_xNumRules->replaceByIndex(nLevel, uno::makeAny(comphelper::containerToSequence(aLvlProps)));
// Handle the outline level here
- if (pAbsLevel && pAbsLevel->isOutlineNumbering())
+ if (GetId() == nOutline && pAbsLevel && pAbsLevel->GetParaStyle())
{
uno::Reference< text::XChapterNumberingSupplier > xOutlines (
xFactory, uno::UNO_QUERY_THROW );
@@ -585,19 +614,6 @@ void ListDef::CreateNumberingRules( DomainMapper& rDMapper,
xOutlineRules->replaceByIndex(nLevel, uno::makeAny(comphelper::containerToSequence(aLvlProps)));
}
- if (pAbsLevel)
- {
- // first level without default outline paragraph style
- const tools::SvRef< StyleSheetEntry >& aParaStyle = pAbsLevel->GetParaStyle();
- if ( WW_OUTLINE_MAX + 1 == m_nDefaultParentLevels && ( !aParaStyle ||
- aParaStyle->sConvertedStyleName.getLength() != RTL_CONSTASCII_LENGTH( "Heading 1" ) ||
- !aParaStyle->sConvertedStyleName.startsWith("Heading ") ||
- aParaStyle->sConvertedStyleName[ RTL_CONSTASCII_LENGTH( "Heading " ) ] - u'1' != nLevel ) )
- {
- m_nDefaultParentLevels = nLevel;
- }
- }
-
nLevel++;
}
@@ -1162,10 +1178,26 @@ ListDef::Pointer ListsManager::GetList( sal_Int32 nId )
void ListsManager::CreateNumberingRules( )
{
+ // Try to determine which numId would best work as LO's Chapter Numbering Outline rule.
+ sal_Int16 nChosenAsChapterNumberingId = -1;
+ sal_uInt16 nHighestWeight = 0;
+ for (const auto& rList : m_aLists)
+ {
+ sal_uInt16 nWeight = rList->GetChapterNumberingWeight();
+ if (nWeight > nHighestWeight)
+ {
+ nHighestWeight = nWeight;
+ nChosenAsChapterNumberingId = rList->GetId();
+ //Optimization: if the weight cannot be beaten anymore, then quit early
+ if (nHighestWeight > 17)
+ break;
+ }
+ }
+
// Loop over the definitions
for ( const auto& rList : m_aLists )
{
- rList->CreateNumberingRules( m_rDMapper, m_xFactory );
+ rList->CreateNumberingRules(m_rDMapper, m_xFactory, nChosenAsChapterNumberingId);
}
m_rDMapper.GetStyleSheetTable()->ApplyNumberingStyleNameToParaStyles();
}
diff --git a/writerfilter/source/dmapper/NumberingManager.hxx b/writerfilter/source/dmapper/NumberingManager.hxx
index 2a1e0204e02d..252f26149fea 100644
--- a/writerfilter/source/dmapper/NumberingManager.hxx
+++ b/writerfilter/source/dmapper/NumberingManager.hxx
@@ -51,7 +51,6 @@ class ListLevel : public PropertyMap
css::uno::Reference<css::awt::XBitmap> m_xGraphicBitmap;
std::optional<sal_Int32> m_nTabstop;
tools::SvRef< StyleSheetEntry > m_pParaStyle;
- bool m_outline;
bool m_bHasValues = false;
public:
@@ -63,7 +62,6 @@ public:
,m_nStartOverride(-1)
,m_nNFC(-1)
,m_nXChFollow(SvxNumberFormat::LISTTAB)
- ,m_outline(false)
{}
// Setters for the import
@@ -77,9 +75,9 @@ public:
void SetParaStyle( const tools::SvRef< StyleSheetEntry >& pStyle );
// Getters
+ sal_Int16 GetNumberingType(sal_Int16 nDefault) const;
const OUString& GetBulletChar( ) const { return m_sBulletChar; };
const tools::SvRef< StyleSheetEntry >& GetParaStyle( ) const { return m_pParaStyle; };
- bool isOutlineNumbering() const { return m_outline; }
sal_Int32 GetStartOverride() const { return m_nStartOverride; };
/// Determines if SetValue() was called at least once.
bool HasValues() const;
@@ -163,7 +161,6 @@ public:
const OUString& GetStyleLink() const { return m_sStyleLink; };
const OUString& MapListId(OUString const& rId);
- bool isOutlineNumbering( sal_uInt16 nLvl ) { return GetLevel(nLvl) && GetLevel(nLvl)->isOutlineNumbering(); }
};
class ListDef : public AbstractListDef
@@ -178,9 +175,6 @@ private:
/// mapped list style name
OUString m_StyleName;
- /// not custom outline parent levels
- sal_Int16 m_nDefaultParentLevels;
-
public:
typedef tools::SvRef< ListDef > Pointer;
@@ -195,11 +189,10 @@ public:
const OUString & GetStyleName() const { return m_StyleName; };
const OUString & GetStyleName(sal_Int32 nId, css::uno::Reference<css::container::XNameContainer> const& xStyles);
- sal_Int16 GetDefaultParentLevels() const { return m_nDefaultParentLevels; };
-
css::uno::Sequence< css::uno::Sequence<css::beans::PropertyValue> > GetMergedPropertyValues();
- void CreateNumberingRules(DomainMapper& rDMapper, css::uno::Reference<css::lang::XMultiServiceFactory> const& xFactory);
+ sal_uInt16 GetChapterNumberingWeight() const;
+ void CreateNumberingRules(DomainMapper& rDMapper, css::uno::Reference<css::lang::XMultiServiceFactory> const& xFactory, sal_Int16 nOutline);
const css::uno::Reference<css::container::XIndexReplace>& GetNumberingRules() const { return m_xNumRules; }
More information about the Libreoffice-commits
mailing list