tdf#117707 Correctly Scale ItemSets when pasting from External

Change-Id: Id01777a786fa7bb75d46ee0f78da645e94f7b840
Reviewed-on: https://gerrit.libreoffice.org/57556
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@cib.de>

include/svx/sdr/properties/defaultproperties.hxx

@@ -101,9 +101,6 @@ namespace sdr
             // DefaultProperties::GetObjectItemSet() if a new ItemSet is created.
             // Default implementation does nothing.
             virtual void ForceDefaultAttributes();
-
-            // Scale the included ItemSet.
-            void Scale(const Fraction& rScale);
         };
     } // end of namespace properties
 } // end of namespace sdr

svx/source/sdr/properties/attributeproperties.cxx

@@ -158,44 +158,53 @@ namespace sdr
         {
             SfxStyleSheet* pTargetStyleSheet(rProps.GetStyleSheet());
 
-            if(pTargetStyleSheet &&
-                &rObj.getSdrModelFromSdrObject() != &rProps.GetSdrObject().getSdrModelFromSdrObject())
+            if(pTargetStyleSheet)
             {
-                // tdf#117506
-                // The error shows that it is definitely necessary to solve this problem.
-                // Interestingly I already had a note here for 'work needed'.
-                // Checked in libreoffice-6-0 what happened there. In principle, the whole
-                // ::Clone of SdrPage and SdrObject happened in the same SdrModel, only
-                // afterwards a ::SetModel was used at the cloned SdrPage which went through
-                // all layers. The StyleSheet-problem was solved in
-                // AttributeProperties::MoveToItemPool at the end. There, a StyleSheet with the
-                // same name was searched for in the target-SdrModel.
-                // Start by resetting the current TargetStyleSheet so that nothing goes wrong
-                // when we do not find a fitting TargetStyleSheet.
-                // Note: The test for SdrModelChange above was wrong (compared the already set
-                // new SdrObject), so this never triggered and pTargetStyleSheet was never set to
-                // nullptr before. This means that a StyleSheet from another SdrModel was used
-                // what of course is very dangerous. Interestingly did not crash since when that
-                // other SdrModel was destroyed the ::Notify mechanism still worked reliably
-                // and de-connected this Properties successfully from the alien-StyleSheet.
-                pTargetStyleSheet = nullptr;
-
-                // Check if we have a TargetStyleSheetPool at the target-SdrModel. This *should*
-                // be the case already (SdrModel::Merge and SdDrawDocument::InsertBookmarkAsPage)
-                // have already cloned the StyleSheets to the target-SdrModel.
-                // If none is found, ImpGetDefaultStyleSheet will be used to set a 'default'
-                // StyleSheet as StyleSheet (that's what happened in the task, thus the FillStyle
-                // changed to the 'default' Blue).
-                SfxStyleSheetBasePool* pTargetStyleSheetPool(rObj.getSdrModelFromSdrObject().GetStyleSheetPool());
-
-                if(nullptr != pTargetStyleSheetPool)
+                const bool bModelChange(&rObj.getSdrModelFromSdrObject() != &rProps.GetSdrObject().getSdrModelFromSdrObject());
+
+                if(bModelChange)
                 {
-                    // If we have a TargetStyleSheetPool, search for the StyleSheet used
-                    // in the original Properties in the source-SdrModel.
-                    pTargetStyleSheet = dynamic_cast< SfxStyleSheet* >(
-                        pTargetStyleSheetPool->Find(
-                            rProps.GetStyleSheet()->GetName(),
-                            SfxStyleFamily::All));
+                    // tdf#117506
+                    // The error shows that it is definitely necessary to solve this problem.
+                    // Interestingly I already had a note here for 'work needed'.
+                    // Checked in libreoffice-6-0 what happened there. In principle, the whole
+                    // ::Clone of SdrPage and SdrObject happened in the same SdrModel, only
+                    // afterwards a ::SetModel was used at the cloned SdrPage which went through
+                    // all layers. The StyleSheet-problem was solved in
+                    // AttributeProperties::MoveToItemPool at the end. There, a StyleSheet with the
+                    // same name was searched for in the target-SdrModel.
+                    // Start by resetting the current TargetStyleSheet so that nothing goes wrong
+                    // when we do not find a fitting TargetStyleSheet.
+                    // Note: The test for SdrModelChange above was wrong (compared the already set
+                    // new SdrObject), so this never triggered and pTargetStyleSheet was never set to
+                    // nullptr before. This means that a StyleSheet from another SdrModel was used
+                    // what of course is very dangerous. Interestingly did not crash since when that
+                    // other SdrModel was destroyed the ::Notify mechanism still worked reliably
+                    // and de-connected this Properties successfully from the alien-StyleSheet.
+                    pTargetStyleSheet = nullptr;
+
+                    // Check if we have a TargetStyleSheetPool at the target-SdrModel. This *should*
+                    // be the case already (SdrModel::Merge and SdDrawDocument::InsertBookmarkAsPage
+                    // have already cloned the StyleSheets to the target-SdrModel when used in Draw/impress).
+                    // If none is found, ImpGetDefaultStyleSheet will be used to set a 'default'
+                    // StyleSheet as StyleSheet implicitely later (that's what happened in the task,
+                    // thus the FillStyle changed to the 'default' Blue).
+                    // Note: It *may* be necessary to do more for StyleSheets, e.g. clone/copy the
+                    // StyleSheet Hierarchy from the source SdrModel and/or add the Items from there
+                    // as hard attibutes. If needed, have a look at the older AttributeProperties::SetModel
+                    // implementation from e.g. libreoffice-6-0.
+                    SfxStyleSheetBasePool* pTargetStyleSheetPool(rObj.getSdrModelFromSdrObject().GetStyleSheetPool());
+
+                    if(nullptr != pTargetStyleSheetPool)
+                    {
+                        // If we have a TargetStyleSheetPool, search for the used StyleSheet
+                        // in the target SdrModel using the Name from the original StyleSheet
+                        // in the source-SdrModel.
+                        pTargetStyleSheet = dynamic_cast< SfxStyleSheet* >(
+                            pTargetStyleSheetPool->Find(
+                                rProps.GetStyleSheet()->GetName(),
+                                SfxStyleFamily::All));
+                    }
                 }
             }
 

svx/source/sdr/properties/defaultproperties.cxx

@@ -58,24 +58,27 @@ namespace sdr
                 // Clone may be to another model and thus another ItemPool.
                 // SfxItemSet supports that thus we are able to Clone all
                 // SfxItemState::SET items to the target pool.
-                mpItemSet =
-                    rProps.mpItemSet->Clone(
-                        true,
-                        &rObj.getSdrModelFromSdrObject().GetItemPool());
+                mpItemSet = rProps.mpItemSet->Clone(
+                    true,
+                    &rObj.getSdrModelFromSdrObject().GetItemPool());
 
                 // React on ModelChange: If metric has changed, scale items.
                 // As seen above, clone is supported, but scale is not included,
                 // thus: TTTT maybe add scale to SfxItemSet::Clone() (?)
-                if(&rObj.getSdrModelFromSdrObject() != &GetSdrObject().getSdrModelFromSdrObject())
+                // tdf#117707 correct ModelChange detection
+                const bool bModelChange(&rObj.getSdrModelFromSdrObject() != &rProps.GetSdrObject().getSdrModelFromSdrObject());
+
+                if(bModelChange)
                 {
-                    const MapUnit aOldUnit(GetSdrObject().getSdrModelFromSdrObject().GetScaleUnit());
+                    const MapUnit aOldUnit(rProps.GetSdrObject().getSdrModelFromSdrObject().GetScaleUnit());
                     const MapUnit aNewUnit(rObj.getSdrModelFromSdrObject().GetScaleUnit());
                     const bool bScaleUnitChanged(aNewUnit != aOldUnit);
 
                     if(bScaleUnitChanged)
                     {
                         const Fraction aMetricFactor(GetMapFactor(aOldUnit, aNewUnit).X());
-                        Scale(aMetricFactor);
+
+                        ScaleItemSet(*mpItemSet, aMetricFactor);
                     }
                 }
 
@@ -229,14 +232,6 @@ namespace sdr
         {
         }
 
-        void DefaultProperties::Scale(const Fraction& rScale)
-        {
-            if(mpItemSet)
-            {
-                ScaleItemSet(*mpItemSet, rScale);
-            }
-        }
-
         void DefaultProperties::dumpAsXml(struct _xmlTextWriter * pWriter) const
         {
             xmlTextWriterStartElement(pWriter, BAD_CAST("DefaultProperties"));