From ee2ee942524cefc9ab29989bc53ccc008288c524 Mon Sep 17 00:00:00 2001
From: elkowar <5300871+elkowar@users.noreply.github.com>
Date: Sun, 11 Oct 2020 22:19:29 +0200
Subject: [PATCH] Correctly clean up old onhover, onscroll, etc handlers

---
 src/widgets/widget_definitions.rs | 69 +++++++++++++++----------------
 1 file changed, 34 insertions(+), 35 deletions(-)

diff --git a/src/widgets/widget_definitions.rs b/src/widgets/widget_definitions.rs
index 0dc17a3..cab3a84 100644
--- a/src/widgets/widget_definitions.rs
+++ b/src/widgets/widget_definitions.rs
@@ -52,15 +52,8 @@ pub(super) fn resolve_widget_attrs(bargs: &mut BuilderArgs, gtk_widget: &gtk::Wi
         })
     }
 
-    let on_scroll_handler: Rc<RefCell<Option<Box<dyn Fn(ScrollDir)>>>> = Rc::new(RefCell::new(None));
-    gtk_widget.add_events(gdk::EventMask::SCROLL_MASK);
-    gtk_widget.add_events(gdk::EventMask::SMOOTH_SCROLL_MASK);
-    gtk_widget.connect_scroll_event(glib::clone!(@strong on_scroll_handler => move |_, evt| {
-        if let Some(ref handler) = *on_scroll_handler.borrow() {
-            handler(if evt.get_delta().1 < 0f64 {ScrollDir::Up } else {ScrollDir::Down});
-        }
-        gtk::Inhibit(false)
-    }));
+    let on_scroll_handler_id: Rc<RefCell<Option<glib::SignalHandlerId>>> = Rc::new(RefCell::new(None));
+    let on_hover_handler_id: Rc<RefCell<Option<glib::SignalHandlerId>>> = Rc::new(RefCell::new(None));
 
     resolve_block!(bargs, gtk_widget, {
         prop(class:   as_string) { gtk_widget.get_style_context().add_class(&class) },
@@ -79,14 +72,25 @@ pub(super) fn resolve_widget_attrs(bargs: &mut BuilderArgs, gtk_widget: &gtk::Wi
             gtk_widget.get_style_context().add_provider(&css_provider, gtk::STYLE_PROVIDER_PRIORITY_APPLICATION)
         },
         prop(onscroll: as_string) {
-            on_scroll_handler.replace(Some(Box::new(move |evt| run_command(&onscroll, evt))));
+            gtk_widget.add_events(gdk::EventMask::SCROLL_MASK);
+            gtk_widget.add_events(gdk::EventMask::SMOOTH_SCROLL_MASK);
+            let old_id = on_scroll_handler_id.replace(Some(
+                gtk_widget.connect_scroll_event(move |_, evt| {
+                    run_command(&onscroll, if evt.get_delta().1 < 0f64 { "up" } else { "down" });
+                    gtk::Inhibit(false)
+                })
+            ));
+            old_id.map(|id| gtk_widget.disconnect(id));
         },
         prop(onhover: as_string) {
             gtk_widget.add_events(gdk::EventMask::ENTER_NOTIFY_MASK);
-            gtk_widget.connect_enter_notify_event(move |_, evt| {
-                run_command(&onhover, format!("{} {}", evt.get_position().0, evt.get_position().1));
-                gtk::Inhibit(false)
-            });
+            let old_id = on_hover_handler_id.replace(Some(
+                gtk_widget.connect_scroll_event(move |_, evt| {
+                    run_command(&onhover, format!("{} {}", evt.get_position().0, evt.get_position().1));
+                    gtk::Inhibit(false)
+                })
+            ));
+            old_id.map(|id| gtk_widget.disconnect(id));
         }
     });
 }
@@ -100,16 +104,21 @@ pub(super) fn resolve_container_attrs(bargs: &mut BuilderArgs, gtk_widget: &gtk:
 }
 
 pub(super) fn resolve_range_attrs(bargs: &mut BuilderArgs, gtk_widget: &gtk::Range) {
+    let on_change_handler_id: Rc<RefCell<Option<glib::SignalHandlerId>>> = Rc::new(RefCell::new(None));
     resolve_block!(bargs, gtk_widget, {
         prop(value       : as_f64)    { gtk_widget.set_value(value)},
         prop(min         : as_f64)    { gtk_widget.get_adjustment().set_lower(min)},
         prop(max         : as_f64)    { gtk_widget.get_adjustment().set_upper(max)},
         prop(orientation : as_string) { gtk_widget.set_orientation(parse_orientation(&orientation)?) },
         prop(onchange    : as_string) {
-            // TODO FIX: if this is loaded by a varref, and changes, the value _must_ be disconnected.
-            gtk_widget.connect_value_changed(move |gtk_widget| {
-                run_command(&onchange, gtk_widget.get_value());
-            });
+            gtk_widget.add_events(gdk::EventMask::ENTER_NOTIFY_MASK);
+            let old_id = on_change_handler_id.replace(Some(
+                gtk_widget.connect_value_changed(move |gtk_widget| {
+                    run_command(&onchange, gtk_widget.get_value());
+                })
+            ));
+            old_id.map(|id| gtk_widget.disconnect(id));
+
         }
     });
 }
@@ -136,10 +145,15 @@ fn build_gtk_scale(bargs: &mut BuilderArgs) -> Result<gtk::Scale> {
 
 fn build_gtk_button(bargs: &mut BuilderArgs) -> Result<gtk::Button> {
     let gtk_widget = gtk::Button::new();
+    let on_click_handler_id: Rc<RefCell<Option<glib::SignalHandlerId>>> = Rc::new(RefCell::new(None));
     resolve_block!(bargs, gtk_widget, {
         prop(onclick: as_string) {
-            // TODO FIX: if this is loaded by a varref, and changes, the value _must_ be disconnected.
-            gtk_widget.connect_clicked(move |_| run_command(&onclick, ""));
+            gtk_widget.add_events(gdk::EventMask::ENTER_NOTIFY_MASK);
+            let old_id = on_click_handler_id.replace(Some(
+                gtk_widget.connect_clicked(move |_| run_command(&onclick, ""))
+            ));
+            old_id.map(|id| gtk_widget.disconnect(id));
+
         }
     });
     Ok(gtk_widget)
@@ -254,18 +268,3 @@ fn connect_first_map<W: IsA<gtk::Widget>, F: Fn(&W) + 'static>(widget: &W, func:
         }
     });
 }
-
-#[derive(Debug, Eq, PartialEq)]
-pub enum ScrollDir {
-    Up,
-    Down,
-}
-
-impl fmt::Display for ScrollDir {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match self {
-            ScrollDir::Up => write!(f, "up"),
-            ScrollDir::Down => write!(f, "down"),
-        }
-    }
-}