GNOME.org
 

[gtkmm] Gtk::ScrolledWindow: Deprecate remove_with_viewport()

commit 71cb2fd9e12a4cbf557d6f944b6aa3055551fd9c
Author: Kjell Ahlstedt <kjell ahlstedt bredband net>
Date:   Sun Jun 26 12:54:59 2016 +0200

    Gtk::ScrolledWindow: Deprecate remove_with_viewport()
    
    * gtk/src/bin.[ccg|hg]: Bin::remove() calls Container::remove().
    * gtk/src/container.[ccg|hg]: Container::add() and Container::remove() handle
    all the complications with an added Viewport in a ScrolledWindow.
    * gtk/src/scrolledwindow.[ccg|hg]: Deprecate remove_with_viewport() in favour
    of remove().
    * gtk/src/gtk_docs_override.xml: Move the description of gtk_container_remove()
    to container.hg.
    
    The fix of gtk+ bug 710471 has made gtk_scrolled_window_remove() smart enough
    to destroy a GtkViewport that gtk_scrolled_window_add() has created.
    A call to gtk_container_remove() now does all that remove_with_viewport()
    used to do. Bug #685739.

 gtk/src/bin.ccg               |   21 ++-----------
 gtk/src/bin.hg                |   14 +++++----
 gtk/src/container.ccg         |   65 +++++++++++++++++++++++++++++++++++------
 gtk/src/container.hg          |   17 +++++++---
 gtk/src/gtk_docs_override.xml |   10 ------
 gtk/src/scrolledwindow.ccg    |   43 +++------------------------
 gtk/src/scrolledwindow.hg     |   23 ++++++---------
 7 files changed, 93 insertions(+), 100 deletions(-)
