strepsirrhini/office-gobmx
9
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

tdf#159549 sw: fix ODF import of newly colliding Body Text styles

Browse source 
Commit c83d241eff "tdf#154933 Rename "Text
Body" para style to "Body Text"" introduced a regression when importing
certain ODF documents, but the problem is actually pre-existing.

What happens is that first the built-in "Text body" style is created,
and then a non-built-in style with the same translated name as "Text
body" is imported, and instead of creating a new style, the built-in one
is found and used, and so its properties are overwritten.

The root cause is that SwStyleNameMapper::FillProgName() and in
particular SwStyleNameMapper::FillUIName() are defined poorly, ever
since they were introduced in 2001 in commit
4fbc9dd48b

It becomes obvious relatively quickly that the way style names work is
that at the UNO API level, the "ProgName" (internal, non-localised)
names are used, and at the core document level, the "UIName" (localised)
names are used.

This is in itself questionable - why is the translation from ProgName to
UIName not done in the UI? - but also very expensive to change now.

So then the UNO services are responsible for translating between
ProgName and UIName.

But the 2 functions don't do that properly; both need to check if the
given name is a known ProgName *or* a known UIName, and rename it in
case it collides with a known target name; also the 2 functions need to
cancel each other out, not add " (user)" at the end in both directions.

Fixing this causes numerous tests to fail, due to:

1. the UNO services calling themselves with already converted style
   names, which are then translated a second time, which fails now.
   (or calling the wrong function like SwXStyleFamily::getByIndex())

2. many tests call the UNO API with UINames instead of ProgNames

3. somehow the writerfilter import is also changed, causing failures in
   e.g. testTdf113182 and testTdf104492

4. buggy code elsewhere (lcl_getUsedPageStyles()), problem similar to
   1., for PageDescs

5. potentially more buggy code yet to be discovered (definitely table
   styles, forgot which test that was)

So limit this fix for now to only paragraph styles, and don't do it in
writerfilter import, now at least sw.check passes.

Change-Id: I5cbdf3e174622e83f9af8787c3671b88c0e37bac
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/177858
Reviewed-by: Thorsten Behrens <thorsten.behrens@allotropia.de>
Tested-by: Jenkins
This commit is contained in:
Michael Stahl 2024-12-05 12:22:14 +01:00 committed by Thorsten Behrens
parent 418a55e0dc
commit bd727654ec
8 changed files with 55 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
sw/inc/SwStyleNameMapper.hxx
View file

@ -89,7 +89,7 @@ public:
// This gets the UI Name from the programmatic name
static const OUString& GetUIName(const OUString& rName, SwGetPoolIdFromName);
static void FillUIName(const OUString& rName, OUString& rFillName,
SwGetPoolIdFromName);
SwGetPoolIdFromName, bool bBugfix = false);
// Get the programmatic Name from the UI name
static const OUString& GetProgName(const OUString& rName,

2
sw/qa/extras/layout/layout3.cxx
View file

@ -794,7 +794,7 @@ CPPUNIT_TEST_FIXTURE(SwLayoutWriter3, testTdf155177)
createSwDoc("tdf155177-1-min.odt");
uno::Reference<beans::XPropertySet> xStyle(
getStyles(u"ParagraphStyles"_ustr)->getByName(u"Body Text"_ustr), uno::UNO_QUERY_THROW);
getStyles(u"ParagraphStyles"_ustr)->getByName(u"Text body"_ustr), uno::UNO_QUERY_THROW);
CPPUNIT_ASSERT_EQUAL(sal_Int32(210), getProperty<sal_Int32>(xStyle, u"ParaTopMargin"_ustr));
{

4
sw/qa/extras/ooxmlexport/ooxmlexport9.cxx
View file

@ -481,7 +481,7 @@ DECLARE_OOXMLEXPORT_TEST(testKern, "kern.docx")
// This failed: kerning was also enabled for the second paragraph.
CPPUNIT_ASSERT(!getProperty<bool>(getRun(getParagraph(2), 1), u"CharAutoKerning"_ustr));
uno::Reference<beans::XPropertySet> xStyle(getStyles(u"ParagraphStyles"_ustr)->getByName(u"Default Paragraph Style"_ustr), uno::UNO_QUERY);
uno::Reference<beans::XPropertySet> xStyle(getStyles(u"ParagraphStyles"_ustr)->getByName(u"Standard"_ustr), uno::UNO_QUERY);
//tdf107801: kerning normally isn't enabled by default for .docx
CPPUNIT_ASSERT_EQUAL_MESSAGE("AutoKern should be false", false, getProperty<bool>(xStyle, u"CharAutoKerning"_ustr));
}
@ -491,7 +491,7 @@ DECLARE_OOXMLEXPORT_TEST(testTdf89377, "tdf89377_tableWithBreakBeforeParaStyle.d
// the paragraph style should set table's text-flow break-before-page
CPPUNIT_ASSERT_EQUAL( 3, getPages() );
uno::Reference<beans::XPropertySet> xStyle(getStyles(u"ParagraphStyles"_ustr)->getByName(u"Default Paragraph Style"_ustr), uno::UNO_QUERY);
uno::Reference<beans::XPropertySet> xStyle(getStyles(u"ParagraphStyles"_ustr)->getByName(u"Standard"_ustr), uno::UNO_QUERY);
//tdf107801: kerning info wasn't exported previously.
CPPUNIT_ASSERT_EQUAL_MESSAGE("AutoKern should be true", true, getProperty<bool>(xStyle, u"CharAutoKerning"_ustr));
}

