filter/source/msfilter/eschesdo.cxx | 2
include/svx/svdsob.hxx | 12 +++--
include/svx/svdtypes.hxx | 4 -
reportdesign/source/core/api/ReportDefinition.cxx | 6 +-
sc/qa/extras/macros-test.cxx | 48 +++++++++++++++++++++
sc/qa/unit/scshapetest.cxx | 12 ++---
sc/source/core/data/drwlayer.cxx | 10 ++--
sc/source/filter/xml/XMLTableShapeImportHelper.cxx | 2
sc/source/filter/xml/xmlwrap.cxx | 10 ++--
sc/source/ui/Accessibility/AccessibleDocument.cxx | 8 +--
svx/inc/sxlayitm.hxx | 6 +-
svx/source/uitest/sdrobject.cxx | 2
svx/source/unodraw/unoshape.cxx | 2
sw/source/uibase/shells/drwbassh.cxx | 2
14 files changed, 90 insertions(+), 36 deletions(-)
New commits:
commit f4a62e5479b47d90d6de518f38a97ac0b5322c54
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Tue Sep 7 12:16:24 2021 +0200
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Wed Feb 23 16:00:28 2022 +0100
SdrLayerID must be based on sal_Int16
... which is the type corresponding to the related published property
"LayerID" of com.sun.star.drawing.Shape service.
Without this, the code asserts on values passed to the published API
from external sources to be in the 8-bit limits, which is incorrect.
Change-Id: I0449a7dd313f7e6c4adbc1c1f7b8c50b6a51434e
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/121760
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
diff --git a/filter/source/msfilter/eschesdo.cxx
b/filter/source/msfilter/eschesdo.cxx
index 12e6e62be0a2..d052b5968780 100644
--- a/filter/source/msfilter/eschesdo.cxx
+++ b/filter/source/msfilter/eschesdo.cxx
@@ -669,7 +669,7 @@ sal_uInt32 ImplEESdrWriter::ImplWriteShape(
ImplEESdrObject& rObj,
if( SDRLAYER_NOTFOUND != mpEscherEx->GetHellLayerId() &&
rObj.ImplGetPropertyValue( "LayerID" ) &&
- *o3tl::doAccess<sal_uInt16>(rObj.GetUsrAny()) ==
sal_uInt8(mpEscherEx->GetHellLayerId()) )
+ *o3tl::doAccess<sal_Int16>(rObj.GetUsrAny()) ==
mpEscherEx->GetHellLayerId().get() )
{
aPropOpt.AddOpt( ESCHER_Prop_fPrint, 0x200020 );
}
diff --git a/include/svx/svdsob.hxx b/include/svx/svdsob.hxx
index 2d17ab05640a..7e2b8578e01d 100644
--- a/include/svx/svdsob.hxx
+++ b/include/svx/svdsob.hxx
@@ -30,6 +30,7 @@
class SVXCORE_DLLPUBLIC SdrLayerIDSet final
{
+ // For now, have up to 256 layers
sal_uInt8 aData[32];
public:
@@ -45,12 +46,16 @@ public:
void Set(SdrLayerID a)
{
- aData[sal_uInt8(a)/8] |= 1 << (sal_uInt8(a) % 8);
+ const sal_Int16 nId = a.get();
+ if (nId >= 0 && nId < 256)
+ aData[nId / 8] |= 1 << (nId % 8);
}
void Clear(SdrLayerID a)
{
- aData[sal_uInt8(a)/8] &= ~(1 << (sal_uInt8(a) % 8));
+ const sal_Int16 nId = a.get();
+ if (nId >= 0 && nId < 256)
+ aData[nId / 8] &= ~(1 << (nId % 8));
}
void Set(SdrLayerID a, bool b)
@@ -63,7 +68,8 @@ public:
bool IsSet(SdrLayerID a) const
{
- return (aData[sal_uInt8(a)/8] & 1<<sal_uInt8(a)%8) != 0;
+ const sal_Int16 nId = a.get();
+ return nId >= 0 && nId < 256 && (aData[nId / 8] & 1 << nId % 8) != 0;
}
void SetAll()
diff --git a/include/svx/svdtypes.hxx b/include/svx/svdtypes.hxx
index 5b73552a5719..1009b0314923 100644
--- a/include/svx/svdtypes.hxx
+++ b/include/svx/svdtypes.hxx
@@ -53,11 +53,11 @@ enum class SdrDragMode
// You can use this value in the methods of SdrLayerSet, but false is returned
// every time or the method does nothing.
// type declaration for Layer-IDs
-typedef o3tl::strong_int<sal_uInt8, struct SdrLayerIDTag> SdrLayerID;
+typedef o3tl::strong_int<sal_Int16, struct SdrLayerIDTag> SdrLayerID;
// If there is no layer when it should be identified, then
// SdrLayerAdmin::GetLayerID(const String&) returns a value.
-constexpr SdrLayerID SDRLAYER_NOTFOUND(0xff);
+constexpr SdrLayerID SDRLAYER_NOTFOUND(-1);
/*
* Repeat
diff --git a/reportdesign/source/core/api/ReportDefinition.cxx
b/reportdesign/source/core/api/ReportDefinition.cxx
index 636346a163c3..9954b50c359e 100644
--- a/reportdesign/source/core/api/ReportDefinition.cxx
+++ b/reportdesign/source/core/api/ReportDefinition.cxx
@@ -588,9 +588,9 @@ void OReportDefinition::init()
m_pImpl->m_pReportModel->GetItemPool().FreezeIdRanges();
m_pImpl->m_pReportModel->SetScaleUnit( MapUnit::Map100thMM );
SdrLayerAdmin& rAdmin = m_pImpl->m_pReportModel->GetLayerAdmin();
- rAdmin.NewLayer("front", sal_uInt8(RPT_LAYER_FRONT));
- rAdmin.NewLayer("back", sal_uInt8(RPT_LAYER_BACK));
- rAdmin.NewLayer("HiddenLayer", sal_uInt8(RPT_LAYER_HIDDEN));
+ rAdmin.NewLayer("front", RPT_LAYER_FRONT.get());
+ rAdmin.NewLayer("back", RPT_LAYER_BACK.get());
+ rAdmin.NewLayer("HiddenLayer", RPT_LAYER_HIDDEN.get());
m_pImpl->m_pUndoManager = new ::dbaui::UndoManager( *this, m_aMutex );
m_pImpl->m_pReportModel->SetSdrUndoManager(
&m_pImpl->m_pUndoManager->GetSfxUndoManager() );
diff --git a/sc/qa/extras/macros-test.cxx b/sc/qa/extras/macros-test.cxx
index 6c41b269111e..cc77943ab879 100644
--- a/sc/qa/extras/macros-test.cxx
+++ b/sc/qa/extras/macros-test.cxx
@@ -64,6 +64,7 @@ public:
void testTdf130307();
void testTdf146742();
void testMacroButtonFormControlXlsxExport();
+ void testShapeLayerId();
CPPUNIT_TEST_SUITE(ScMacrosTest);
CPPUNIT_TEST(testStarBasic);
@@ -92,6 +93,7 @@ public:
CPPUNIT_TEST(testTdf130307);
CPPUNIT_TEST(testTdf146742);
CPPUNIT_TEST(testMacroButtonFormControlXlsxExport);
+ CPPUNIT_TEST(testShapeLayerId);
CPPUNIT_TEST_SUITE_END();
};
@@ -999,6 +1001,52 @@ void ScMacrosTest::testTdf105558()
xCloseable->close(true);
}
+void ScMacrosTest::testShapeLayerId()
+{
+ auto xComponent = loadFromDesktop("private:factory/scalc");
+
+ // insert initial library
+ css::uno::Reference<css::document::XEmbeddedScripts> xDocScr(xComponent,
UNO_QUERY_THROW);
+ auto xLibs = xDocScr->getBasicLibraries();
+ auto xLibrary = xLibs->createLibrary("TestLibrary");
+ xLibrary->insertByName(
+ "TestModule",
+ uno::Any(
+ OUString("Function TestLayerID\n"
+ " xShape =
thisComponent.createInstance(\"com.sun.star.drawing.TextShape\")\n"
+ " thisComponent.DrawPages(0).Add(xShape)\n"
+ " origID = xShape.LayerID\n"
+ " On Error Goto handler\n"
+ " xShape.LayerID = 257 ' 1 if wrongly converted to
unsigned 8-bit type\n"
+ " TestLayerID = origID & \" \" & xShape.LayerID ' Should
not happen\n"
+ " Exit Function\n"
+ "handler:\n"
+ " ' This is expected to happen\n"
+ " TestLayerID = origID & \" Expected runtime error
happened\"\n"
+ "End Function\n")));
+
+ Any aRet;
+ Sequence<sal_Int16> aOutParamIndex;
+ Sequence<Any> aOutParam;
+
+ SfxObjectShell* pFoundShell =
SfxObjectShell::GetShellFromComponent(xComponent);
+ ScDocShell* pDocSh = static_cast<ScDocShell*>(pFoundShell);
+ CPPUNIT_ASSERT(pDocSh);
+
+ SfxObjectShell::CallXScript(
+ xComponent,
+
"vnd.sun.Star.script:TestLibrary.TestModule.TestLayerID?language=Basic&location=document",
+ {}, aRet, aOutParamIndex, aOutParam);
+ // Without the fix in place, this test would have failed in non-debug
builds with
+ // - Expected : <Any: (string) 0 Expected runtime error happened>
+ // - Actual : <Any: (string) 0 1>
+ // In debug builds, it would crash on assertion inside strong_int ctor.
+ // The LayerID property of com.sun.star.drawing.Shape service has 'short'
IDL type.
+ // The expected run-time error is because there are only 5 layers there.
+ CPPUNIT_ASSERT_EQUAL(Any(OUString("0 Expected runtime error happened")),
aRet);
+ pDocSh->DoClose();
+}
+
ScMacrosTest::ScMacrosTest()
: UnoApiTest("/sc/qa/extras/testdocuments")
{
diff --git a/sc/qa/unit/scshapetest.cxx b/sc/qa/unit/scshapetest.cxx
index 96bbbc3c8912..761db2da687c 100644
--- a/sc/qa/unit/scshapetest.cxx
+++ b/sc/qa/unit/scshapetest.cxx
@@ -362,8 +362,8 @@ void ScShapeTest::testTdf140252_DragCreateFormControl()
SdrUnoObj* pObj = static_cast<SdrUnoObj*>(lcl_getSdrObjectWithAssert(rDoc,
0));
// Without the fix in place, the shape would be on layer SC_LAYER_FRONT (0)
- sal_uInt8 nExpectedID = sal_uInt8(SC_LAYER_CONTROLS);
- sal_uInt8 nActualID = pObj->GetLayer().get();
+ sal_Int16 nExpectedID = SC_LAYER_CONTROLS.get();
+ sal_Int16 nActualID = pObj->GetLayer().get();
CPPUNIT_ASSERT_EQUAL(nExpectedID, nActualID);
pDocSh->DoClose();
@@ -407,8 +407,8 @@ void ScShapeTest::testTdf134355_DragCreateCustomShape()
SdrObjCustomShape* pObj =
static_cast<SdrObjCustomShape*>(lcl_getSdrObjectWithAssert(rDoc, 0));
// Without the fix in place, the shape would be on layer SC_LAYER_CONTROLS
(3)
- sal_uInt8 nExpectedID = sal_uInt8(SC_LAYER_FRONT);
- sal_uInt8 nActualID = pObj->GetLayer().get();
+ sal_Int16 nExpectedID = SC_LAYER_FRONT.get();
+ sal_Int16 nActualID = pObj->GetLayer().get();
CPPUNIT_ASSERT_EQUAL(nExpectedID, nActualID);
pDocSh->DoClose();
@@ -441,8 +441,8 @@ void ScShapeTest::testTdf140252_LayerOfControl()
SdrObject* pObj = lcl_getSdrObjectWithAssert(rDoc, 0);
// Check LayerID of object. Without the fix in place it was 0.
- sal_uInt8 nExpectedID = sal_uInt8(SC_LAYER_CONTROLS);
- sal_uInt8 nActualID = pObj->GetLayer().get();
+ sal_Int16 nExpectedID = SC_LAYER_CONTROLS.get();
+ sal_Int16 nActualID = pObj->GetLayer().get();
CPPUNIT_ASSERT_EQUAL(nExpectedID, nActualID);
pDocSh->DoClose();
diff --git a/sc/source/core/data/drwlayer.cxx b/sc/source/core/data/drwlayer.cxx
index 47985ebeda51..0c1aa0216581 100644
--- a/sc/source/core/data/drwlayer.cxx
+++ b/sc/source/core/data/drwlayer.cxx
@@ -279,12 +279,12 @@ ScDrawLayer::ScDrawLayer( ScDocument* pDocument, const
OUString& rName ) :
rPool.FreezeIdRanges(); // the pool is also used
directly
SdrLayerAdmin& rAdmin = GetLayerAdmin();
- rAdmin.NewLayer("vorne", sal_uInt8(SC_LAYER_FRONT));
- rAdmin.NewLayer("hinten", sal_uInt8(SC_LAYER_BACK));
- rAdmin.NewLayer("intern", sal_uInt8(SC_LAYER_INTERN));
+ rAdmin.NewLayer("vorne", SC_LAYER_FRONT.get());
+ rAdmin.NewLayer("hinten", SC_LAYER_BACK.get());
+ rAdmin.NewLayer("intern", SC_LAYER_INTERN.get());
// tdf#140252 use same name as in ctor of SdrLayerAdmin
- rAdmin.NewLayer(rAdmin.GetControlLayerName(),
sal_uInt8(SC_LAYER_CONTROLS));
- rAdmin.NewLayer("hidden", sal_uInt8(SC_LAYER_HIDDEN));
+ rAdmin.NewLayer(rAdmin.GetControlLayerName(), SC_LAYER_CONTROLS.get());
+ rAdmin.NewLayer("hidden", SC_LAYER_HIDDEN.get());
// Set link for URL-Fields
ScModule* pScMod = SC_MOD();
diff --git a/sc/source/filter/xml/XMLTableShapeImportHelper.cxx
b/sc/source/filter/xml/XMLTableShapeImportHelper.cxx
index f4a27c152a5a..ba2382580eba 100644
--- a/sc/source/filter/xml/XMLTableShapeImportHelper.cxx
+++ b/sc/source/filter/xml/XMLTableShapeImportHelper.cxx
@@ -56,7 +56,7 @@ void XMLTableShapeImportHelper::SetLayer(const
uno::Reference<drawing::XShape>&
{
uno::Reference< beans::XPropertySet > xShapeProp( rShape,
uno::UNO_QUERY );
if( xShapeProp.is() )
- xShapeProp->setPropertyValue( SC_LAYERID,
uno::makeAny<sal_uInt16>(sal_uInt8(nLayerID)) );
+ xShapeProp->setPropertyValue( SC_LAYERID,
uno::makeAny(nLayerID.get()) );
}
}
diff --git a/sc/source/filter/xml/xmlwrap.cxx b/sc/source/filter/xml/xmlwrap.cxx
index 1d19f18be4b3..c0aac4c43e9f 100644
--- a/sc/source/filter/xml/xmlwrap.cxx
+++ b/sc/source/filter/xml/xmlwrap.cxx
@@ -786,15 +786,15 @@ bool ScXMLImportWrapper::Export(bool bStylesOnly)
xShape->getPropertyValue("LayerID") >>= nLayerID;
switch (nLayerID)
{
- case sal_uInt8(SC_LAYER_FRONT):
+ case sal_Int16(SC_LAYER_FRONT):
return 1;
- case sal_uInt8(SC_LAYER_BACK):
+ case sal_Int16(SC_LAYER_BACK):
return 0;
- case sal_uInt8(SC_LAYER_INTERN):
+ case sal_Int16(SC_LAYER_INTERN):
return 2;
- case sal_uInt8(SC_LAYER_CONTROLS):
+ case sal_Int16(SC_LAYER_CONTROLS):
return 3;
- case sal_uInt8(SC_LAYER_HIDDEN):
+ case sal_Int16(SC_LAYER_HIDDEN):
return 1; // treat as equivalent to front
default:
O3TL_UNREACHABLE;
diff --git a/sc/source/ui/Accessibility/AccessibleDocument.cxx
b/sc/source/ui/Accessibility/AccessibleDocument.cxx
index c619bc4c44c2..1d58af05fe2a 100644
--- a/sc/source/ui/Accessibility/AccessibleDocument.cxx
+++ b/sc/source/ui/Accessibility/AccessibleDocument.cxx
@@ -136,19 +136,19 @@ struct ScShapeDataLess
static void ConvertLayerId(sal_Int16& rLayerID) // changes the number of
the LayerId so it the accessibility order
{
// note: MSVC 2017 ICE's if this is written as "switch" so use "if"
- if (sal_uInt8(SC_LAYER_FRONT) == rLayerID)
+ if (SC_LAYER_FRONT.get() == rLayerID)
{
rLayerID = 1;
}
- else if (sal_uInt8(SC_LAYER_BACK) == rLayerID)
+ else if (SC_LAYER_BACK.get() == rLayerID)
{
rLayerID = 0;
}
- else if (sal_uInt8(SC_LAYER_INTERN) == rLayerID)
+ else if (SC_LAYER_INTERN.get() == rLayerID)
{
rLayerID = 2;
}
- else if (sal_uInt8(SC_LAYER_CONTROLS) == rLayerID)
+ else if (SC_LAYER_CONTROLS.get() == rLayerID)
{
rLayerID = 3;
}
diff --git a/svx/inc/sxlayitm.hxx b/svx/inc/sxlayitm.hxx
index a52c47f3d9af..be444bec33dd 100644
--- a/svx/inc/sxlayitm.hxx
+++ b/svx/inc/sxlayitm.hxx
@@ -24,10 +24,10 @@
#include <svl/intitem.hxx>
#include <svx/svdtypes.hxx>
-class SdrLayerIdItem final : public SfxUInt16Item {
+class SdrLayerIdItem final : public SfxInt16Item {
public:
- SdrLayerIdItem(SdrLayerID nId):
SfxUInt16Item(SDRATTR_LAYERID,sal_uInt8(nId)) {}
- SdrLayerID GetValue() const { return
SdrLayerID(SfxUInt16Item::GetValue()); }
+ SdrLayerIdItem(SdrLayerID nId): SfxInt16Item(SDRATTR_LAYERID,nId.get()) {}
+ SdrLayerID GetValue() const { return SdrLayerID(SfxInt16Item::GetValue());
}
virtual SdrLayerIdItem* Clone(SfxItemPool* pPool=nullptr) const override;
};
diff --git a/svx/source/uitest/sdrobject.cxx b/svx/source/uitest/sdrobject.cxx
index 0d287de2a8a7..bc72ee2a8c93 100644
--- a/svx/source/uitest/sdrobject.cxx
+++ b/svx/source/uitest/sdrobject.cxx
@@ -28,7 +28,7 @@ StringMap SdrUIObject::get_state()
aMap["Description"] = pObject->GetDescription();
aMap["Title"] = pObject->GetTitle();
aMap["Z-Order"] = OUString::number(pObject->GetOrdNum());
- aMap["Layer"] = OUString::number(sal_uInt8(pObject->GetLayer()));
+ aMap["Layer"] = OUString::number(pObject->GetLayer().get());
aMap["IsGroupObject"] = OUString::boolean(pObject->IsGroupObject());
aMap["IsPolyObject"] = OUString::boolean(pObject->IsPolyObj());
aMap["PointCount"] = OUString::number(pObject->GetPointCount());
diff --git a/svx/source/unodraw/unoshape.cxx b/svx/source/unodraw/unoshape.cxx
index 86c2cb74af5f..4591c49c5899 100644
--- a/svx/source/unodraw/unoshape.cxx
+++ b/svx/source/unodraw/unoshape.cxx
@@ -2778,7 +2778,7 @@ bool SvxShape::getPropertyValueImpl( const OUString&,
const SfxItemPropertyMapEn
break;
}
case SDRATTR_LAYERID:
- rValue <<= sal_uInt16(sal_uInt8(GetSdrObject()->GetLayer()));
+ rValue <<= GetSdrObject()->GetLayer().get();
break;
case SDRATTR_LAYERNAME:
diff --git a/sw/source/uibase/shells/drwbassh.cxx
b/sw/source/uibase/shells/drwbassh.cxx
index 9aaf727f7155..8cafe328f74a 100644
--- a/sw/source/uibase/shells/drwbassh.cxx
+++ b/sw/source/uibase/shells/drwbassh.cxx
@@ -136,7 +136,7 @@ void SwDrawBaseShell::Execute(SfxRequest const &rReq)
aSet.Put(SfxBoolItem(SID_HTML_MODE,
0 != ::GetHtmlMode(pSh->GetView().GetDocShell())));
- aSet.Put(SfxInt16Item(FN_DRAW_WRAP_DLG,
sal_uInt8(pSh->GetLayerId())));
+ aSet.Put(SfxInt16Item(FN_DRAW_WRAP_DLG,
pSh->GetLayerId().get()));
pSh->GetObjAttr(aSet);
SwAbstractDialogFactory* pFact =
SwAbstractDialogFactory::Create();