---
diff --git a/gtk/src/bin.ccg b/gtk/src/bin.ccg
index 73077be..ddb07d2 100644
--- a/gtk/src/bin.ccg
+++ b/gtk/src/bin.ccg
@@ -25,27 +25,12 @@
 namespace Gtk
 {
 
-
 void
 Bin::remove()
 {
-  auto child = gtk_bin_get_child(gobj());
-  if(child)
-  {
-    auto cppChild = Glib::wrap(child);
-
-    //If this is a managed widget, then do an extra ref so that it will not be
-    //destroyed when it is removed, and restore the floating state of the ref.
-    //This should leave it in the same state as when it was instantiated,
-    //before being added to the first container.
-    if(cppChild->is_managed_())
-    {
-      cppChild->reference();
-      g_object_force_floating(static_cast<Glib::Object*>(cppChild)->gobj());
-    }
-
-    gtk_container_remove(Container::gobj(), cppChild->gobj());
-  }
+  auto child = get_child();
+  if (child)
+    Container::remove(*child);
 }
 
 _DEPRECATE_IFDEF_START
diff --git a/gtk/src/bin.hg b/gtk/src/bin.hg
index c7f6f42..5e51794 100644
--- a/gtk/src/bin.hg
+++ b/gtk/src/bin.hg
@@ -47,13 +47,15 @@ public:
   _WRAP_METHOD(Gtk::Widget* get_child(), gtk_bin_get_child)
   _WRAP_METHOD(const Gtk::Widget* get_child() const, gtk_bin_get_child)
 
-  /** Remove the contained object
-   * Since this can only hold one object it is not necessary to
-   * specify which object to remove like other containers.
+  /** Remove the contained widget.
+   * Since this can only hold one widget it is not necessary to
+   * specify which widget to remove like other containers.
    *
-   * When calling remove() on a Gtk::ScrolledWindow this might not remove the
-   * expected child directly, because Gtk::ScrolledWindow::add() sometimes creates a
-   * Gtk::ViewPort child and places the widget in that.
+   * Sometimes when a widget is added to a Gtk::ScrolledWindow, a Gtk::Viewport
+   * is created and inserted between the ScrolledWindow and the added widget.
+   * If the child is such an automatically created Viewport, remove() removes
+   * the Viewport's child from the Viewport, and removes the Viewport from the
+   * ScrolledWindow. The Viewport is destroyed.
    */
   void remove();
 
diff --git a/gtk/src/container.ccg b/gtk/src/container.ccg
index 2c3b7a2..28aa4a6 100644
--- a/gtk/src/container.ccg
+++ b/gtk/src/container.ccg
@@ -18,12 +18,19 @@
 #include <glibmm/vectorutils.h>
 
 #include <gtkmm/adjustment.h>
+#include <gtkmm/scrolledwindow.h>
 #include <gtk/gtk.h>
 
 
 namespace
 {
 
+// Marks a widget that has been automatically created and added by gtk_container_add().
+// The only reason we must use this GQuark or a similar trick, is that we want
+// to restore the state of a managed removed object.
+// gtk_container_remove() does not do that.
+GQuark quark_auto_added_widget = g_quark_from_static_string("gtkmm_Container::auto_added_widget");
+
 static void container_foreach_callback(GtkWidget* widget_gobj, void* data)
 {
   try
@@ -172,9 +179,31 @@ void Container::forall(const Container::ForeachSlot& slot)
   gtk_container_forall(gobj(), &container_foreach_callback, &slot_copy);
 }
 
-void  Container::add(Widget& widget)
+// Why not let both add() and remove() be virtual, and override them in ScrolledWindow?
+// Because a virtual add() has an unwanted side-effect in Gtk::Stack.
+// See https://bugzilla.gnome.org/show_bug.cgi?id=724732
+// And a virtual remove() can be problematic because ScrolledWindow and other
+// subclasses of Bin have a remove() overload with no parameter.
+
+void Container::add(Widget& widget)
 {
-  gtk_container_add(gobj(), widget.gobj());
+  auto scrolled_window = dynamic_cast<ScrolledWindow*>(this);
+  if (scrolled_window)
+  {
+    auto old_child = gtk_bin_get_child(scrolled_window->Bin::gobj()); // Normally nullptr
+    gtk_container_add(gobj(), widget.gobj());
+    auto child = gtk_bin_get_child(scrolled_window->Bin::gobj());
+    if (child && child != old_child)
+    {
+      // If the child is not the inserted widget, gtk_scrolled_window_add() has
+      // created and inserted a GtkViewport. Mark this GtkViewport as automatically
+      // added. Container::remove() needs this information.
+      const gpointer qdata_value = (child != widget.gobj()) ? child : nullptr;
+      g_object_set_qdata(G_OBJECT(child), quark_auto_added_widget, qdata_value);
+    }
+  }
+  else
+    gtk_container_add(gobj(), widget.gobj());
 }
 
 bool Container::has_focus_chain() const
@@ -214,16 +243,34 @@ void Container::show_all_children(bool recursive)
 
 void Container::remove(Widget& widget)
 {
-  //If this is a managed widget, then do an extra ref so that it will not be
-  //destroyed when it is removed, and restore the floating state of the ref.
-  //This should leave it in the same state as when it was instantiated,
-  //before being added to the first container.
-  if(widget.is_managed_())
+  auto child_to_float = &widget;
+  if (g_object_get_qdata(widget.Glib::Object::gobj(), quark_auto_added_widget))
+  {
+    // If widget was created by gtk_scrolled_window_add(),
+    // don't restore the floating state of that widget. Instead, restore the
+    // state of that widget's child, which was added by add().
+    auto scrolled_window = dynamic_cast<ScrolledWindow*>(this);
+    if (scrolled_window)
+    {
+      auto bin = dynamic_cast<Bin*>(&widget);
+      if (bin)
+        child_to_float = bin->get_child();
+    }
+  }
+
+  // If this is a managed widget, then do an extra ref so that it will not be
+  // destroyed when it is removed, and restore the floating state of the ref.
+  // This should leave it in the same state as when it was instantiated,
+  // before being added to the container.
+  if (child_to_float && child_to_float->is_managed_())
   {
-    widget.reference();
-    g_object_force_floating(static_cast<Glib::Object&>(widget).gobj());
+    child_to_float->reference();
+    g_object_force_floating(child_to_float->Glib::Object::gobj());
   }
 
+  // gtk_container_remove() removes what gtk_container_add() added, even if
+  // gtk_container_add() added an extra GtkViewport in a GtkScrolledWindow.
+  // See https://bugzilla.gnome.org/show_bug.cgi?id=710471
   gtk_container_remove(gobj(), widget.gobj());
 }
 
diff --git a/gtk/src/container.hg b/gtk/src/container.hg
index 679fc6a..881b8f3 100644
--- a/gtk/src/container.hg
+++ b/gtk/src/container.hg
@@ -69,10 +69,21 @@ public:
   //This is virtual so that we can override it in Gtk::ScrolledWindow.
   //TODO: Remove the virtual keyword when we can break ABI,
   //because the override in ScrolledWindow is no longer necessary.
+  _WRAP_METHOD_DOCS_ONLY(gtk_container_add)
   virtual void add(Widget& widget);
   _IGNORE(gtk_container_add)
 
-  _WRAP_METHOD_DOCS_ONLY(gtk_container_remove)
+  /** Removes @a widget from the container.
+   * @a widget must be inside this container.
+   * If @a widget is managed with Gtk::manage(), and you don't want to use @a widget
+   * again, then you should delete it, because there will no longer be any parent
+   * container to delete it automatically.
+   *
+   * What's said about ScrolledWindow in the documentation of Bin::remove()
+   * applies also to Container::remove().
+   *
+   * @param widget A current child of the container.
+   */
   void remove(Widget& widget);
   _IGNORE(gtk_container_remove)
 
@@ -136,9 +147,6 @@ public:
 
   _WRAP_METHOD(void unset_focus_chain(), gtk_container_unset_focus_chain)
 
-
-
-
 /* Widget-level methods */
 
   _WRAP_METHOD(void set_reallocate_redraws(bool needs_redraws = true),
@@ -273,7 +281,6 @@ dnl
     static void remove_callback_normal(GtkContainer* self, GtkWidget* p0);
   _POP()
 #m4end
-
 };
 
 } // namespace Gtk
diff --git a/gtk/src/gtk_docs_override.xml b/gtk/src/gtk_docs_override.xml
index 054985c..581c5e8 100644
--- a/gtk/src/gtk_docs_override.xml
+++ b/gtk/src/gtk_docs_override.xml
@@ -1485,16 +1485,6 @@ Since: 2.4
 </description>
 </function>
 
-
-<function name="gtk_container_remove">
-<description>
-Removes @widget from @container. @widget must be inside @container.
-If @widget is managed with Gtk::manage(), and you don&apos;t want to use @widget
-again then you should delete @widget, because there will no longer be any parent 
-container to delete it automatically.
-</description>
-</function>
-
 <function name="gtk_page_setup_to_key_file">
 <parameters>
 <parameter name="setup">
diff --git a/gtk/src/scrolledwindow.ccg b/gtk/src/scrolledwindow.ccg
index 5ded54a..53fec4a 100644
--- a/gtk/src/scrolledwindow.ccg
+++ b/gtk/src/scrolledwindow.ccg
@@ -17,11 +17,9 @@
  */
 
 #include <gtkmm/scrollbar.h>
-#include <gtkmm/viewport.h>
 #include <gtkmm/adjustment.h>
 #include <gtk/gtk.h>
 
-
 namespace Gtk
 {
 
@@ -33,44 +31,13 @@ void ScrolledWindow::add(Gtk::Widget& widget)
   Bin::add(widget);
 }
 
+_DEPRECATE_IFDEF_START
 void ScrolledWindow::remove_with_viewport()
 {
-  auto child = gtk_bin_get_child(Bin::gobj());
-  if (child)
-  {
-    if (GTK_IS_VIEWPORT(child))
-    {
-      // Remove the Viewport's child, if any, from the Viewport.
-      auto grandchild = gtk_bin_get_child(GTK_BIN(child)); // A GtkViewport is a GtkBin
-      if (grandchild)
-      {
-        auto cppGrandchild = Glib::wrap(grandchild);
-
-        //If the grandchild is a managed widget, then do an extra ref so that it will not be
-        //destroyed when it is removed, and restore the floating state of the ref.
-        //This should leave it in the same state as when it was instantiated,
-        //before being added to the first container.
-        if (cppGrandchild->is_managed_())
-        {
-          cppGrandchild->reference();
-          g_object_force_floating(static_cast<Glib::Object*>(cppGrandchild)->gobj());
-        }
-
-        gtk_container_remove(GTK_CONTAINER(child), grandchild);
-      }
-
-      // Remove the Viewport.
-      // Don't do an extra ref on the child (the Viewport). If it's added
-      // by ScrolledWindow::add() or created as a managed widget,
-      // let it be deleted, when it's removed and the ref count reaches 0.
-      gtk_container_remove(Container::gobj(), child);
-    }
-    else
-    {
-      // The child is not a Viewport. Just remove it.
-      Bin::remove();
-    }
-  }
+  //We used to do what GTK+ now does for us:
+  //See https://bugzilla.gnome.org/show_bug.cgi?id=710471
+  remove();
 }
+_DEPRECATE_IFDEF_END
 
 } //namespace Gtk
