svx/qa/unit/customshapes.cxx                |   36 ++++++++++++++++++++++++++++
 svx/qa/unit/data/tdf147409_GeomItemHash.odg |binary
 svx/source/toolbars/extrusionbar.cxx        |   13 ++++++----
 svx/source/toolbars/fontworkbar.cxx         |    5 +++
 4 files changed, 48 insertions(+), 6 deletions(-)

New commits:
commit 4c0b033ece143b6f398157e812ff1c6b22e2855d
Author:     Regina Henschel <rb.hensc...@t-online.de>
AuthorDate: Thu Feb 17 22:13:54 2022 +0100
Commit:     Regina Henschel <rb.hensc...@t-online.de>
CommitDate: Thu Feb 17 23:56:37 2022 +0100

    tdf#147409 tdf#146866 use SetPropertyValue for toggle
    
    The toggles (Textpath,SameLetterHeights) and (Extrusion,Extrusion)
    change a value by directly writing the value into the property. That
    is lines (*pAny) <<= bOn;, for example. But that does not trigger
    InvalidateHash() of the SdrCustomShapeGeometryItem. So the item has
    still aHashState 'Valid'. On the other hand because of the change the
    hash itself has changed. Therefore the == comparison between the
    original item and its clone returns 'false' in customshapeitem.cxx#238.
    And as a result, the assert in itempool.cxx#679 fails.
    
    My solution replaces the direct writing with setting the value via
    SetPropertyValue(), which includes the needed InvalidateHash(). The
    method InvalidateHash() is private and so cannot be directly used in
    fontwork.cxx and extrusionbar.cxx.
    
    Change-Id: Ib6021defb61478de9cbefa8f26466a2fe21352a2
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/130117
    Tested-by: Jenkins
    Reviewed-by: Regina Henschel <rb.hensc...@t-online.de>

diff --git a/svx/qa/unit/customshapes.cxx b/svx/qa/unit/customshapes.cxx
index 081a6b7789e0..bcce0ad7b458 100644
--- a/svx/qa/unit/customshapes.cxx
+++ b/svx/qa/unit/customshapes.cxx
@@ -132,6 +132,42 @@ void lcl_AssertRectEqualWithTolerance(std::string_view 
sInfo, const tools::Recta
                            std::abs(rExpected.GetHeight() - 
rActual.GetHeight()) <= nTolerance);
 }
 