2
sw/qa/extras/ooxmlimport/ooxmlimport.cxx
View file

@ -734,7 +734,7 @@ CPPUNIT_TEST_FIXTURE(Test, testN820504)
uno::Reference<style::XStyleFamiliesSupplier> xFamiliesSupplier(mxComponent, uno::UNO_QUERY);
uno::Reference<container::XNameAccess> xFamiliesAccess = xFamiliesSupplier->getStyleFamilies();
uno::Reference<container::XNameAccess> xStylesAccess(xFamiliesAccess->getByName(u"ParagraphStyles"_ustr), uno::UNO_QUERY);
uno::Reference<beans::XPropertySet> xStyle(xStylesAccess->getByName(u"Default Paragraph Style"_ustr), uno::UNO_QUERY);
uno::Reference<beans::XPropertySet> xStyle(xStylesAccess->getByName(u"Standard"_ustr), uno::UNO_QUERY);
// The problem was that the CharColor was set to AUTO (-1) even if we have some default char color set
CPPUNIT_ASSERT_EQUAL(Color(0x3da7bb), getProperty<Color>(xStyle, u"CharColor"_ustr));

3
sw/qa/extras/rtfexport/rtfexport4.cxx
View file

@ -681,8 +681,7 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf136587_noStyleName)
getProperty<sal_Int16>(xStyleProps, u"ParaAdjust"_ustr));
// The problem was that the default style wasn't imported at all, so the fontsize was only 12.
xStyleProps.set(paragraphStyles->getByName(u"Default Paragraph Style"_ustr),
uno::UNO_QUERY_THROW);
xStyleProps.set(paragraphStyles->getByName(u"Standard"_ustr), uno::UNO_QUERY_THROW);
CPPUNIT_ASSERT_EQUAL(32.0f, getProperty<float>(xStyleProps, u"CharHeight"_ustr));
};
createSwDoc("tdf136587_noStyleName.rtf");

3
sw/qa/extras/uiwriter/uiwriter2.cxx
View file

@ -2578,8 +2578,7 @@ CPPUNIT_TEST_FIXTURE(SwUiWriterTest2, testRTLparaStyle_LocaleArabic)
saveAndReload(u"Office Open XML Text"_ustr);
uno::Reference<beans::XPropertySet> xPageStyle(
getStyles(u"ParagraphStyles"_ustr)->getByName(u"Default Paragraph Style"_ustr),
uno::UNO_QUERY_THROW);
getStyles(u"ParagraphStyles"_ustr)->getByName(u"Standard"_ustr), uno::UNO_QUERY_THROW);
// Test the text Direction value for the -none- based paragraph styles
CPPUNIT_ASSERT_EQUAL_MESSAGE("RTL Writing Mode", sal_Int32(1),
getProperty<sal_Int32>(xPageStyle, u"WritingMode"_ustr));

31
sw/source/core/doc/SwStyleNameMapper.cxx
View file

