Issue 120015

Created attachment 78364 [details]
Fix patch for this issue

Root Cause:
When apply one style sheet to one object,the old style sheet will be saved in the SdrUndoAttrObj,but when the old style is deleted by user,this reference will not be changed,so the app will crash when undo.

Resolution:
Make the SdrUndoAtrrObj as one listener to the applied style sheet, if this style sheet is destroyed,we change the referenced style sheet to the default style sheet.

ALG: Shouldnt the deletion of the Style also create a undo action, so that the stack is complete and undoing would first restore the style, then reset the style at the SdrObject. I thik it is an error that no undo action gets created for deletingthe StyleSheet.

Change status to CONFIRMED.

ALG: I investigated, none of the actions on the opened styles dialog (F11) creates any undo/redo actions, this is missing implementation functionality and too much to create now for this fix.
I saw that SdrUndoAttrObj which is used to protocol SfxItemSet changes to SdrObjects uses SfxStyleSheet* to remember the used StyleSheets. One possibility would be to use the Strings Sheet.GetName(), GetFamily() to remember the Style instead and do a SfxStyleSheetBasePool::Find(..) on Undo/Redo to detect if the style still exists. The default style could then be used. This still allows no true undo functionality, settng the default style is not correct.
Interestingly SfxStyleSheet is derived from SfxStyleSheetBase and that is derived from comphelper::OWeakTypeObject, thus refcounted. It is possible to use

rtl::Reference< SfxStyleSheetBase > mxUndoStyleSheet;

instead of

SfxStyleSheet*				pUndoStyleSheet;

This allows to survive the removal of the StyleSheet from the StyleSheetPool. When undoing/redoing code is needed to check if it still is in the pool, add it if not and use it.
This allows full undo/redo even with removed StyleSheets; on undo a deleted StyleSheet is re-inserted to the StyleSheetPool, on redo it will stay there as if deletion did not happen. This is acceptable due to the fact that StyleSheet removal has no undo itself and that it will be removed anyways with the next save/load cycle (AFAIK only used StyleSheets survive, but not sure right now).

I will add the code so far as patch. It still triggers one assertion (not find parent), but parent works and is set. I'm pretty sure the assertion is wrong.

Created attachment 78394 [details]
Partial solution with using refcounting of SfxStyleSheetBase

ALG: The patch creates living documents which throw errors on saving. Investigation shows that in SfxStyleSheetBasePool::Remove the UNO API implementation is got and the object gets disposed, thus being invalid. Re-inserting such an object is of course erroneous.
I checked object deletion by watching SfxStyleSheetBase constructors and destructors, works well just by refcounting. It seems an error to dispose the object when it gets removed from the StyleSheetPool.
Also a small correction in SdrUndoAttrObj::ensureStyleSheetInStyleSheetPool. Insert the StyleSheet without parent and set the parent afterwards, this makes the assertion go away.
Adding a 2nd patch proposal with the mentioned changes.

Created attachment 78395 [details]
Corrected patch

(In reply to comment #2)
> ALG: Shouldnt the deletion of the Style also create a undo action, so that
> the stack is complete and undoing would first restore the style, then reset
> the style at the SdrObject. I thik it is an error that no undo action gets
> created for deletingthe StyleSheet.

  Thanks for you suggestion. It is a better solution and I am agree with you.
  I have some questions about adding undo action for deleting StyleSheet.
    1. If the "Style list" isn't closed, the undo action just re-add the style sheet to "Style sheet", it is all right.
    2. If the "Style list" is closed, we may have some choices:
       a. open the "Style list " dialog and re-add the style sheet
       b. re-add the style sheet and show nothing in the UI
       c. re-add the style sheet and reset the style at the SdrObject

  Do you have any ideas about them? Thanks!

ALG: Hi Ma Bingbing,

I would try to get as close to the behaviour as if the actions on the dialog would produce undo actions (what is just missing), thus the less possible change in behaviour will happen when someone would implement these missing undos. This means when undoing, first the (a) 'delete style' and then the (b) 'Style applied to SdrObject' would be undone.

Since we only have (b), (a) would be implicit and only if needed (if a deleted style was used by (b)). It will re-add the style to the model and apply it to the object, so everything is as before the undo.
If the dialog is open, it will show again the style. If not open, no visual feedback. This is exactly the same as if undo (b) existed and was executed.

Comitting the change now.

ALG: Comitted, done.

Have no crash according reproduce procedure.
Verified pass.

Verified pass on AOo trunk r1378003.