Re: known bug still in 1.0.0.

[Owen Taylor <otaylor gtk org>]

Patrice Fortier <Patrice.Fortier@aquarel.fr> writes:

>   A couple of weeks ago I sent a patch (gtk-fortier-980405-0 or
> something like that) to correct a bug in the scrolled_window
> widget.
> 
>   The gtk_scrolled_window_foreach function is buggy. It only does
> one third of its job:
> calling this function only send the callback to the viewport widget,
> and not to the scrollbars.
> What's the need of a foreach function if you don't send the callback
> for each widget in this widget???

It is used to provide the default method of doing many things
for container widgets. I'm not sure if we want to make the
_default_ method required to be the only method.

For instance, one consequence of your patch is that if you
gtk_widget_set_sensitive(...,FALSE) a ScrolledWindow, you not only
make all its children insensitive, you also make the window
non-scrollable. That might be the correct behavior, but the 
current behavior is also somewhat useful.
 
> A quick example:
> 
> I want to unrealize my scrolled_window:

(Note, you are never supposed to unrealize a widget explicitely,
it would come up though, if you removed the scrolledwindow from its 
parent)

> 
> gtk_widget_unrealized is called and it executes this:
> 
>   if (GTK_IS_CONTAINER (widget))
>     gtk_container_foreach (GTK_CONTAINER (widget),
> 			   (GtkCallback)gtk_widget_unrealize,
> 			   NULL);
> 
> Now foreach does NOT send gtk_widget_unrealize to the scrollbars
> of the scrolled_window, so they're not unrealized :(.
 
True. This could alternatively be solved by simply creating an
unrealize() callback for scrolledwindow that unrealized its
two scrollbars then called the parent class unrealize().

The natural question is, what other default actions are affected:

 show/hide_all: 
   Not an issue - since hiding and showing the scrollbars is
   the responsibility of the scrolled window.

 gtk_widget_propagate state:
   It could be argued that the default action is not correct here,
   as mentioned above
  
 gtk_widget_draw_children:
   Don't really understand this function, but I don't think it
   is a problem.

 reparent:
   Now this is a major problem... The current effect if you reparent
   a scrolled window is rather amusing. The scrollbars stay behind
   (and continue scrolling the window at the new location.)

   To solve this without changing foreach(), you'd have to make
   ScrolledWindow not 

 gtk_container_children:
   Depends on what behavior you want. The current behavior means
   that you can never tab to the scrollbars on a ScrolledWindow,
   which some people might think a good thing.

The other widget that would be affected (CList already does
things the way your want them), is the Notebook widget. It
exhibits the unparenting problem, and also doesn't show
its tabs as disabled when it is made insensitive.

I think there are really three questions

 - Should the ScrolledWindow's foreach() do the scrollbars?
 
 - Should the Notebook's foreach do the tabs? (Only problem
   I see with this is that exisiting code may well be
   using gtk_container_foreach() to iterate over the pages
   of a notebook)

 - Should every container widget have to include foreach()
   in all of its children.

   Advantage: makes things simpler, less chance of hidden bugs.

   Disadvantage:
     If this is done, there will be _no_ way to do things like 
     a ScrolledWindow that allows its area to be scrolled when 
     not sensitive.

     Application developers loose a handy interface to interate
     over the widgets that _they_ have added to a container.

     (Imagine a CList with some cells containing embedded 
      widgets)

     Yes, this functionality could be separated out, but that
     would require either:

      - Specific functions on a widget-by-widget basis (bad
        when you consider the _full variant of foreach(), which
        is used by language bindings)

      - Modifiying the foreach signal
    
      - Creating a whole new signal "foreach_user".

So, not to make a mountain out of a molehill, but some more
explainaition why I didn't just go ahead and apply the patch.  Yes, it
does fix some (fairly rare) bugs, but it also raises some non-trivial
issues.

Regards,
                                        Owen