Re: window sizing patch

[Havoc Pennington <hp redhat com>]

Tim Janik <timj gtk org> writes:
> ok, i think i went through all of the relevant portions of this patch
> and only have a couple minor issues:
> 
> +void       gtk_widget_get_size_request    (GtkWidget           *widget,
> +                                           gint                *width,
> +                                           gint                *height);
> i thought we agreed on not having this function, since people
> looking for a widget's size will want allocation anyways and
> would wrongly figure get_usize/get_size_request (yeah, setter/
> getter symmetrie would be an argument, but a getter here doesn't
> really make much sense).

You're right, your notes and my notes both say we don't have this.
I probably just left it there by accident. ;-)

However we did go through and add that huge "getters" patch after our
discussion, adding getters for all setters on principle. Does that
change our decision?

I am pretty indifferent, just pointing out the possible policy change
on getters.

> +static void
> +gtk_window_finalize (GObject *object)
> +{
> +  GtkWindow *window;
> +
> +  g_return_if_fail (GTK_IS_WINDOW (object));
> +
> +  window = GTK_WINDOW (object);
> +
> +  toplevel_list = g_slist_remove (toplevel_list, window);
> +
> +  g_free (window->title);
> +  g_free (window->wmclass_name);
> +  g_free (window->wmclass_class);
> +  g_free (window->wm_role);
> +
> +  g_hash_table_foreach_remove (mnemonic_hash_table,
> +                              gtk_window_mnemonic_hash_remove,
> +                              window);
> +  if (window->geometry_info)
> +    {
> +      if (window->geometry_info->widget)
> +       gtk_signal_disconnect_by_func (GTK_OBJECT (window->geometry_info->widget),
> +                                      GTK_SIGNAL_FUNC (gtk_widget_destroyed),
> +                                      &window->geometry_info->widget);
> +      g_free (window->geometry_info);
> +    }
> +
> +  G_OBJECT_CLASS (parent_class)->finalize (object);
> +}
> 
> the disconnect doesn't look right to me, window->geometry_info->widget
> not being part of window's widget tree is somewhat pathological, if
> it is however, the widget should already be finalized and you can't
> disconnect at handler at that point.

I didn't change this code, it's just "diff" having a bad day - look at
how gtk_window_finalize() being -'d is interleaved with
gtk_window_move() being +'d, and then gtk_window_finalize() is
re-added again in its entirety. Go figure.

Anyhow, the reason it works is:

 gtk_signal_connect (GTK_OBJECT (geometry_widget), "destroy",
                        GTK_SIGNAL_FUNC (gtk_widget_destroyed),
                        &info->widget);

in gtk_window_set_geometry_hints(). I guess the finalize code only
catches the weird-pathology case you mention however.

> auto_shrink:
> we wanted to remove this, however the way i read your patch, you removed
> the property installation code in the class but still handle it in
> get/set?

Good catch, fixed.

> 
> PROP_WIDTH_REQUEST/PROP_HEIGHT_REQUEST:
> from what i see, you only handle these in the setter, not the
> getter though they are readwrite props.

Fixed.

OK, I'm going to do some testing since GTK has changed a bit since I
wrote this, then put what I have in CVS and let the bug finding begin.

Havoc