[[ Bug 22121 ]] Implement delete undo for widgets

montegoulding · montegoulding · commit 2208e44cccd2 · 2019-06-04T09:54:21.000+10:00
This patch ensures that when widgets are added to the undo queue when the selected
objects are deleted that the undo will rebind the widget correctly. This special
case of deleting a widget but leaving it in a state where no data loss will occur if
it is replaced with undo is handled by a new method `delforundo`.

This method ensures that the widget on the undo queue is in its correct unbound state
with an updated rep if the widget was bound. When the undo operation is a replace of
a deleted widget then the widget is re-bound. When the undo operation is a delete of
a replaced widget then the `delforundo` method is again called.
diff --git a/docs/notes/bugfix-22121.md b/docs/notes/bugfix-22121.md
@@ -0,0 +1 @@
+# Fix widgets not rebinding when undo is called after deletion
diff --git a/engine/src/control.cpp b/engine/src/control.cpp
@@ -44,6 +44,7 @@ along with LiveCode.  If not see <http://www.gnu.org/licenses/>.  */
 #include "bitmapeffect.h"
 #include "graphicscontext.h"
 #include "graphics_util.h"
+#include "widget.h"
 
 #include "exec.h"
 
@@ -631,7 +632,14 @@ void MCControl::undo(Ustruct *us)
 		}
 		return;
 	case UT_REPLACE:
-		del(true);
+        if (gettype() == CT_WIDGET)
+        {
+            static_cast<MCWidget*>(this)->delforundo(true);
+        }
+        else
+        {
+            del(true);
+        }
 		us->type = UT_DELETE;
 		return;
 	default:
diff --git a/engine/src/sellst.cpp b/engine/src/sellst.cpp
@@ -29,6 +29,7 @@ along with LiveCode.  If not see <http://www.gnu.org/licenses/>.  */
 #include "field.h"
 #include "paragraf.h"
 #include "image.h"
+#include "widget.h"
 #include "mcerror.h"
 #include "sellst.h"
 #include "undolst.h"
@@ -583,8 +584,18 @@ Boolean MCSellist::del()
             MCControl *cptr = tptr->m_ref.GetAs<MCControl>();
             uint2 num = 0;
             cptr->getcard()->count(CT_LAYER, CT_UNDEFINED, cptr, num, True);
+            
+            bool t_del;
+            if (cptr->gettype() == CT_WIDGET)
+            {
+                t_del = static_cast<MCWidget*>(cptr)->delforundo(true);
+            }
+            else
+            {
+                t_del = cptr->del(true);
+            }
 
-            if (cptr->del(true))
+            if (t_del)
             {
                 Ustruct *us = new (nothrow) Ustruct;
                 us->type = UT_DELETE;
diff --git a/engine/src/widget.cpp b/engine/src/widget.cpp
@@ -44,6 +44,7 @@
 #include "system.h"
 #include "globals.h"
 #include "context.h"
+#include "undolst.h"
 
 #include "widget-ref.h"
 #include "widget-events.h"
@@ -776,6 +777,54 @@ Boolean MCWidget::del(bool p_check_flag)
     return True;
 }
 
+void MCWidget::undo(Ustruct *us)
+{
+    MCControl::undo(us);
+
+    if (us->type == UT_REPLACE)
+    {
+        MCAutoValueRef t_rep;
+        t_rep.Give(m_rep);
+        m_rep = nullptr;
+        
+        MCNewAutoNameRef t_kind;
+        t_kind.Give(m_kind);
+        m_kind = nullptr;
+        
+        bind(*t_kind, *t_rep);
+    }
+}
+
+Boolean MCWidget::delforundo(bool p_check_flag)
+{
+    MCAutoValueRef t_rep;
+    // Make the widget generate a rep.
+    if (m_rep == nullptr)
+    {
+        if (m_widget != nullptr)
+        {
+            MCWidgetOnSave(m_widget, &t_rep);
+        }
+        
+        if (!t_rep.IsSet())
+        {
+            t_rep = kMCNull;
+        }
+    }
+    
+    if (!del(p_check_flag))
+    {
+        return false;
+    }
+    
+    if (t_rep.IsSet())
+    {
+        m_rep = t_rep.Take();
+    }
+    return true;
+}
+
+
 void MCWidget::OnOpen()
 {
 	if (m_widget != nil)
diff --git a/engine/src/widget.h b/engine/src/widget.h
@@ -195,6 +195,9 @@ class MCWidget: public MCControl, public MCMixinObjectHandle<MCWidget>
     
     virtual Boolean del(bool p_check_flag);
     
+    virtual void undo(Ustruct *us);
+    Boolean delforundo(bool p_check_flag);
+    
 	virtual void OnOpen();
 	virtual void OnClose();
 	
diff --git a/tests/lcs/core/engine/widget.livecodescript b/tests/lcs/core/engine/widget.livecodescript
@@ -115,3 +115,45 @@ on TestWidgetRetainRep
    delete stack "WidgetRetainRepTest"
    delete file tPath
 end TestWidgetRetainRep
+
+on TestWidgetDeleteUndo
+   -- ensure the extension is not already loaded
+   if "com.livecode.lcs_tests.core.widget_save" is \
+            among the lines of the loadedExtensions then
+      unload extension "com.livecode.lcs_tests.core.widget_save"
+   end if
+
+   create stack "WidgetDeleteUndo"
+   set the defaultStack to "WidgetDeleteUndo"
+
+   local tWidget
+   put "com.livecode.lcs_tests.core.widget_save" into tWidget["$kind"]
+   put uuid() into tWidget["$state"]["info"]
+   import widget from array tWidget
+
+   select widget 1
+   delete
+   undo
+
+   local tOut
+   export widget 1 to array tOut
+   TestAssert "Undo on deleted unbound extension retains rep", tOut["$state"]["info"] is tWidget["$state"]["info"]
+   
+   select widget 1
+   delete
+
+   TestLoadAuxiliaryExtension "_widget_save"
+   undo
+   TestAssert "Undo on deleted unbound extension rebinds and retains rep", the savedInfo of widget 1 is tWidget["$state"]["info"]
+   
+   select widget 1
+   delete
+   undo -- replace
+   TestAssert "Undo on deleted bound extension rebinds and retains rep", the savedInfo of widget 1 is tWidget["$state"]["info"]
+   
+   undo -- delete
+   undo -- replace
+   TestAssert "Undo delete/replace on deleted bound extension rebinds and retains rep", the savedInfo of widget 1 is tWidget["$state"]["info"]
+   
+   delete stack "WidgetDeleteUndo"
+end TestWidgetDeleteUndo

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+# Fix widgets not rebinding when undo is called after deletion`