Community
Participate
Working Groups
Any change made through the following gmf-property sections cannot be undone/redone: domain model: - generated XXXPropertySection notation model: - ConnectorAppearancePropertySection - generated XXXShapeColorAndFontPropertySection - DiagramColorsAndFontsPropertySection - RulerGridPropertySection Neither through the keybinding <ctrl>+<z> nor through menu...->edit...->undo. These changes from the properties view seem not to be registered in the operation history. Given that some features of diagram-elements can exclusively be modified through the corresponding property section, clients are not able to undo/redo their settings. GMF 2.0. provides undo/redo from property changes, GMF 2.1.0 and 2.1.1 do not.
the problem does not only exist for generated diagrams but also e.g. for Geoshape example. When you use this and change, e.g. font, the editor gets dirty but undo doesn't work.
I've taken a quick look. The regression in GMF's undo/redo behaviour orginates from the fix for bugzilla https://bugs.eclipse.org/bugs/show_bug.cgi?id=203351 The WorkspaceCommandStackImpl used to add the necessary ResourceUndoContext to the operation (an instance of CompositeCommand, in this case) via its DomainListener#historyNotification method. It doesn't anymore. I think this fix simply exposed a defect waiting to happen in GMF. The DiagramEditor's operation history listener doesn't add the required undo context because the CompositeCommand doesn't have the expected ResourceUndoContext anymore - although one of its children does. Seems to me that CompositeCommand (and CompositeEMFOperation from EMF Transaction) ought to have an implementation of #getContexts and #hasContext that aggregate all of the contexts of their children. Unfortunately, these methods are final in AbstractOperation (not sure why, more investigation required...). In the mean time, we could work around the issue by adding logic to DiagramEditor#shouldAddUndoContext to consider the undo contexts on children of composite commands.
Adding /** * @generated NOT */ @Override protected boolean shouldAddUndoContext(IUndoableOperation operation) { return true; } to the generated XXXDiagramEditor seems to do the trick as a workaround for my use-case. Changes to the notation as well as the domain model triggered by the GMF-property-tabs can be undone/redone now.
I've opened bug 248483 on the platform to request removing 'final' from AbstractOperation#getContexts and AbstractOperation#hasContext. However, even if we can override hasContext and getContexts for CompositeEMFOperation and CompositeCommand to aggregate contexts from their children, there's no guarantee that those operations aren't nested in someone else's "composite" on the history. I think we should restore the old behaviour in WorkspaceCommandStackImpl.DomainListener to make sure that when an operation affects an EMF resource in a transactional editing domain, the corresponding ResourceUndoContext is added to the operation currently executing on the history. To this end, I'm moving this bug to EMF Transaction to request that this behaviour be restored to WorkspaceCommandStackImpl.DomainListener.
I'll work on this.
Created attachment 113415 [details] Proposed fix and JUnit test
Comment on attachment 113415 [details] Proposed fix and JUnit test Patch committed to 1.2.3 maintenance branch.
Propagated fix to HEAD (1.3 branch).
Fix available in HEAD: 1.3.0.I200811021821.