diff --git a/gtk/src/scrolledwindow.hg b/gtk/src/scrolledwindow.hg
index 0488112..31be853 100644
--- a/gtk/src/scrolledwindow.hg
+++ b/gtk/src/scrolledwindow.hg
@@ -77,28 +77,23 @@ public:
   void add(Gtk::Widget& widget) override;
   _IGNORE(gtk_scrolled_window_add_with_viewport)
 
-  //TODO: When we can break ABI/API, make Bin::remove() virtual and delete remove_with_viewport().
-  //      Add a ScrolledWindow::remove() override, and let it do approximately
-  //      what remove_with_viewport() does. https://bugzilla.gnome.org/show_bug.cgi?id=685739#c2
-  /** Remove the contained object, possibly also a Gtk::Viewport.
-   * Since this can only hold one object it is not necessary to specify which
-   * object to remove, like containers that don't derive from Gtk::Bin.
+_DEPRECATE_IFDEF_START
+  /** Removes the contained widget, possibly also a Gtk::Viewport.
+   * Since this can only hold one widget it is not necessary to specify which
+   * widget to remove, like containers that don't derive from Gtk::Bin.
    *
    * This method is useful if you have added a widget without native scrolling
    * capability, and ScrolledWindow::add() has created a Gtk::Viewport.
    *
-   * If the child of the ScrolledWindow is a Gtk::Viewport, remove_with_viewport()
-   * removes the Viewport's child from the Viewport, and removes the Viewport from the
-   * ScrolledWindow. The Viewport is destroyed in two cases.
-   * <OL>
-   * <LI>If it was created by ScrolledWindow::add().</LI>
-   * <LI>If it was created as a managed widget
-   *     (<code>Gtk::Viewport* vp = Gtk::manage(new Gtk::Viewport(hadj, vadj));</code>).</LI>
-   * </OL>
+   * If the child of the ScrolledWindow is a Gtk::Viewport that was created by add(),
+   * remove_with_viewport() removes the Viewport's child from the Viewport,
+   * and removes the Viewport from the ScrolledWindow. The Viewport is destroyed.
    *
    * @newin{3,8}
+   * @deprecated Use Bin::remove() instead. It's now identical to remove_with_viewport().
    */
   void remove_with_viewport();
+_DEPRECATE_IFDEF_END
 
 #m4 _CONVERSION(`GtkWidget*',`Scrollbar*',`Glib::wrap((GtkScrollbar*)$3)')
 #m4 _CONVERSION(`GtkWidget*',`const Scrollbar*',`Glib::wrap((GtkScrollbar*)$3)')
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]