include/docmodel/styles/ChartStyle.hxx | 118 +++++++++++++++++++--
include/oox/export/chartexport.hxx | 9 +
oox/inc/drawingml/chart/stylefragment.hxx | 24 +++-
oox/inc/drawingml/chart/stylemodel.hxx | 4
oox/source/drawingml/chart/stylefragment.cxx | 147 ++++++++++++---------------
oox/source/export/chartexport.cxx | 35 ++++--
6 files changed, 228 insertions(+), 109 deletions(-)
New commits:
commit 5effa8d0977ac9ce13a8e66cbd2302e585c41c6d
Author: Kurt Nordback <kurt.nordb...@collabora.com>
AuthorDate: Fri Sep 19 04:39:14 2025 -0600
Commit: Tomaž Vajngerl <qui...@gmail.com>
CommitDate: Mon Sep 22 06:04:55 2025 +0200
tdf#168472 - Chart round-trip MSO -> LO -> MSO broken
Chart style import/export was not handling a:ST_FontCollectionIndex
correctly. Despite the name, its value is not an integer index,
but one of three strings (major, minor, none). Separate the
FontReference and StyleReference code better in order to handle
this OOXML value correctly.
Change-Id: I1638f6484ecfe5a9bd8c1adab8d854cff47ce14f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/191195
Reviewed-by: Tomaž Vajngerl <qui...@gmail.com>
Tested-by: Jenkins
diff --git a/include/docmodel/styles/ChartStyle.hxx
b/include/docmodel/styles/ChartStyle.hxx
index b05060a6f5d2..a5a7155421ba 100644
--- a/include/docmodel/styles/ChartStyle.hxx
+++ b/include/docmodel/styles/ChartStyle.hxx
@@ -21,8 +21,6 @@ namespace model
{
struct FontOrStyleRef
{
- sal_Int32 mnIdx; // required
-
enum class StyleColorEnum
{
AUTO
@@ -55,6 +53,56 @@ struct FontOrStyleRef
return sV;
}
+ // Convert an input string to ColorVal
+ void setColorValStr(const std::optional<OUString>& str)
+ {
+ // The attribute here can be an integer, a string, or "auto" (which
+ // of course is also a string, but is considered special). So we
+ // need to try to convert the input string to an integer, and handle
+ // it as an int if we can. OUString provides toInt(), but it returns
+ // 0 in the case of failure, which is eminently unhelpful, sinze 0
+ // is a perfectly acceptable value. So convert it to a
+ // std::basic_string, which has stoi(), which throws if it can't do
+ // the conversion.
+ //
+ // Unfortunately OUString characters are sal_Unicode which can be
+ // uint16_t, and while there's string::stoi() and wstring::stoi(),
+ // there's no basic_string<uint16_t>::stoi(). So we use wstring and
+ // construct character by character.
+ if (str)
+ {
+ StyleColorVal v;
+
+ const sal_Unicode* pRawStr = str->getStr();
+ std::wstring sBStr;
+ sBStr.reserve(str->getLength());
+ for (const sal_Unicode* pS = pRawStr; pS < pRawStr +
str->getLength(); ++pS)
+ {
+ sBStr.push_back(*pS);
+ }
+
+ sal_uInt32 nIntVal = 0;
+ try
+ {
+ nIntVal = stoi(sBStr);
+ v = nIntVal;
+ }
+ catch (std::invalid_argument&)
+ {
+ // Not an integer, so see if it's the fixed enum
+ if (*str == "auto")
+ {
+ v = StyleColorEnum::AUTO;
+ }
+ else
+ {
+ v = *str;
+ }
+ }
+ maStyleClr = std::make_unique<StyleColorVal>(v);
+ }
+ }
+
// A child element of cs:CT_FontReference or cs:CT_StyleReference is
// a:EG_ColorChoice. The latter is handled by ColorValueContext.
oox::drawingml::Color maColor; // needed for ColorValueContext
@@ -63,23 +111,73 @@ struct FontOrStyleRef
// FontOrStyleRef. Ignore those for now. TODO
};
+struct StyleRef : public FontOrStyleRef
+{
+ sal_Int32 mnIdx = 0; // StyleMatrixColumnIndex
+};
+
+struct FontRef : public FontOrStyleRef
+{
+ enum FontCollectionIndex
+ {
+ MAJOR,
+ MINOR,
+ NONE
+ };
+
+ FontCollectionIndex meIdx = MINOR; // required
+
+ // Get the string value of the font collection index
+ const char* getFontCollectionStr() const
+ {
+ switch (meIdx)
+ {
+ case MINOR:
+ return "minor";
+ case MAJOR:
+ return "major";
+ case NONE:
+ return "none";
+ default:
+ assert(false);
+ }
+ return nullptr;
+ }
+
+ // Set the font collection index from a string
+ void setFontCollectionIndex(std::u16string_view sIdx)
+ {
+ if (sIdx == std::u16string_view(u"major"))
+ {
+ meIdx = MAJOR;
+ }
+ else if (sIdx == std::u16string_view(u"minor"))
+ {
+ meIdx = MINOR;
+ }
+ else if (sIdx == std::u16string_view(u"none"))
+ {
+ meIdx = NONE;
+ }
+ }
+};
+
struct StyleEntry
{
- std::shared_ptr<FontOrStyleRef> mxLnClr;
+ std::shared_ptr<StyleRef> mxLnClr;
double mfLineWidthScale = 1.0;
- std::shared_ptr<FontOrStyleRef> mxFillClr;
- std::shared_ptr<FontOrStyleRef> mxEffectClr;
- std::shared_ptr<FontOrStyleRef> mxFontClr;
+ std::shared_ptr<StyleRef> mxFillClr;
+ std::shared_ptr<StyleRef> mxEffectClr;
+ std::shared_ptr<FontRef> mxFontClr;
std::shared_ptr<oox::drawingml::Shape> mxShapePr;
// The following is derived from a TextCharacterProperties
std::shared_ptr<oox::PropertyMap> mrTextCharacterPr;
// The following is derived from a TextBodyProperties
std::shared_ptr<oox::PropertyMap> mxTextBodyPr;
- StyleEntry(std::shared_ptr<FontOrStyleRef> aLnClr, double fLineScale,
- std::shared_ptr<FontOrStyleRef> aFillClr,
std::shared_ptr<FontOrStyleRef> aEffectClr,
- std::shared_ptr<FontOrStyleRef> aFontClr,
- std::shared_ptr<oox::drawingml::Shape> aShape,
+ StyleEntry(std::shared_ptr<StyleRef> aLnClr, double fLineScale,
+ std::shared_ptr<StyleRef> aFillClr, std::shared_ptr<StyleRef>
aEffectClr,
+ std::shared_ptr<FontRef> aFontClr,
std::shared_ptr<oox::drawingml::Shape> aShape,
std::shared_ptr<oox::PropertyMap> aCharProps,
std::shared_ptr<oox::PropertyMap> aBodyProps)
: mxLnClr(std::move(aLnClr))
diff --git a/include/oox/export/chartexport.hxx
b/include/oox/export/chartexport.hxx
index 66c1dd5e3dda..e52abebb9b46 100644
--- a/include/oox/export/chartexport.hxx
+++ b/include/oox/export/chartexport.hxx
@@ -70,7 +70,8 @@ namespace core {
}}
namespace model {
- struct FontOrStyleRef;
+ struct StyleRef;
+ struct FontRef;
struct StyleEntry;
}
@@ -291,8 +292,10 @@ private:
// Functions for style file output
void outputStyleEntry(::sax_fastparser::FSHelperPtr pFS,
sal_Int32 nElTokenId, model::StyleEntry& aEntry);
- void outputFontOrStyleRef(::sax_fastparser::FSHelperPtr pFS,
- sal_Int32 nElTokenId, const model::FontOrStyleRef& aColor);
+ void outputStyleRef(::sax_fastparser::FSHelperPtr pFS,
+ sal_Int32 nElTokenId, const model::StyleRef& aColor);
+ void outputFontRef(::sax_fastparser::FSHelperPtr pFS,
+ sal_Int32 nElTokenId, const model::FontRef& aColor);
public:
diff --git a/oox/inc/drawingml/chart/stylefragment.hxx
b/oox/inc/drawingml/chart/stylefragment.hxx
index a7bb698a43e2..9adbea526aba 100644
--- a/oox/inc/drawingml/chart/stylefragment.hxx
+++ b/oox/inc/drawingml/chart/stylefragment.hxx
@@ -23,24 +23,38 @@
namespace model
{
-struct FontOrStyleRef;
+struct StyleRef;
+struct FontRef;
}
namespace oox::drawingml::chart
{
-/** Handler for a cs:CT_StyleReference or cs:CT_FontReference element.
+/** Handler for a cs:CT_StyleReference element.
*/
-class StyleReferenceContext final : public ContextBase<model::FontOrStyleRef>
+class StyleReferenceContext final : public ContextBase<model::StyleRef>
{
public:
- StyleReferenceContext(ContextHandler2Helper& rParent, sal_Int32 nIdx,
- model::FontOrStyleRef& rModel);
+ StyleReferenceContext(ContextHandler2Helper& rParent, const sal_Int32 nIdx,
+ model::StyleRef& rModel);
virtual ~StyleReferenceContext() override;
virtual ::oox::core::ContextHandlerRef onCreateContext(sal_Int32 nElement,
const
AttributeList& rAttribs) override;
};
+/** Handler for a cs:CT_FontReference element.
+ */
+class FontReferenceContext final : public ContextBase<model::FontRef>
+{
+public:
+ FontReferenceContext(ContextHandler2Helper& rParent, std::u16string_view
sIdx,
+ model::FontRef& rModel);
+ virtual ~FontReferenceContext() override;
+
+ virtual ::oox::core::ContextHandlerRef onCreateContext(sal_Int32 nElement,
+ const
AttributeList& rAttribs) override;
+};
+
struct StyleEntryModel;
/** Handler for a cs:CT_StyleEntry element.
diff --git a/oox/inc/drawingml/chart/stylemodel.hxx
b/oox/inc/drawingml/chart/stylemodel.hxx
index ce8c2c8e64d6..55427a2b1c0c 100644
--- a/oox/inc/drawingml/chart/stylemodel.hxx
+++ b/oox/inc/drawingml/chart/stylemodel.hxx
@@ -28,8 +28,8 @@ namespace oox::drawingml::chart
// model for both.
struct StyleEntryModel
{
- typedef ModelRef<model::FontOrStyleRef> StyleRef; // "idx" is
ST_StyleMatrixColumnIndex
- typedef ModelRef<model::FontOrStyleRef> FontRef; // "idx" is
ST_FontCollectionIndex
+ typedef ModelRef<model::StyleRef> StyleRef; // "idx" is
ST_StyleMatrixColumnIndex
+ typedef ModelRef<model::FontRef> FontRef; // "idx" is
ST_FontCollectionIndex
typedef ModelRef<TextCharacterProperties> TextCharacterPropsRef;
typedef ModelRef<TextBodyProperties> TextBodyPropsRef;
typedef ModelRef<Shape> ShapeRef;
diff --git a/oox/source/drawingml/chart/stylefragment.cxx
b/oox/source/drawingml/chart/stylefragment.cxx
index a353ce445d91..b93d983ca6f9 100644
--- a/oox/source/drawingml/chart/stylefragment.cxx
+++ b/oox/source/drawingml/chart/stylefragment.cxx
@@ -39,85 +39,6 @@ namespace oox::drawingml::chart
using namespace ::oox::core;
using namespace model;
-//=======
-// StyleReferenceContext
-//=======
-StyleReferenceContext::StyleReferenceContext(ContextHandler2Helper& rParent,
sal_Int32 nIdx,
- model::FontOrStyleRef& rModel)
- : ContextBase<FontOrStyleRef>(rParent, rModel)
-{
- mrModel.mnIdx = nIdx;
-}
-
-StyleReferenceContext::~StyleReferenceContext() {}
-
-ContextHandlerRef StyleReferenceContext::onCreateContext(sal_Int32 nElement,
- const AttributeList&
rAttribs)
-{
- if (isRootElement())
- switch (nElement)
- {
- case CS_TOKEN(styleClr):
- {
- // The attribute here can be an integer, a string, or "auto"
(which
- // of course is also a string, but is considered special). So
we
- // need to try to convert the input string to an integer, and
handle
- // it as an int if we can. OUString provides toInt(), but it
returns
- // 0 in the case of failure, which is eminently unhelpful,
sinze 0
- // is a perfectly acceptable value. So convert it to a
- // std::basic_string, which has stoi(), which throws if it
can't do
- // the conversion.
- //
- // Unfortunately OUString characters are sal_Unicode which can
be
- // uint16_t, and while there's string::stoi() and
wstring::stoi(),
- // there's no basic_string<uint16_t>::stoi(). So we use
wstring and
- // construct character by character.
- std::optional<OUString> str = rAttribs.getString(XML_val);
- if (str)
- {
- FontOrStyleRef::StyleColorVal v;
-
- const sal_Unicode* pRawStr = str->getStr();
- std::wstring sBStr;
- sBStr.reserve(str->getLength());
- for (const sal_Unicode* pS = pRawStr; pS < pRawStr +
str->getLength(); ++pS)
- {
- sBStr.push_back(*pS);
- }
-
- sal_uInt32 nIntVal = 0;
- try
- {
- nIntVal = stoi(sBStr);
- v = nIntVal;
- }
- catch (std::invalid_argument&)
- {
- // Not an integer, so see if it's the fixed enum
- if (*str == "auto")
- {
- v = FontOrStyleRef::StyleColorEnum::AUTO;
- }
- else
- {
- v = *str;
- }
- }
- mrModel.maStyleClr =
std::make_unique<FontOrStyleRef::StyleColorVal>(v);
- }
- return nullptr;
- }
- case A_TOKEN(scrgbClr):
- case A_TOKEN(srgbClr):
- case A_TOKEN(hslClr):
- case A_TOKEN(sysClr):
- case A_TOKEN(schemeClr):
- case A_TOKEN(prstClr):
- return new ColorValueContext(*this, mrModel.maColor,
&mrModel.maComplexColor);
- }
- return nullptr;
-}
-
//=======
// StyleEntryContext
//=======
@@ -146,8 +67,8 @@ ContextHandlerRef
StyleEntryContext::onCreateContext(sal_Int32 nElement,
return new StyleReferenceContext(*this,
rAttribs.getInteger(XML_idx, -1),
mrModel.mxEffectRef.create());
case CS_TOKEN(fontRef): // CT_FontReference
- return new StyleReferenceContext(*this,
rAttribs.getInteger(XML_idx, -1),
- mrModel.mxFontRef.create());
+ return new FontReferenceContext(*this,
rAttribs.getString(XML_idx, ""),
+ mrModel.mxFontRef.create());
case CS_TOKEN(spPr): // a:CT_ShapeProperties
return new ShapePropertiesContext(*this,
mrModel.mxShapeProp.create());
case CS_TOKEN(defRPr): // a:CT_TextCharacterProperties
@@ -173,6 +94,70 @@ void StyleEntryContext::onCharacters(const OUString& rChars)
}
}
+//=======
+// StyleReferenceContext
+//=======
+StyleReferenceContext::StyleReferenceContext(ContextHandler2Helper& rParent,
const sal_Int32 nIdx,
+ model::StyleRef& rModel)
+ : ContextBase<StyleRef>(rParent, rModel)
+{
+ mrModel.mnIdx = nIdx;
+}
+
+StyleReferenceContext::~StyleReferenceContext() {}
+
+ContextHandlerRef StyleReferenceContext::onCreateContext(sal_Int32 nElement,
+ const AttributeList&
rAttribs)
+{
+ if (isRootElement())
+ switch (nElement)
+ {
+ case CS_TOKEN(styleClr):
+ mrModel.setColorValStr(rAttribs.getString(XML_val));
+ return nullptr;
+ case A_TOKEN(scrgbClr):
+ case A_TOKEN(srgbClr):
+ case A_TOKEN(hslClr):
+ case A_TOKEN(sysClr):
+ case A_TOKEN(schemeClr):
+ case A_TOKEN(prstClr):
+ return new ColorValueContext(*this, mrModel.maColor,
&mrModel.maComplexColor);
+ }
+ return nullptr;
+}
+
+//=======
+// FontReferenceContext
+//=======
+FontReferenceContext::FontReferenceContext(ContextHandler2Helper& rParent,
std::u16string_view sIdx,
+ model::FontRef& rModel)
+ : ContextBase<FontRef>(rParent, rModel)
+{
+ mrModel.setFontCollectionIndex(sIdx);
+}
+
+FontReferenceContext::~FontReferenceContext() {}
+
+ContextHandlerRef FontReferenceContext::onCreateContext(sal_Int32 nElement,
+ const AttributeList&
rAttribs)
+{
+ if (isRootElement())
+ switch (nElement)
+ {
+ case CS_TOKEN(styleClr):
+ mrModel.setColorValStr(rAttribs.getString(XML_val));
+ return nullptr;
+ case A_TOKEN(scrgbClr):
+ case A_TOKEN(srgbClr):
+ case A_TOKEN(hslClr):
+ case A_TOKEN(sysClr):
+ case A_TOKEN(schemeClr):
+ case A_TOKEN(prstClr):
+ return new ColorValueContext(*this, mrModel.maColor,
&mrModel.maComplexColor);
+ }
+ return nullptr;
+}
+
//=======
// StyleFragment
//=======
diff --git a/oox/source/export/chartexport.cxx
b/oox/source/export/chartexport.cxx
index 390f83412626..58b8e44d1442 100644
--- a/oox/source/export/chartexport.cxx
+++ b/oox/source/export/chartexport.cxx
@@ -5971,11 +5971,30 @@ OUString ChartExport::getNumberFormatCode(sal_Int32
nKey) const
return aCode;
}
-// Create cs:CT_FontReference or cs:CT_StyleReference
-void ChartExport::outputFontOrStyleRef(FSHelperPtr pFS, sal_Int32 nElTokenId,
const
- model::FontOrStyleRef& aColor)
+// Create cs:CT_StyleReference
+void ChartExport::outputStyleRef(FSHelperPtr pFS, sal_Int32 nElTokenId,
+ const model::StyleRef& aColor)
{
- pFS->startElement(FSNS(XML_cs, nElTokenId), XML_idx,
OUString::number(aColor.mnIdx));
+ pFS->startElement(FSNS(XML_cs, nElTokenId), XML_idx,
+ OUString::number(aColor.mnIdx));
+
+ ThemeExport aTE(mpFB, GetDocumentType(), pFS);
+ aTE.writeComplexColor(aColor.maComplexColor);
+
+ // Get the string for the StyleColorVal
+ OUString sV = aColor.getColorValStr();
+ if (!sV.isEmpty()) {
+ pFS->singleElement(FSNS(XML_cs, XML_styleClr), XML_val, sV);
+ }
+
+ pFS->endElement(FSNS(XML_cs, nElTokenId));
+}
+
+// Create cs:CT_FontReference
+void ChartExport::outputFontRef(FSHelperPtr pFS, sal_Int32 nElTokenId,
+ const model::FontRef& aColor)
+{
+ pFS->startElement(FSNS(XML_cs, nElTokenId), XML_idx,
aColor.getFontCollectionStr());
ThemeExport aTE(mpFB, GetDocumentType(), pFS);
aTE.writeComplexColor(aColor.maComplexColor);
@@ -5995,15 +6014,15 @@ void ChartExport::outputStyleEntry(FSHelperPtr pFS,
sal_Int32 nElTokenId, model:
// Just default values for now
pFS->startElement(FSNS(XML_cs, nElTokenId));
- outputFontOrStyleRef(pFS, FSNS(XML_cs, XML_lnRef), *(aEntry.mxLnClr));
+ outputStyleRef(pFS, FSNS(XML_cs, XML_lnRef), *(aEntry.mxLnClr));
pFS->startElement(FSNS(XML_cs, XML_lineWidthScale));
pFS->write(aEntry.mfLineWidthScale);
pFS->endElement(FSNS(XML_cs, XML_lineWidthScale));
- outputFontOrStyleRef(pFS, FSNS(XML_cs, XML_fillRef), *(aEntry.mxFillClr));
- outputFontOrStyleRef(pFS, FSNS(XML_cs, XML_effectRef),
*(aEntry.mxEffectClr));
- outputFontOrStyleRef(pFS, FSNS(XML_cs, XML_fontRef), *(aEntry.mxFontClr));
+ outputStyleRef(pFS, FSNS(XML_cs, XML_fillRef), *(aEntry.mxFillClr));
+ outputStyleRef(pFS, FSNS(XML_cs, XML_effectRef), *(aEntry.mxEffectClr));
+ outputFontRef(pFS, FSNS(XML_cs, XML_fontRef), *(aEntry.mxFontClr));
if (aEntry.mxShapePr) {
exportShapeProps(aEntry.mxShapePr->getShapeProperties().makePropertySet(),
XML_cs);
