editeng/source/items/textitem.cxx |   48 ++++++++++++++++++++++++++++++++++
 include/editeng/fontitem.hxx      |    1 
 sw/source/filter/xml/xmlfonte.cxx |   18 ++++++++----
 xmloff/qa/unit/style.cxx          |   53 +++++++++++++++++++++++++++++++++-----
 4 files changed, 108 insertions(+), 12 deletions(-)

New commits:
commit 610374f668e2239234f11766fb272b242df1a2ab
Author:     Miklos Vajna <vmik...@collabora.com>
AuthorDate: Thu Sep 9 13:04:01 2021 +0200
Commit:     Miklos Vajna <vmik...@collabora.com>
CommitDate: Fri Sep 10 10:02:30 2021 +0200

    ODT export: order <style:font-face> elements inside <office:font-face-decls>
    
    This builds on top of commit 92471550b8c43d8ff0cef8b414884d697edf9e63
    (ODF export: sort <style:font-face> elements based on the style:name
    attribute, 2021-03-11), the additional problem was that the style:name
    attribute already has number suffixes to have unique names for fonts
    where the style name would match.
    
    This means that even if we sort the container right before writing the
    elements, which font gets the number suffix depends on the insert order.
    
    Fix this by additionally sorting the font items before insertion, given
    that a single call-site does all the insertion, at least for Writer
    documents. This is required as SfxItemPool::GetItemSurrogates() exposes
    a container which is based on SfxPoolItemArray_Impl, which uses an
    o3tl::sorted_vector<> of pointers, so effectively unsorted, the order
    depends on the pointer address of the font items.
    
    (cherry picked from commit 7a8bb65e1b8dc7fdd7f89c8c546e71e4208da574)
    
    Change-Id: I46569b40796243f7f95b92870504c2023b2ce943

diff --git a/editeng/source/items/textitem.cxx 
b/editeng/source/items/textitem.cxx
index 2e2cf4fe7604..e6817adb9a8d 100644
--- a/editeng/source/items/textitem.cxx
+++ b/editeng/source/items/textitem.cxx
@@ -153,6 +153,24 @@ bool SvxFontListItem::GetPresentation
 
 // class SvxFontItem -----------------------------------------------------
 
+namespace
+{
+sal_Int32 CompareTo(sal_Int32 nA, sal_Int32 nB)
+{
+    if (nA < nB)
+    {
+        return -1;
+    }
+
+    if (nA > nB)
+    {
+        return 1;
+    }
+
+    return 0;
+}
+}
+
 SvxFontItem::SvxFontItem( const sal_uInt16 nId ) :
     SfxPoolItem( nId )
 {
@@ -290,6 +308,36 @@ bool SvxFontItem::operator==( const SfxPoolItem& rAttr ) 
const
     return bRet;
 }
 
+bool SvxFontItem::operator<(const SfxPoolItem& rCmp) const
+{
+    const auto& rOther = static_cast<const SvxFontItem&>(rCmp);
+    sal_Int32 nRet = GetFamilyName().compareTo(rOther.GetFamilyName());
+    if (nRet != 0)
+    {
+        return nRet < 0;
+    }
+
+    nRet = GetStyleName().compareTo(rOther.GetStyleName());
+    if (nRet != 0)
+    {
+        return nRet < 0;
+    }
+
+    nRet = CompareTo(GetFamily(), rOther.GetFamily());
+    if (nRet != 0)
+    {
+        return nRet < 0;
+    }
+
+    nRet = CompareTo(GetPitch(), rOther.GetPitch());
+    if (nRet != 0)
+    {
+        return nRet < 0;
+    }
+
+    return GetCharSet() < rOther.GetCharSet();
+}
+
 SvxFontItem* SvxFontItem::Clone( SfxItemPool * ) const
 {
     return new SvxFontItem( *this );
diff --git a/include/editeng/fontitem.hxx b/include/editeng/fontitem.hxx
index 9a73a051f79e..2ccaade20121 100644
--- a/include/editeng/fontitem.hxx
+++ b/include/editeng/fontitem.hxx
@@ -46,6 +46,7 @@ public:
 
     // "pure virtual Methods" from SfxPoolItem
     virtual bool operator==(const SfxPoolItem& rItem) const override;
+    bool operator<(const SfxPoolItem& rCmp) const override;
     virtual SvxFontItem* Clone(SfxItemPool *pPool = nullptr) const override;
     virtual bool QueryValue(css::uno::Any& rVal, sal_uInt8 nMemberId = 0) 
const override;
     virtual bool PutValue(const css::uno::Any& rVal, sal_uInt8 nMemberId) 
override;
diff --git a/sw/source/filter/xml/xmlfonte.cxx 
b/sw/source/filter/xml/xmlfonte.cxx
index 9b90f94fd419..79207700c204 100644
--- a/sw/source/filter/xml/xmlfonte.cxx
+++ b/sw/source/filter/xml/xmlfonte.cxx
@@ -46,21 +46,27 @@ 
SwXMLFontAutoStylePool_Impl::SwXMLFontAutoStylePool_Impl(SwXMLExport& _rExport,
                                       RES_CHRATR_CTL_FONT };
 
     const SfxItemPool& rPool = _rExport.getDoc()->GetAttrPool();
+    std::vector<const SvxFontItem *> aFonts;
     for(sal_uInt16 nWhichId : aWhichIds)
     {
         const SvxFontItem& rFont =
             static_cast<const SvxFontItem&>(rPool.GetDefaultItem( nWhichId ));
-        Add( rFont.GetFamilyName(), rFont.GetStyleName(),
-             rFont.GetFamily(), rFont.GetPitch(),
-             rFont.GetCharSet() );
+        aFonts.push_back(&rFont);
         for (const SfxPoolItem* pItem : rPool.GetItemSurrogates(nWhichId))
         {
             auto pFont = static_cast<const SvxFontItem *>(pItem);
-            Add( pFont->GetFamilyName(), pFont->GetStyleName(),
-                 pFont->GetFamily(), pFont->GetPitch(),
-                 pFont->GetCharSet() );
+            aFonts.push_back(pFont);
         }
     }
+
+    std::sort(aFonts.begin(), aFonts.end(),
+              [](const SvxFontItem* pA, const SvxFontItem* pB) -> bool { 
return *pA < *pB; });
+    for (const auto& pFont : aFonts)
+    {
+        Add(pFont->GetFamilyName(), pFont->GetStyleName(), pFont->GetFamily(), 
pFont->GetPitch(),
+            pFont->GetCharSet());
+    }
+
     auto const & pDocument = _rExport.getDoc();
 
     m_bEmbedUsedOnly = 
pDocument->getIDocumentSettingAccess().get(DocumentSettingId::EMBED_USED_FONTS);
diff --git a/xmloff/qa/unit/style.cxx b/xmloff/qa/unit/style.cxx
index f859c3619837..1ccffa8a039c 100644
--- a/xmloff/qa/unit/style.cxx
+++ b/xmloff/qa/unit/style.cxx
@@ -19,6 +19,7 @@
 #include <comphelper/propertysequence.hxx>
 #include <unotools/tempfile.hxx>
 #include <unotools/ucbstreamhelper.hxx>
+#include <rtl/character.hxx>
 
 using namespace ::com::sun::star;
 
@@ -82,6 +83,25 @@ CPPUNIT_TEST_FIXTURE(XmloffStyleTest, testFillImageBase64)
     CPPUNIT_ASSERT(xBitmaps->hasByName("libreoffice_0"));
 }
 
