chart2/qa/extras/data/ods/tdf153706_XY_scatter_chart.ods |binary
chart2/qa/extras/uichart.cxx | 54 ++++++++++
chart2/source/controller/chartapiwrapper/ChartDataWrapper.cxx | 10 +
chart2/source/controller/chartapiwrapper/ChartDataWrapper.hxx | 11 +-
4 files changed, 74 insertions(+), 1 deletion(-)
New commits:
commit 0c3580828811496052f41b09ad68fcc00a525f6f
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Sun Mar 3 16:12:55 2024 +0600
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Sun Mar 3 17:22:08 2024 +0100
tdf#153706: do not add categories, when source data doesn't have them
lcl_AllOperator is used in XChartDocument::attachData implementation.
When a data without existing categories is passed there, like an XY
scatter, lcl_AllOperator used to force creation of the categories in
the target, by returning 'true' unconditionally from setsCategories.
This meant, that a new sequence of numbers starting from 1 was used
as X values, and the old X data was interpreted as an extra Y series.
This changes lcl_AllOperator::setsCategories to try to check if its
data actually contains categories. Thus, XChartDocument::attachData
will use categories either when the chart already uses categories,
and ChartDataWrapper::applyData detects that using a call to
DataSourceHelper::detectRangeSegmentation; or when the new data has
it; but not when neither had it. When it's not possible to detect if
there were categories in the new data (e.g., with user data), old
behavior is used, setting categories.
It could be an alternative to detect the chart type using
xOldDoc->getDiagram()->getDiagramType() == "com.sun.star.chart.XYDiagram"
in XChartDocument::attachData; and then decide to force the creation
or not. But it seems hackish, and not really universal: other chart
types must be tested (bubble?), no idea how to handle hypothetical
cases when applied data contains categories in case of XY chart, etc.
Change-Id: I86b34f6799c30b103f7fc6b2faf6ec255a9d137b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164298
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
diff --git a/chart2/qa/extras/data/ods/tdf153706_XY_scatter_chart.ods
b/chart2/qa/extras/data/ods/tdf153706_XY_scatter_chart.ods
new file mode 100644
index 000000000000..2c6153ed6ab2
Binary files /dev/null and
b/chart2/qa/extras/data/ods/tdf153706_XY_scatter_chart.ods differ
diff --git a/chart2/qa/extras/uichart.cxx b/chart2/qa/extras/uichart.cxx
index 84b8658d29ea..2638367c8506 100644
--- a/chart2/qa/extras/uichart.cxx
+++ b/chart2/qa/extras/uichart.cxx
@@ -436,6 +436,60 @@ CPPUNIT_TEST_FIXTURE(Chart2UiChartTest, testTdf158223)
}
}
+CPPUNIT_TEST_FIXTURE(Chart2UiChartTest, testTdf153706)
+{
+ // Load a spreadsheet with a to-page XY scatter chart with the sheet as
data source
+ loadFromFile(u"ods/tdf153706_XY_scatter_chart.ods");
+
+ // Select the cell range around the chart, and copy the range to
clipboard, including the chart
+ dispatchCommand(mxComponent, u".uno:GoToCell"_ustr,
+ { comphelper::makePropertyValue(u"ToPoint"_ustr,
u"D1:K23"_ustr) });
+ dispatchCommand(mxComponent, u".uno:Copy"_ustr, {});
+
+ // create a new document
+ load(u"private:factory/scalc"_ustr);
+
+ // Paste; this must create a chart with own data source having a proper
copy of the data
+ dispatchCommand(mxComponent, u".uno:Paste"_ustr, {});
+
+ css::uno::Reference xChartDoc(getChartDocFromSheet(0, mxComponent),
css::uno::UNO_SET_THROW);
+ auto
xDataArray(xChartDoc->getDataProvider().queryThrow<chart::XChartDataArray>());
+
+ css::uno::Sequence<Sequence<double>> aData = xDataArray->getData();
+ CPPUNIT_ASSERT_EQUAL(sal_Int32(3), aData.getLength());
+ CPPUNIT_ASSERT_EQUAL(sal_Int32(2), aData[0].getLength());
+ CPPUNIT_ASSERT_EQUAL(sal_Int32(2), aData[1].getLength());
+ CPPUNIT_ASSERT_EQUAL(sal_Int32(2), aData[2].getLength());
+ CPPUNIT_ASSERT_EQUAL(2.0, aData[0][0]);
+ CPPUNIT_ASSERT_EQUAL(3.0, aData[0][1]);
+ CPPUNIT_ASSERT_EQUAL(3.0, aData[1][0]);
+ CPPUNIT_ASSERT_EQUAL(2.0, aData[1][1]);
+ CPPUNIT_ASSERT_EQUAL(4.0, aData[2][0]);
+ CPPUNIT_ASSERT_EQUAL(1.0, aData[2][1]);
+
+ // Without the fix, this would fail with
+ // - Expected: 1
+ // - Actual : 2
+ // i.e., the X values were treated as another Y series
+ CPPUNIT_ASSERT_EQUAL(sal_Int32(1), getNumberOfDataSeries(xChartDoc));
+
+ auto xSeries(getDataSeriesFromDoc(xChartDoc,
0).queryThrow<chart2::data::XDataSource>());
+ auto sequences = xSeries->getDataSequences();
+ // Without the fix, this would fail with
+ // - Expected: 2
+ // - Actual : 1
+ CPPUNIT_ASSERT_EQUAL(sal_Int32(2), sequences.getLength());
+
+ auto propX(sequences[0]->getValues().queryThrow<beans::XPropertySet>());
+ // Without the fix, this would fail with
+ // - Expected: values-x
+ // - Actual : values-y
+ CPPUNIT_ASSERT_EQUAL(u"values-x"_ustr,
propX->getPropertyValue("Role").get<OUString>());
+
+ auto propY(sequences[1]->getValues().queryThrow<beans::XPropertySet>());
+ CPPUNIT_ASSERT_EQUAL(u"values-y"_ustr,
propY->getPropertyValue("Role").get<OUString>());
+}
+
CPPUNIT_PLUGIN_IMPLEMENT();
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/chart2/source/controller/chartapiwrapper/ChartDataWrapper.cxx
b/chart2/source/controller/chartapiwrapper/ChartDataWrapper.cxx
index 84b98f0d8018..7988ef90df94 100644
--- a/chart2/source/controller/chartapiwrapper/ChartDataWrapper.cxx
+++ b/chart2/source/controller/chartapiwrapper/ChartDataWrapper.cxx
@@ -121,6 +121,11 @@ struct lcl_AllOperator : public lcl_Operator
virtual bool setsCategories( bool /*bDataInColumns*/ ) override
{
+ // Do not force creation of categories, when original has no categories
+ if (auto pDataWrapper = dynamic_cast<const
ChartDataWrapper*>(m_xDataToApply.get()))
+ if (auto xChartModel = pDataWrapper->getChartModel())
+ if (auto xDiagram = xChartModel->getFirstChartDiagram())
+ return xDiagram->getCategories().is();
return true;
}
@@ -698,6 +703,11 @@ css::uno::Sequence< OUString > SAL_CALL
ChartDataWrapper::getSupportedServiceNam
};
}
+rtl::Reference<ChartModel> ChartDataWrapper::getChartModel() const
+{
+ return m_spChart2ModelContact->getDocumentModel();
+}
+
} // namespace chart
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/chart2/source/controller/chartapiwrapper/ChartDataWrapper.hxx
b/chart2/source/controller/chartapiwrapper/ChartDataWrapper.hxx
index 3c6602d4e028..9a44e53b8b12 100644
--- a/chart2/source/controller/chartapiwrapper/ChartDataWrapper.hxx
+++ b/chart2/source/controller/chartapiwrapper/ChartDataWrapper.hxx
@@ -20,6 +20,8 @@
#include <cppuhelper/implbase.hxx>
#include <comphelper/interfacecontainer4.hxx>
+#include <rtl/ref.hxx>
+
#include <com/sun/star/chart2/XAnyDescriptionAccess.hpp>
#include <com/sun/star/chart/XDateCategories.hpp>
#include <com/sun/star/lang/XComponent.hpp>
@@ -28,7 +30,11 @@
#include <memory>
-namespace chart::wrapper
+namespace chart
+{
+class ChartModel;
+
+namespace wrapper
{
class Chart2ModelContact;
@@ -53,6 +59,8 @@ public:
virtual sal_Bool SAL_CALL supportsService( const OUString& ServiceName )
override;
virtual css::uno::Sequence< OUString > SAL_CALL getSupportedServiceNames()
override;
+ rtl::Reference<ChartModel> getChartModel() const;
+
private:
// ____ XDateCategories ____
virtual css::uno::Sequence< double > SAL_CALL getDateCategories() override;
@@ -113,5 +121,6 @@ private:
};
} // namespace chart::wrapper
+} // namespace chart
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
