[[ Bug 20462 ]] Fix crash when undo group deletion

runrevmark · runrevmark · commit 804d341c4154 · 2017-12-11T12:59:20.000Z
This patch fixes a problem when a group is deleted, and then undo
is performed on it.

Deleting an object now causes it's object handle to be cleared
and removed. This means that all descendent object's parent
references are cleared.

When undoing, the control which is temporarily owned by the undo
list must be reinsterted into the object tree which requires
setting the parent references correctly.

This is done in MCControl::undo by using a visitor which visits
a control to recreate its weak proxy, and then visits its children
to set the parent link.
diff --git a/docs/notes/bugfix-20642.md b/docs/notes/bugfix-20642.md
@@ -0,0 +1 @@
+# Fix crash when undoing a group deletion
diff --git a/engine/src/control.cpp b/engine/src/control.cpp
@@ -561,6 +561,44 @@ void MCControl::paste(void)
 	}
 }
 
+/* The MCRereferenceChildrenVisitor visits each of a control's descendents
+ * recursively and makes sure they have a weak-proxy and that the parent is
+ * updated. */
+class MCRereferenceChildrenVisitor: public MCObjectVisitor
+{
+public:
+    static void Visit(MCControl *p_control)
+    {
+        MCRereferenceChildrenVisitor t_visitor(p_control);
+        
+        /* Make sure the control has a weak proxy (needed to set the parent of
+         * its children!). */
+        p_control->ensure_weak_proxy();
+        
+        /* Now visit the controls children. */
+        p_control->visit_children(0, 0, &t_visitor);
+    }
+
+private:
+    MCRereferenceChildrenVisitor(MCObject *p_new_parent)
+        : m_new_parent(p_new_parent)
+    {
+    }
+
+    bool OnControl(MCControl* p_control)
+    {
+        /* Update the control's parent */
+        p_control->setparent(m_new_parent);
+        
+        /* Update its children */
+        MCRereferenceChildrenVisitor::Visit(p_control);
+        
+        return true;
+    }
+    
+    MCObject *m_new_parent;
+};
+
 void MCControl::undo(Ustruct *us)
 {
 	MCRectangle newrect = rect;
@@ -583,7 +621,11 @@ void MCControl::undo(Ustruct *us)
 		{
 			MCCard *card = (MCCard *)parent->getcard();
 			getstack()->appendcontrol(this);
-			this->MCObject::m_weak_proxy = new MCObjectProxyBase(this);
+            
+            /* Visit the control and its children, creating weak_proxys and
+             * reparenting as we go. */
+            MCRereferenceChildrenVisitor::Visit(this);
+            
 			card->newcontrol(this, False);
 			Boolean oldrlg = MCrelayergrouped;
 			MCrelayergrouped = True;
diff --git a/engine/src/object.h b/engine/src/object.h
@@ -1239,6 +1239,10 @@ class MCObject :
     //  in the parent chain.
     bool isancestorof(MCObject *p_object);
     
+    // Reinstate the weak proxy object (used after an object is deleted, but is
+    // in the undo queue).
+    void ensure_weak_proxy(void) { if (m_weak_proxy == nullptr) m_weak_proxy = new MCObjectProxyBase(this); }
+    
     ////////// PROPERTY SUPPORT METHODS
 
 	void Redraw(void);
diff --git a/tests/lcs/core/interface/group.livecodescript b/tests/lcs/core/interface/group.livecodescript
@@ -49,3 +49,17 @@ on TestGroupByScript
 
    delete stack "Test"
 end TestGroupByScript
+
+on TestGroupUndo_Bug20642
+   create stack "Test"
+   set the defaultStack to "Test"
+
+   create button "A"
+   select button "A"
+   group
+   select the last group
+   delete
+   undo
+
+   TestAssert "Undo grouped button deletion does not crash", the short name of button "A" is "A"
+end TestGroupUndo_Bug20642

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+# Fix crash when undoing a group deletion`