+CPPUNIT_TEST_FIXTURE(CustomshapesTest, testTdf147409_GeomItemHash)
+{
+    OUString aURL = m_directories.getURLFromSrc(sDataDirectory) + 
"tdf147409_GeomItemHash.odg";
+    mxComponent = loadFromDesktop(aURL, 
"com.sun.star.comp.drawing.DrawingDocument");
+    uno::Reference<drawing::XShape> xShape(getShape(0));
+    SdrObjCustomShape* pSdrCustomShape(
+        
static_cast<SdrObjCustomShape*>(SdrObject::getSdrObjectFromXShape(xShape)));
+
+    // Mark Object
+    SfxViewShell* pViewShell = SfxViewShell::Current();
+    SdrView* pSdrView = pViewShell->GetDrawView();
+    pSdrView->MarkObj(pSdrCustomShape, pSdrView->GetSdrPageView());
+
+    // Apply FontworkSameLetterHeights toggle
+    // Without patch a debug build fails assert in SfxItemPool::PutImpl and so 
crashes.
+    dispatchCommand(mxComponent, ".uno:FontworkSameLetterHeights", {});
+}
+
+CPPUNIT_TEST_FIXTURE(CustomshapesTest, testTdf146866_GeomItemHash)
+{
+    OUString aURL = m_directories.getURLFromSrc(sDataDirectory) + 
"tdf147409_GeomItemHash.odg";
+    mxComponent = loadFromDesktop(aURL, 
"com.sun.star.comp.drawing.DrawingDocument");
+    uno::Reference<drawing::XShape> xShape(getShape(0));
+    SdrObjCustomShape* pSdrCustomShape(
+        
static_cast<SdrObjCustomShape*>(SdrObject::getSdrObjectFromXShape(xShape)));
+
+    // Mark Object
+    SfxViewShell* pViewShell = SfxViewShell::Current();
+    SdrView* pSdrView = pViewShell->GetDrawView();
+    pSdrView->MarkObj(pSdrCustomShape, pSdrView->GetSdrPageView());
+
+    // Apply extrusion toggle
+    // Without patch a debug build fails assert in SfxItemPool::PutImpl and so 
crashes.
+    dispatchCommand(mxComponent, ".uno:ExtrusionToggle", {});
+}
+
 CPPUNIT_TEST_FIXTURE(CustomshapesTest, testTdf145700_3D_NonUI)
 {
     // The document contains first light soft, no ambient color, no second 
light and shininess 6.
diff --git a/svx/qa/unit/data/tdf147409_GeomItemHash.odg 
b/svx/qa/unit/data/tdf147409_GeomItemHash.odg
new file mode 100644
index 000000000000..770b7a6c0b42
Binary files /dev/null and b/svx/qa/unit/data/tdf147409_GeomItemHash.odg differ
diff --git a/svx/source/toolbars/extrusionbar.cxx 
b/svx/source/toolbars/extrusionbar.cxx
index 26ac4805cde0..10b56ad4b369 100644
--- a/svx/source/toolbars/extrusionbar.cxx
+++ b/svx/source/toolbars/extrusionbar.cxx
@@ -125,23 +125,26 @@ static void impl_execute( SfxRequest const & rReq, 
SdrCustomShapeGeometryItem& r
     {
     case SID_EXTRUSION_TOGGLE:
     {
-        css::uno::Any* pAny = rGeometryItem.GetPropertyValueByName( 
sExtrusion, sExtrusion );
-
         bool bOn(false);
-        if( pAny )
+        css::uno::Any* pAny = rGeometryItem.GetPropertyValueByName( 
sExtrusion, sExtrusion );
+        if ( pAny )
         {
             (*pAny) >>= bOn;
             bOn = !bOn;
-            (*pAny) <<= bOn;
+            css::beans::PropertyValue aPropValue;
+            aPropValue.Name = sExtrusion;
+            aPropValue.Value <<= bOn;
+            rGeometryItem.SetPropertyValue(sExtrusion, aPropValue);
         }
         else
         {
             css::beans::PropertyValue aPropValue;
             aPropValue.Name = sExtrusion;
             aPropValue.Value <<= true;
-            rGeometryItem.SetPropertyValue( sExtrusion,  aPropValue );
+            rGeometryItem.SetPropertyValue(sExtrusion, aPropValue);
             bOn = true;
         }
+
         // draw:extrusion-diffusion has default 0% and c3DDiffuseAmt has 
default 100%. We set property
         // "Diffusion" with value 100% here if it does not exist already. This 
forces, that the
         // property is written to file in case an extrusion is newly created, 
and users of old
diff --git a/svx/source/toolbars/fontworkbar.cxx 
b/svx/source/toolbars/fontworkbar.cxx
index 888e2d2f43a9..7b2e8653b09d 100644
--- a/svx/source/toolbars/fontworkbar.cxx
+++ b/svx/source/toolbars/fontworkbar.cxx
@@ -247,7 +247,10 @@ static void impl_execute( SfxRequest const & rReq, 
SdrCustomShapeGeometryItem& r
                 bool bOn = false;
                 (*pAny) >>= bOn;
                 bOn = !bOn;
-                (*pAny) <<= bOn;
+                css::beans::PropertyValue aPropValue;
+                aPropValue.Name = "SameLetterHeights";
+                aPropValue.Value <<= bOn;
+                rGeometryItem.SetPropertyValue("TextPath", aPropValue);
             }
         }
         break;

Reply via email to