@ -263,10 +263,18 @@ void SwStyleNameMapper::FillProgName(
rFillName = rName;
if (nId == USHRT_MAX )
{
// It isn't ...make sure the suffix isn't already " (user)"...if it is,
// we need to add another one
if (lcl_SuffixIsUser(rFillName))
rFillName += " (user)";
if (eFlags == SwGetPoolIdFromName::TxtColl)
{
// check if it has a " (user)" suffix, if so remove it
lcl_CheckSuffixAndDelete(rFillName);
}
else // FIXME don't do this
{
// It isn't ...make sure the suffix isn't already " (user)"...if it is,
// we need to add another one
if (lcl_SuffixIsUser(rFillName))
rFillName += " (user)";
}
}
else
{
@ -287,7 +295,7 @@ void SwStyleNameMapper::FillProgName(
// Get the UI name from the programmatic name in rName and put it into rFillName
void SwStyleNameMapper::FillUIName(
const OUString& rName, OUString& rFillName,
SwGetPoolIdFromName const eFlags)
SwGetPoolIdFromName const eFlags, bool const bBugfix)
{
OUString aName = rName;
if (eFlags == SwGetPoolIdFromName::ChrFmt && rName == "Standard")
@ -297,8 +305,17 @@ void SwStyleNameMapper::FillUIName(
if ( nId == USHRT_MAX )
{
rFillName = aName;
// aName isn't in our Prog name table...check if it has a " (user)" suffix, if so remove it
lcl_CheckSuffixAndDelete ( rFillName );
if (eFlags != SwGetPoolIdFromName::TxtColl || // FIXME do it for all ids
!bBugfix || // TODO why does it change DOCX imports
GetPoolIdFromUIName(aName, eFlags) == USHRT_MAX)
{
// aName isn't in our Prog name table...check if it has a " (user)" suffix, if so remove it
lcl_CheckSuffixAndDelete(rFillName);
}
else
{
rFillName += " (user)";
}
}
else
{

37
sw/source/core/unocore/unostyle.cxx
View file

@ -915,7 +915,7 @@ uno::Any SwXStyleFamily::getByIndex(sal_Int32 nIndex)
OUString sStyleName;
try
{
SwStyleNameMapper::FillUIName(m_rEntry.translateIndex(nIndex), sStyleName);
SwStyleNameMapper::FillProgName(m_rEntry.translateIndex(nIndex), sStyleName);
} catch(...) {}
if (sStyleName.isEmpty())
GetCountOrName(&sStyleName, nIndex);
@ -957,7 +957,8 @@ rtl::Reference<SwXBaseStyle> SwXStyleFamily::getStyleByName(const OUString& rNam
{
SolarMutexGuard aGuard;
OUString sStyleName;
SwStyleNameMapper::FillUIName(rName, sStyleName, m_rEntry.poolId());
SwStyleNameMapper::FillUIName(rName, sStyleName, m_rEntry.poolId(),
m_pDocShell && !m_pDocShell->GetDoc()->IsInWriterfilterImport());
if(!m_pBasePool)
throw uno::RuntimeException();
SfxStyleSheetBase* pBase = m_pBasePool->Find(sStyleName, m_rEntry.family());
@ -1012,7 +1013,8 @@ sal_Bool SwXStyleFamily::hasByName(const OUString& rName)
if(!m_pBasePool)
throw uno::RuntimeException();
OUString sStyleName;
SwStyleNameMapper::FillUIName(rName, sStyleName, m_rEntry.poolId());
SwStyleNameMapper::FillUIName(rName, sStyleName, m_rEntry.poolId(),
m_pDocShell && !m_pDocShell->GetDoc()->IsInWriterfilterImport());
SfxStyleSheetBase* pBase = m_pBasePool->Find(sStyleName, m_rEntry.family());
return nullptr != pBase;
}
@ -1023,7 +1025,8 @@ void SwXStyleFamily::insertStyleByName(const OUString& rName, const rtl::Referen
if(!m_pBasePool)
throw uno::RuntimeException();
OUString sStyleName;
SwStyleNameMapper::FillUIName(rName, sStyleName, m_rEntry.poolId());
SwStyleNameMapper::FillUIName(rName, sStyleName, m_rEntry.poolId(),
m_pDocShell && !m_pDocShell->GetDoc()->IsInWriterfilterImport());
SfxStyleSheetBase* pBase = m_pBasePool->Find(sStyleName, m_rEntry.family());
if (pBase)
throw container::ElementExistException();
@ -1036,7 +1039,8 @@ void SwXStyleFamily::insertByName(const OUString& rName, const uno::Any& rElemen
if(!m_pBasePool)
throw uno::RuntimeException();
OUString sStyleName;
SwStyleNameMapper::FillUIName(rName, sStyleName, m_rEntry.poolId());
SwStyleNameMapper::FillUIName(rName, sStyleName, m_rEntry.poolId(),
m_pDocShell && !m_pDocShell->GetDoc()->IsInWriterfilterImport());
SfxStyleSheetBase* pBase = m_pBasePool->Find(sStyleName, m_rEntry.family());
if (pBase)
throw container::ElementExistException();
@ -1099,7 +1103,8 @@ void SwXStyleFamily::replaceByName(const OUString& rName, const uno::Any& rEleme
if(!m_pBasePool)
throw uno::RuntimeException();
OUString sStyleName;
SwStyleNameMapper::FillUIName(rName, sStyleName, m_rEntry.poolId());
SwStyleNameMapper::FillUIName(rName, sStyleName, m_rEntry.poolId(),
m_pDocShell && !m_pDocShell->GetDoc()->IsInWriterfilterImport());
SfxStyleSheetBase* pBase = m_pBasePool->Find(sStyleName, m_rEntry.family());
// replacements only for userdefined styles
if(!pBase)
@ -1160,7 +1165,8 @@ void SwXStyleFamily::removeByName(const OUString& rName)
if(!m_pBasePool)
throw uno::RuntimeException();
OUString sName;
SwStyleNameMapper::FillUIName(rName, sName, m_rEntry.poolId());
SwStyleNameMapper::FillUIName(rName, sName, m_rEntry.poolId(),
m_pDocShell && !m_pDocShell->GetDoc()->IsInWriterfilterImport());
SfxStyleSheetBase* pBase = m_pBasePool->Find(sName, m_rEntry.family());
if(!pBase)
throw container::NoSuchElementException();
@ -1447,7 +1453,8 @@ void SwXStyle::setParentStyle(const OUString& rParentStyle)
{
SolarMutexGuard aGuard;
OUString sParentStyle;
SwStyleNameMapper::FillUIName(rParentStyle, sParentStyle, lcl_GetSwEnumFromSfxEnum ( m_rEntry.family()) );
SwStyleNameMapper::FillUIName(rParentStyle, sParentStyle, lcl_GetSwEnumFromSfxEnum ( m_rEntry.family()),
m_pDoc && !m_pDoc->IsInWriterfilterImport());
if(!m_pBasePool)
{
if(!m_bIsDescriptor)
@ -1455,7 +1462,7 @@ void SwXStyle::setParentStyle(const OUString& rParentStyle)
m_sParentStyleName = sParentStyle;
try
{
const auto aAny = m_xStyleFamily->getByName(sParentStyle);
const auto aAny = m_xStyleFamily->getByName(rParentStyle);
m_xStyleData = aAny.get<decltype(m_xStyleData)>();
}
catch(...)
@ -1753,7 +1760,8 @@ void SwXStyle::SetPropertyValue<FN_UNO_FOLLOW_STYLE>(const SfxItemPropertyMapEnt
return;
const auto sValue(rValue.get<OUString>());
OUString aString;
SwStyleNameMapper::FillUIName(sValue, aString, m_rEntry.poolId());
SwStyleNameMapper::FillUIName(sValue, aString, m_rEntry.poolId(),
m_pDoc && !m_pDoc->IsInWriterfilterImport());
o_rStyleBase.getNewBase()->SetFollow(aString);
}
@ -1767,7 +1775,8 @@ void SwXStyle::SetPropertyValue<FN_UNO_LINK_STYLE>(const SfxItemPropertyMapEntry
return;
const auto sValue(rValue.get<OUString>());
OUString aString;
SwStyleNameMapper::FillUIName(sValue, aString, m_rEntry.poolId());
SwStyleNameMapper::FillUIName(sValue, aString, m_rEntry.poolId(),
m_pDoc && !m_pDoc->IsInWriterfilterImport());
o_rStyleBase.getNewBase()->SetLink(aString);
}
@ -1849,7 +1858,8 @@ void SwXStyle::SetPropertyValue<FN_UNO_PARA_STYLE_CONDITIONS>(const SfxItemPrope
const OUString sValue(rNamedValue.Value.get<OUString>());
// get UI style name from programmatic style name
OUString aStyleName;
SwStyleNameMapper::FillUIName(sValue, aStyleName, lcl_GetSwEnumFromSfxEnum(m_rEntry.family()));
SwStyleNameMapper::FillUIName(sValue, aStyleName, lcl_GetSwEnumFromSfxEnum(m_rEntry.family()),
m_pDoc && !m_pDoc->IsInWriterfilterImport());
// check for correct context and style name
const auto nIdx(GetCommandContextIndex(rNamedValue.Name));
@ -1893,7 +1903,8 @@ void SwXStyle::SetPropertyValue<SID_SWREGISTER_COLLECTION>(const SfxItemProperty
aReg.SetWhich(SID_SWREGISTER_MODE);
o_rStyleBase.GetItemSet().Put(aReg);
OUString aString;
SwStyleNameMapper::FillUIName(sName, aString, SwGetPoolIdFromName::TxtColl);
SwStyleNameMapper::FillUIName(sName, aString, SwGetPoolIdFromName::TxtColl,
m_pDoc && !m_pDoc->IsInWriterfilterImport());
o_rStyleBase.GetItemSet().Put(SfxStringItem(SID_SWREGISTER_COLLECTION, aString ) );
}
template<>