+namespace
+{
+struct XmlFont
+{
+    OString aName;
+    OString aFontFamilyGeneric;
+    bool operator<(const XmlFont& rOther) const
+    {
+        sal_Int32 nRet = aName.compareTo(rOther.aName);
+        if (nRet != 0)
+        {
+            return nRet < 0;
+        }
+
+        return aFontFamilyGeneric.compareTo(rOther.aFontFamilyGeneric) < 0;
+    }
+};
+}
+
 CPPUNIT_TEST_FIXTURE(XmloffStyleTest, testFontSorting)
 {
     // Given an empty document with default fonts (Liberation Sans, Lucida 
Sans, etc):
@@ -107,25 +127,46 @@ CPPUNIT_TEST_FIXTURE(XmloffStyleTest, testFontSorting)
         = getXPathNode(pXmlDoc, 
"/office:document-content/office:font-face-decls/style:font-face");
     xmlNodeSetPtr pXmlNodes = pXPath->nodesetval;
     int nNodeCount = xmlXPathNodeSetGetLength(pXmlNodes);
-    std::vector<OString> aXMLNames;
-    std::set<OString> aSortedNames;
+    std::vector<XmlFont> aXMLFonts;
+    std::vector<XmlFont> aSortedFonts;
     for (int i = 0; i < nNodeCount; ++i)
     {
         xmlNodePtr pXmlNode = pXmlNodes->nodeTab[i];
         xmlChar* pName = xmlGetProp(pXmlNode, BAD_CAST("name"));
         OString aName(reinterpret_cast<char const*>(pName));
-        aXMLNames.push_back(aName);
-        aSortedNames.insert(aName);
+
+        // Ignore numbers at the end, those are just appended to make all 
names unique.
+        while 
(rtl::isAsciiDigit(static_cast<sal_uInt32>(aName[aName.getLength() - 1])))
+        {
+            aName = aName.copy(0, aName.getLength() - 1);
+        }
+
+        xmlChar* pFontFamilyGeneric = xmlGetProp(pXmlNode, 
BAD_CAST("font-family-generic"));
+        OString aFontFamilyGeneric;
+        if (pFontFamilyGeneric)
+        {
+            aFontFamilyGeneric = OString(reinterpret_cast<char 
const*>(pFontFamilyGeneric));
+        }
+
+        aXMLFonts.push_back(XmlFont{ aName, aFontFamilyGeneric });
+        aSortedFonts.push_back(XmlFont{ aName, aFontFamilyGeneric });
         xmlFree(pName);
     }
+    std::sort(aSortedFonts.begin(), aSortedFonts.end());
     size_t nIndex = 0;
-    for (const auto& rName : aSortedNames)
+    for (const auto& rFont : aSortedFonts)
     {
         // Without the accompanying fix in place, this test would have failed 
with:
         // - Expected: Liberation Sans
         // - Actual  : Lucida Sans1
         // i.e. the output was not lexicographically sorted, "u" was before 
"i".
-        CPPUNIT_ASSERT_EQUAL(rName, aXMLNames[nIndex]);
+        CPPUNIT_ASSERT_EQUAL(rFont.aName, aXMLFonts[nIndex].aName);
+        // Without the accompanying fix in place, this test would have failed 
with:
+        // - Expected: swiss
+        // - Actual  : system
+        // i.e. the output was not lexicographically sorted when style:name 
was the same, but
+        // style:font-family-generic was not the same.
+        CPPUNIT_ASSERT_EQUAL(rFont.aFontFamilyGeneric, 
aXMLFonts[nIndex].aFontFamilyGeneric);
         ++nIndex;
     }
     xmlXPathFreeObject(pXPath);

Reply via email to