From 7d08664be4d9a949718ce66e3bc974105a10284c Mon Sep 17 00:00:00 2001 From: Maxim Monastirsky Date: Mon, 3 Feb 2020 00:28:04 +0200 Subject: [PATCH] tdf#129568 Rename the default table style As it turns out, UI names of table styles are leaking into documents, and changing those names actually breaks the import of documents from previous versions. The problem is that a table style itself is saved using its programmatic name, but is referenced by tables using its UI name. So after changing the UI name, these no longer map. It's still possible to manually reapply the style, but if not doing this and just saving, the style and its child cell styles will be silently lost. Moreover, if the given document is of fodt type, it's not even possible to save it (even not as "save as" to odt). Obviously, the issue isn't just with renaming. The same happens also with documents created with a different UI language (even English). Fortunately, up to now English UI names were identical to the programmatic ones. So the first thing we can do is to accept both kinds of names for table:template-name. This way, we solved the problem for documents created in an English UI, and in addition made them work in non-English UI (unlike before). As for export, we want to always writes programmatic names, so newly edited documents will continue to work regardless of future UI changes or UI language switching (and also stay compatible with older versions). For the fodt export failure, changed the order of things in SwXTextTableStyle::replaceByName, as setting a new box breaks SwXTextCellStyle::getName in this specific case. Also changed cell styles to be named using the parent style's programmatic name, so new documents won't have this problem when opened in older versions. This also fixed part of the PythonTest_sw_python failure. The remaining PythonTest_sw_python failure was about the "TableTemplateName" UNO API property of a table, which didn't work with programmatic names. That's a real bug by itself, and was fixed. Also an explicit test was added, to make sure the API always returns the programmatic name. Finally, an odf export test was added. It tests files with both old-style UI names, and new-style programmatic names. Styles should be correctly imported, used by the table, and survive export. Change-Id: I45dfda193813fea184dc42e5e75544ebc05d4a92 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/87826 Reviewed-by: Maxim Monastirsky Tested-by: Maxim Monastirsky --- sw/inc/strings.hrc | 2 +- sw/qa/extras/odfexport/data/tdf129568-ui.fodt | 368 ++++++++++++++++++ sw/qa/extras/odfexport/data/tdf129568.fodt | 368 ++++++++++++++++++ sw/qa/extras/odfexport/odfexport.cxx | 16 + sw/qa/python/check_table.py | 5 + sw/source/core/unocore/unostyle.cxx | 43 +- sw/source/core/unocore/unotbl.cxx | 5 +- sw/source/filter/xml/xmltble.cxx | 7 +- sw/source/filter/xml/xmltbli.cxx | 5 +- sw/source/uibase/app/docstyle.cxx | 4 +- 10 files changed, 794 insertions(+), 29 deletions(-) create mode 100644 sw/qa/extras/odfexport/data/tdf129568-ui.fodt create mode 100644 sw/qa/extras/odfexport/data/tdf129568.fodt diff --git a/sw/inc/strings.hrc b/sw/inc/strings.hrc index a1297ed14d6e..b276d5888c75 100644 --- a/sw/inc/strings.hrc +++ b/sw/inc/strings.hrc @@ -221,7 +221,7 @@ #define STR_COLUMN_VALUESET_ITEM3 NC_("STR_COLUMN_VALUESET_ITEM3", "2 columns with different size (left > right)") #define STR_COLUMN_VALUESET_ITEM4 NC_("STR_COLUMN_VALUESET_ITEM4", "2 columns with different size (left < right)") // Table styles, Writer internal, others are taken from Svx -#define STR_TABSTYLE_DEFAULT NC_("STR_TABSTYLE_DEFAULT", "Default Style") +#define STR_TABSTYLE_DEFAULT NC_("STR_TABSTYLE_DEFAULT", "Default Table Style") #define STR_PARAGRAPHSTYLEFAMILY NC_("STR_PARAGRAPHSTYLEFAMILY", "Paragraph Styles") #define STR_CHARACTERSTYLEFAMILY NC_("STR_CHARACTERSTYLEFAMILY", "Character Styles") diff --git a/sw/qa/extras/odfexport/data/tdf129568-ui.fodt b/sw/qa/extras/odfexport/data/tdf129568-ui.fodt new file mode 100644 index 000000000000..8958bd685341 --- /dev/null +++ b/sw/qa/extras/odfexport/data/tdf129568-ui.fodt @@ -0,0 +1,368 @@ + + + + 2020-02-03T11:01:15.2025491202020-02-03T11:02:08.562696933PT55S2LibreOffice/6.3.4.2.0$Linux_X86_64 LibreOffice_project/30$Build-2 + + + 0 + 0 + 25137 + 13522 + true + false + + + view2 + 4165 + 2616 + 0 + 0 + 25135 + 13520 + 0 + 1 + false + 100 + false + false + + + + + false + true + true + true + 0 + true + true + + false + false + true + false + false + false + true + true + false + false + false + false + false + false + false + false + false + true + true + false + false + true + 1284982 + false + + false + true + false + false + false + true + false + false + false + true + 1284982 + + true + false + false + true + false + true + true + false + true + false + false + false + 0 + false + true + high-resolution + false + false + false + true + false + false + false + true + true + + true + false + false + true + false + false + false + + true + false + false + 1 + true + false + false + 0 + false + false + + + false + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/sw/qa/extras/odfexport/data/tdf129568.fodt b/sw/qa/extras/odfexport/data/tdf129568.fodt new file mode 100644 index 000000000000..30256a6cd5be --- /dev/null +++ b/sw/qa/extras/odfexport/data/tdf129568.fodt @@ -0,0 +1,368 @@ + + + + 2020-02-03T11:01:15.2025491202020-02-03T11:02:08.562696933PT55S2LibreOffice/6.3.4.2.0$Linux_X86_64 LibreOffice_project/30$Build-2 + + + 0 + 0 + 25137 + 13522 + true + false + + + view2 + 4165 + 2616 + 0 + 0 + 25135 + 13520 + 0 + 1 + false + 100 + false + false + + + + + false + true + true + true + 0 + true + true + + false + false + true + false + false + false + true + true + false + false + false + false + false + false + false + false + false + true + true + false + false + true + 1284982 + false + + false + true + false + false + false + true + false + false + false + true + 1284982 + + true + false + false + true + false + true + true + false + true + false + false + false + 0 + false + true + high-resolution + false + false + false + true + false + false + false + true + true + + true + false + false + true + false + false + false + + true + false + false + 1 + true + false + false + 0 + false + false + + + false + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/sw/qa/extras/odfexport/odfexport.cxx b/sw/qa/extras/odfexport/odfexport.cxx index 7c21a6ca6d35..a7af9a7f3553 100644 --- a/sw/qa/extras/odfexport/odfexport.cxx +++ b/sw/qa/extras/odfexport/odfexport.cxx @@ -2007,6 +2007,22 @@ DECLARE_ODFEXPORT_TEST(testTableStyles5, "table_styles_5.odt") } +DECLARE_ODFEXPORT_TEST(testTdf129568, "tdf129568.fodt") +{ + // Test that export doesn't fail, and that style is imported and in use. + uno::Reference xStyle(getStyles("CellStyles")->getByName("Default Style.1"), uno::UNO_QUERY); + CPPUNIT_ASSERT(xStyle->isInUse()); + CPPUNIT_ASSERT_EQUAL(sal_Int32(0xffff00), getProperty(xStyle, "BackColor")); +} + +DECLARE_ODFEXPORT_TEST(testTdf129568ui, "tdf129568-ui.fodt") +{ + // Same as above, but styles referenced by UI name. + uno::Reference xStyle(getStyles("CellStyles")->getByName("Default Style.1"), uno::UNO_QUERY); + CPPUNIT_ASSERT(xStyle->isInUse()); + CPPUNIT_ASSERT_EQUAL(sal_Int32(0xffff00), getProperty(xStyle, "BackColor")); +} + DECLARE_ODFEXPORT_TEST(testImageMimetype, "image-mimetype.odt") { // Test that the loext:mimetype attribute is written for exported images, tdf#109202 diff --git a/sw/qa/python/check_table.py b/sw/qa/python/check_table.py index 9cb3b829d4d3..b06f3caa4d7d 100644 --- a/sw/qa/python/check_table.py +++ b/sw/qa/python/check_table.py @@ -547,6 +547,11 @@ class CheckTable(unittest.TestCase): xDoc = CheckTable._uno.openEmptyWriterDoc() xTable = xDoc.createInstance("com.sun.star.text.TextTable") xTable.initialize(1, 1) + xCursor = xDoc.Text.createTextCursor() + xDoc.Text.insertTextContent(xCursor, xTable, True) + # should return programmatic name, even though UI name was set. + xTable.setPropertyValue("TableTemplateName", "Default Table Style") + self.assertEqual(xTable.getPropertyValue("TableTemplateName"), "Default Style") xTable.setPropertyValue("TableTemplateName", "Default") self.assertEqual(xTable.getPropertyValue("TableTemplateName"), "Default") xTable.setPropertyValue("TableTemplateName", "other_style") diff --git a/sw/source/core/unocore/unostyle.cxx b/sw/source/core/unocore/unostyle.cxx index a45d0b9ac601..67da3653b6d3 100644 --- a/sw/source/core/unocore/unostyle.cxx +++ b/sw/source/core/unocore/unostyle.cxx @@ -765,8 +765,8 @@ sal_Int32 lcl_GetCountOrName(const SwDoc& rDoc, OUString* const sal_Int32 nAutoFormat = nIndex / rTableTemplateMap.size(); const sal_Int32 nBoxFormat = rTableTemplateMap[nIndex % rTableTemplateMap.size()]; const SwTableAutoFormat& rTableFormat = rAutoFormats[nAutoFormat]; - *pString = rTableFormat.GetName() - + rTableFormat.GetTableTemplateCellSubName(rTableFormat.GetBoxFormat(nBoxFormat)); + SwStyleNameMapper::FillProgName(rTableFormat.GetName(), *pString, SwGetPoolIdFromName::TabStyle); + *pString += rTableFormat.GetTableTemplateCellSubName(rTableFormat.GetBoxFormat(nBoxFormat)); } else *pString = rDoc.GetCellStyles()[nIndex-nUsedCellStylesCount].GetName(); @@ -1003,8 +1003,10 @@ void XStyleFamily::replaceByName(const OUString& rName, const uno::Any& rElement SolarMutexGuard aGuard; if(!m_pBasePool) throw uno::RuntimeException(); + OUString sStyleName; + SwStyleNameMapper::FillUIName(rName, sStyleName, m_rEntry.m_aPoolId); m_pBasePool->SetSearchMask(m_rEntry.m_eFamily); - SfxStyleSheetBase* pBase = m_pBasePool->Find(rName); + SfxStyleSheetBase* pBase = m_pBasePool->Find(sStyleName); // replacements only for userdefined styles if(!pBase) throw container::NoSuchElementException(); @@ -1012,7 +1014,7 @@ void XStyleFamily::replaceByName(const OUString& rName, const uno::Any& rElement { // handle cell styles, don't call on assigned cell styles (TableStyle child) OUString sParent; - SwBoxAutoFormat* pBoxAutoFormat = SwXTextCellStyle::GetBoxAutoFormat(m_pDocShell, rName, &sParent); + SwBoxAutoFormat* pBoxAutoFormat = SwXTextCellStyle::GetBoxAutoFormat(m_pDocShell, sStyleName, &sParent); if (pBoxAutoFormat && sParent.isEmpty())// if parent exists then this style is assigned to a table style. Don't replace. { uno::Reference xStyle = rElement.get>(); @@ -1020,7 +1022,7 @@ void XStyleFamily::replaceByName(const OUString& rName, const uno::Any& rElement if (!pStyleToReplaceWith) throw lang::IllegalArgumentException(); - pStyleToReplaceWith->setName(rName); + pStyleToReplaceWith->setName(sStyleName); *pBoxAutoFormat = *pStyleToReplaceWith->GetBoxFormat(); pStyleToReplaceWith->SetPhysical(); } @@ -1028,7 +1030,7 @@ void XStyleFamily::replaceByName(const OUString& rName, const uno::Any& rElement else if (SwGetPoolIdFromName::TabStyle == m_rEntry.m_aPoolId) { // handle table styles - SwTableAutoFormat* pTableAutoFormat = SwXTextTableStyle::GetTableAutoFormat(m_pDocShell, rName); + SwTableAutoFormat* pTableAutoFormat = SwXTextTableStyle::GetTableAutoFormat(m_pDocShell, sStyleName); if (pTableAutoFormat) { uno::Reference xStyle = rElement.get>(); @@ -1036,7 +1038,7 @@ void XStyleFamily::replaceByName(const OUString& rName, const uno::Any& rElement if (!pStyleToReplaceWith) throw lang::IllegalArgumentException(); - pStyleToReplaceWith->setName(rName); + pStyleToReplaceWith->setName(sStyleName); *pTableAutoFormat = *pStyleToReplaceWith->GetTableFormat(); pStyleToReplaceWith->SetPhysical(); } @@ -4455,17 +4457,9 @@ sal_Bool SAL_CALL SwXTextTableStyle::isInUse() { if (!pFormat->GetInfo(aGetHt)) { - uno::Reference xTable = SwXTextTables::GetObject(*pFormat); - if (xTable.is()) - { - uno::Reference xTablePropertySet(xTable, uno::UNO_QUERY); - OUString sTableTemplateName; - if (xTablePropertySet.is() && (xTablePropertySet->getPropertyValue("TableTemplateName") >>= sTableTemplateName) - && sTableTemplateName == m_pTableAutoFormat->GetName()) - { - return true; - } - } + SwTable* pTable = SwTable::FindTable(pFormat); + if (pTable->GetTableStyleName() == m_pTableAutoFormat->GetName()) + return true; } } @@ -4606,11 +4600,11 @@ void SAL_CALL SwXTextTableStyle::replaceByName(const OUString& rName, const uno: // move SwBoxAutoFormat to dest. SwTableAutoFormat m_pTableAutoFormat->SetBoxFormat(*pStyleToReplaceWith->GetBoxFormat(), nBoxFormat); + // remove unassigned SwBoxAutoFormat, which is not anymore in use anyways + m_pDocShell->GetDoc()->GetCellStyles().RemoveBoxFormat(xStyle->getName()); // make SwXTextCellStyle use new, moved SwBoxAutoFormat pStyleToReplaceWith->SetBoxFormat(&m_pTableAutoFormat->GetBoxFormat(nBoxFormat)); m_pTableAutoFormat->GetBoxFormat(nBoxFormat).SetXObject(xStyle); - // remove unassigned SwBoxAutoFormat, which is not anymore in use anyways - m_pDocShell->GetDoc()->GetCellStyles().RemoveBoxFormat(xStyle->getName()); // make this SwXTextTableStyle use new SwXTextCellStyle m_aCellStyles[nCellStyle] = xStyle; } @@ -4726,6 +4720,7 @@ SwBoxAutoFormat* SwXTextCellStyle::GetBoxAutoFormat(SwDocShell* pDocShell, const if (rTableTemplateMap.size() <= o3tl::make_unsigned(nTemplateIndex)) return nullptr; + SwStyleNameMapper::FillUIName(sParentName, sParentName, SwGetPoolIdFromName::TabStyle); SwTableAutoFormat* pTableAutoFormat = pDocShell->GetDoc()->GetTableStyles().FindAutoFormat(sParentName); if (!pTableAutoFormat) return nullptr; @@ -4826,16 +4821,18 @@ OUString SAL_CALL SwXTextCellStyle::getName() // if style is physical then we request a name from doc if (m_bPhysical) { - OUString sParentStyle; - SwStyleNameMapper::FillUIName(m_sParentStyle, sParentStyle, SwGetPoolIdFromName::TabStyle); - SwTableAutoFormat* pTableFormat = m_pDocShell->GetDoc()->GetTableStyles().FindAutoFormat(sParentStyle); + SwTableAutoFormat* pTableFormat = m_pDocShell->GetDoc()->GetTableStyles().FindAutoFormat(m_sParentStyle); if (!pTableFormat) { // if auto format is not found as a child of table formats, look in SwDoc cellstyles sName = m_pDocShell->GetDoc()->GetCellStyles().GetBoxFormatName(*m_pBoxAutoFormat); } else + { + OUString sParentStyle; + SwStyleNameMapper::FillProgName(m_sParentStyle, sParentStyle, SwGetPoolIdFromName::TabStyle); sName = sParentStyle + pTableFormat->GetTableTemplateCellSubName(*m_pBoxAutoFormat); + } } else sName = m_sName; diff --git a/sw/source/core/unocore/unotbl.cxx b/sw/source/core/unocore/unotbl.cxx index 779b1078c528..9a1ff2987786 100644 --- a/sw/source/core/unocore/unotbl.cxx +++ b/sw/source/core/unocore/unotbl.cxx @@ -2841,6 +2841,7 @@ void SwXTextTable::setPropertyValue(const OUString& rPropertyName, const uno::An OUString sName; if (!(aValue >>= sName)) break; + SwStyleNameMapper::FillUIName(sName, sName, SwGetPoolIdFromName::TabStyle); pTable->SetTableStyleName(sName); SwDoc* pDoc = pFormat->GetDoc(); pDoc->GetDocShell()->GetFEShell()->UpdateTableStyleFormatting(pTable->GetTableNode()); @@ -3083,7 +3084,9 @@ uno::Any SwXTextTable::getPropertyValue(const OUString& rPropertyName) case FN_UNO_TABLE_TEMPLATE_NAME: { SwTable* pTable = SwTable::FindTable(pFormat); - aRet <<= pTable->GetTableStyleName(); + OUString sName; + SwStyleNameMapper::FillProgName(pTable->GetTableStyleName(), sName, SwGetPoolIdFromName::TabStyle); + aRet <<= sName; } break; diff --git a/sw/source/filter/xml/xmltble.cxx b/sw/source/filter/xml/xmltble.cxx index fc1602e982b2..b2c92a780d30 100644 --- a/sw/source/filter/xml/xmltble.cxx +++ b/sw/source/filter/xml/xmltble.cxx @@ -48,6 +48,7 @@ #include #include #include +#include using namespace ::com::sun::star; using namespace ::com::sun::star::uno; @@ -1087,7 +1088,11 @@ void SwXMLExport::ExportTable( const SwTableNode& rTableNd ) // table:template-name= if (!rTable.GetTableStyleName().isEmpty()) - AddAttribute(XML_NAMESPACE_TABLE, XML_TEMPLATE_NAME, rTable.GetTableStyleName()); + { + OUString sStyleName; + SwStyleNameMapper::FillProgName(rTable.GetTableStyleName(), sStyleName, SwGetPoolIdFromName::TabStyle); + AddAttribute(XML_NAMESPACE_TABLE, XML_TEMPLATE_NAME, sStyleName); + } sal_uInt16 nPrefix = XML_NAMESPACE_TABLE; if (const SwFrameFormat* pFlyFormat = rTableNd.GetFlyFormat()) diff --git a/sw/source/filter/xml/xmltbli.cxx b/sw/source/filter/xml/xmltbli.cxx index 4d617b8991e5..d49f23a76ff6 100644 --- a/sw/source/filter/xml/xmltbli.cxx +++ b/sw/source/filter/xml/xmltbli.cxx @@ -60,6 +60,7 @@ #include "xmltbli.hxx" #include #include +#include #include #include @@ -2613,9 +2614,11 @@ void SwXMLTableContext::MakeTable() sal_uInt8 nPercentWidth = 0U; + OUString sStyleName; + SwStyleNameMapper::FillUIName( m_aTemplateName, sStyleName, SwGetPoolIdFromName::TabStyle ); + m_pTableNode->GetTable().SetTableStyleName( sStyleName ); m_pTableNode->GetTable().SetRowsToRepeat( m_nHeaderRows ); m_pTableNode->GetTable().SetTableModel( !m_bHasSubTables ); - m_pTableNode->GetTable().SetTableStyleName( m_aTemplateName ); const SfxItemSet *pAutoItemSet = nullptr; if( !m_aStyleName.isEmpty() && diff --git a/sw/source/uibase/app/docstyle.cxx b/sw/source/uibase/app/docstyle.cxx index db623a8d9277..de62518f9c5d 100644 --- a/sw/source/uibase/app/docstyle.cxx +++ b/sw/source/uibase/app/docstyle.cxx @@ -344,7 +344,7 @@ static const SwBoxAutoFormat* lcl_FindCellStyle(SwDoc& rDoc, const OUString& rNa const sal_uInt32 nBoxIndex = aTableTemplateMap[nBoxFormat]; const SwBoxAutoFormat& rBoxFormat = rTableStyle.GetBoxFormat(nBoxIndex); OUString sBoxFormatName; - SwStyleNameMapper::FillProgName(rTableStyle.GetName(), sBoxFormatName, SwGetPoolIdFromName::CellStyle); + SwStyleNameMapper::FillProgName(rTableStyle.GetName(), sBoxFormatName, SwGetPoolIdFromName::TabStyle); sBoxFormatName += rTableStyle.GetTableTemplateCellSubName(rBoxFormat); if (rName == sBoxFormatName) pFormat = &rBoxFormat; @@ -3069,7 +3069,7 @@ SfxStyleSheetBase* SwStyleSheetIterator::First() const sal_uInt32 nBoxIndex = aTableTemplateMap[nBoxFormat]; const SwBoxAutoFormat& rBoxFormat = rTableStyle.GetBoxFormat(nBoxIndex); OUString sBoxFormatName; - SwStyleNameMapper::FillProgName(rTableStyle.GetName(), sBoxFormatName, SwGetPoolIdFromName::CellStyle); + SwStyleNameMapper::FillProgName(rTableStyle.GetName(), sBoxFormatName, SwGetPoolIdFromName::TabStyle); sBoxFormatName += rTableStyle.GetTableTemplateCellSubName(rBoxFormat); aLst.Append( cCELLSTYLE, sBoxFormatName ); }