After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 69872 - GTK_WIDGET_SET_FLAGS should be deprecated
GTK_WIDGET_SET_FLAGS should be deprecated
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
2.20.x
Other other
: Normal blocker
: ---
Assigned To: gtk-bugs
gtk-bugs
: 554239 562937 (view as bug list)
Depends on: 593671 614510 614512 614513 614515
Blocks: 562937 586476 586731 590877 594053 594585 594716 595425 595496 597610 597888 604047 606883 612477
 
 
Reported: 2002-01-28 07:32 UTC by Matthias Clasen
Modified: 2011-02-04 16:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Replace app_paintable, sensitive, double_buffered, visible (8.97 KB, patch)
2008-06-20 15:16 UTC, Christian Dywan
none Details | Review
Implement is_realized, mapped, sensitive, default, parent_sensitive, toplevel (4.15 KB, patch)
2008-06-27 16:52 UTC, Christian Dywan
none Details | Review
Implement {set,get}_can_default, has_window, can_focus (4.54 KB, patch)
2008-06-27 17:30 UTC, Christian Dywan
committed Details | Review
Implement has_focus, has_grab (2.23 KB, patch)
2008-07-04 10:52 UTC, Christian Dywan
none Details | Review
Implement has_focus, has_grab, fixed (2.23 KB, patch)
2008-07-04 11:40 UTC, Christian Dywan
none Details | Review
Implement get_composite_child (1.71 KB, patch)
2008-07-04 11:55 UTC, Christian Dywan
none Details | Review
Implement has_focus, has_grab, better docs (2.32 KB, patch)
2008-08-15 10:36 UTC, Christian Dywan
committed Details | Review
WIP: Deprecate widget flags and don't use them inside GTK+ (35.35 KB, patch)
2009-12-03 14:10 UTC, Christian Dywan
none Details | Review
Deprecate some widget flags (46.69 KB, patch)
2009-12-04 17:00 UTC, Christian Dywan
committed Details | Review
Deprecate more widget flags (19.05 KB, patch)
2010-01-04 02:45 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Deprecate widget flag: GTK_WIDGET_CAN_FOCUS (13.49 KB, patch)
2010-01-04 04:08 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Deprecate widget flag: GTK_WIDGET_DRAWABLE (44.01 KB, patch)
2010-01-08 01:58 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Deprecate widget flag: GTK_WIDGET_HAS_FOCUS (52.49 KB, patch)
2010-01-08 04:07 UTC, Javier Jardón (IRC: jjardon)
none Details | Review
Deprecate widget flag: GTK_WIDGET_IS_SENSITIVE (35.61 KB, patch)
2010-01-09 05:02 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Complete the deprecation of widget flag: GTK_WIDGET_NO_WINDOW (14.31 KB, patch)
2010-01-09 05:03 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Deprecate widget flag: GTK_WIDGET_SENSITIVE (13.89 KB, patch)
2010-01-09 05:33 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Deprecate widget flag: GTK_WIDGET_HAS_FOCUS.v2 (53.17 KB, patch)
2010-01-10 03:39 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Deprecate widget flag: GTK_WIDGET_VISIBLE (162.30 KB, patch)
2010-01-10 17:58 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Deprecate widget flag: GTK_WIDGET_PARENT_SENSITIVE (1004 bytes, patch)
2010-01-12 03:40 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Add gtk_widget_has_rc_style() and deprecate GTK_WIDGET_RC_STYLE() (5.04 KB, patch)
2010-01-18 16:17 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Elaborate on what to use instead of GTK_WIDGET_FLAGS (536 bytes, patch)
2010-02-26 20:57 UTC, Christian Dywan
none Details | Review
Deprecate widget flag: GTK_WIDGET_MAPPED (60.88 KB, patch)
2010-03-02 04:27 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Deprecate widget flag: GTK_WIDGET_REALIZED (122.98 KB, patch)
2010-03-02 06:17 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Complete the deprecation of GTK_WIDGET_FLAGS (11.69 KB, patch)
2010-03-03 22:29 UTC, Javier Jardón (IRC: jjardon)
needs-work Details | Review
Don't use GTK_WIDGET_*SET_FLAGS (wid, GTK_TOPLEVEL) (2.24 KB, patch)
2010-03-06 12:26 UTC, Javier Jardón (IRC: jjardon)
needs-work Details | Review
Don't use GTK_WIDGET_*SET_FLAGS (wid, GTK_VISIBLE) (4.86 KB, patch)
2010-03-06 12:27 UTC, Javier Jardón (IRC: jjardon)
needs-work Details | Review
Don't use GTK_WIDGET_*SET_FLAGS (wid, GTK_HAS_GRAB) (1.10 KB, patch)
2010-03-06 12:28 UTC, Javier Jardón (IRC: jjardon)
needs-work Details | Review
Don't use GTK_WIDGET_*SET_FLAGS (wid, GTK_HAS_DEFAUL) (2.37 KB, patch)
2010-03-06 12:29 UTC, Javier Jardón (IRC: jjardon)
needs-work Details | Review
Don't use GTK_WIDGET_*SET_FLAGS (wid, GTK_HAS_FOCUS) (1.48 KB, patch)
2010-03-06 12:30 UTC, Javier Jardón (IRC: jjardon)
needs-work Details | Review
Complete the deprecation of GTK_WIDGET_FLAGS.v2 (5.62 KB, patch)
2010-03-06 12:37 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Complete the deprecation of widget flag: GTK_WIDGET_STATE (73.97 KB, patch)
2010-03-08 18:39 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Deprecate GTK_WIDGET_*SET_FLAGS (1.70 KB, patch)
2010-03-25 18:55 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Add gtk_widget_set_is_toplevel() (2.88 KB, patch)
2010-03-26 01:07 UTC, Javier Jardón (IRC: jjardon)
none Details | Review

Description Matthias Clasen 2002-01-28 07:32:05 UTC
...for application use and replaced by setters for the app-settable 
flags, so that the corresponding properties can have proper change
notification.
Comment 1 Tim Janik 2005-12-15 11:53:23 UTC
this is described in more detail in:

http://mail.gnome.org/archives/gtk-devel-list/2005-December/msg00136.html
Comment 2 Christian Dywan 2008-06-20 15:16:47 UTC
Created attachment 113120 [details] [review]
Replace app_paintable, sensitive, double_buffered, visible

This patch provides functions gtk_widget_get_foo for GTK_WIDGET_APP_PAINTABLE, GTK_WIDGET_DOUBLE_BUFFERED, GTK_WIDGET_SENSITIVE and GTK_WIDGET_VISIBLE.
Plus a property "double-buffered" is implemented.
Comment 3 Christian Dywan 2008-06-27 16:52:38 UTC
Created attachment 113539 [details] [review]
Implement is_realized, mapped, sensitive, default, parent_sensitive, toplevel

This patch implements the following accessor functions, all readonly, no properties.

> GTK_WIDGET_REALIZED
gtk_widget_is_realized
> GTK_WIDGET_MAPPED
gtk_widget_is_mapped
> GTK_WIDGET_IS_SENSITIVE
gtk_widget_is_sensitive
> GTK_WIDGET_HAS_DEFAULT
gtk_widget_is_default
> GTK_WIDGET_PARENT_SENSITIVE
gtk_widget_is_parent_sensitive
> GTK_WIDGET_TOPLEVEL
gtk_widget_is_toplevel
Comment 4 Christian Dywan 2008-06-27 17:30:41 UTC
Created attachment 113542 [details] [review]
Implement {set,get}_can_default, has_window, can_focus

This patch implements the following accessor functions, with getter and setter, no properties.

> GTK_WIDGET_CAN_DEFAULT
gtk_widget_{get,set}_can_default
> GTK_WIDGET_NO_WINDOW
gtk_widget_{get,set}_has_window
> GTK_WIDGET_CAN_FOCUS
gtk_widget_{get,set}_can_focus
Comment 5 Christian Dywan 2008-07-04 10:52:33 UTC
Created attachment 113973 [details] [review]
Implement has_focus, has_grab

This patch implements the following accessor functions, all readonly, no properties.

> GTK_WIDGET_HAS_FOCUS
gtk_widget_has_focus
> GTK_WIDGET_HAS_GRAB
gtk_widget_has_grab
Comment 6 Christian Dywan 2008-07-04 11:40:18 UTC
Created attachment 113977 [details] [review]
Implement has_focus, has_grab, fixed

(Updated patch, there was a nasty mistake in the code)

This patch implements the following accessor functions, all readonly, no
properties.

> GTK_WIDGET_HAS_FOCUS
gtk_widget_has_focus
> GTK_WIDGET_HAS_GRAB
gtk_widget_has_grab
Comment 7 Christian Dywan 2008-07-04 11:55:45 UTC
Created attachment 113980 [details] [review]
Implement get_composite_child

This patch implements the following accessor function, readonly, no property.

> GTK_WIDGET_COMPOSITE_CHILD
gtk_widget_get_composite_child
Comment 8 Tim Janik 2008-07-04 12:02:20 UTC
Comment on attachment 113977 [details] [review]
Implement has_focus, has_grab, fixed

Thanks Christian.

>Index: gtk/gtkwidget.c
>===================================================================
>--- gtk/gtkwidget.c	(Revision 20758)
>+++ gtk/gtkwidget.c	(Arbeitskopie)
>@@ -9888,6 +9888,43 @@ gtk_widget_get_has_tooltip (GtkWidget *w
> }
> 
> /**
>+ * gtk_widget_has_focus:
>+ * @widget: a #GtkWidget
>+ *
>+ * Determines whether the widget was the last one to recently grab focus.

Determines whether the widget has focus, that is, receives keyboard events for the window it is cnotained in.

>+ *
>+ * Return value: %TRUE if widget was the last to grab focus
>+ *
>+ * Since: 2.14
>+ */
>+gboolean
>+gtk_widget_has_focus (GtkWidget *widget)
>+{
>+  g_return_val_if_fail (GTK_IS_WIDGET (widget), FALSE);
>+
>+  return (GTK_WIDGET_FLAGS (widget) & GTK_HAS_FOCUS) != 0;
>+}
>+
>+/**
>+ * gtk_widget_has_grab:
>+ * @widget: a #GtkWidget
>+ *
>+ * Determines whether the widget is in the grab_widgets stack and
>+ * will be the preferred one for receiving events.

Determines whether the widget is currently grabbing events, so it is the only widget receiving input events (keyboard and mouse).
See also gtk_grab_add().

>+ *
>+ * Return value: %TRUE if the widget is in the grab_widgets stack
>+ *
>+ * Since: 2.14
>+ */
>+gboolean
>+gtk_widget_has_grab (GtkWidget *widget)
>+{
>+  g_return_val_if_fail (GTK_IS_WIDGET (widget), FALSE);
>+
>+  return (GTK_WIDGET_FLAGS (widget) & GTK_HAS_GRAB) != 0;
>+}
>+
>+/**
>  * gtk_widget_get_allocation:
>  * @widget: a #GtkWidget
>  *
>Index: gtk/gtkwidget.h
>===================================================================
>--- gtk/gtkwidget.h	(Revision 20758)
>+++ gtk/gtkwidget.h	(Arbeitskopie)
>@@ -572,6 +572,8 @@ void                  gtk_widget_set_par
> void                  gtk_widget_set_child_visible      (GtkWidget    *widget,
> 							 gboolean      is_visible);
> gboolean              gtk_widget_get_child_visible      (GtkWidget    *widget);
>+gboolean              gtk_widget_has_focus              (GtkWidget    *widget);
>+gboolean              gtk_widget_has_grab               (GtkWidget    *widget);
> GtkAllocation	      gtk_widget_get_allocation         (GtkWidget    *widget);
> GdkWindow*            gtk_widget_get_window             (GtkWidget    *widget);

Hm, this header file patch should deprecate the corresponding macros GTK_WIDGET_HAS_GRAB and GTK_WIDGET_HAS_FOCUS (but still allow them for GTK_COMPILATION).
Comment 9 Richard Hult 2008-07-18 09:59:05 UTC
I wonder if get_composite_child should be get_is_composite_child? Doesn't map directly to the property name but otherwise it sounds like it will return a composite child, not whether it is one.

Comment 10 Christian Dywan 2008-08-15 10:36:27 UTC
Comment on attachment 113977 [details] [review]
Implement has_focus, has_grab, fixed


I improved the documentation as suggested, thank you for reviewing.

>Index: gtk/gtkwidget.h
>===================================================================
>--- gtk/gtkwidget.h	(Revision 20758)
>+++ gtk/gtkwidget.h	(Arbeitskopie)
>@@ -572,6 +572,8 @@ void                  gtk_widget_set_par
> void                  gtk_widget_set_child_visible      (GtkWidget    *widget,
> 							 gboolean      is_visible);
> gboolean              gtk_widget_get_child_visible      (GtkWidget    *widget);
>+gboolean              gtk_widget_has_focus              (GtkWidget    *widget);
>+gboolean              gtk_widget_has_grab               (GtkWidget    *widget);
> GtkAllocation	      gtk_widget_get_allocation         (GtkWidget    *widget);
> GdkWindow*            gtk_widget_get_window             (GtkWidget   *widget);

> Hm, this header file patch should deprecate the corresponding macros
> GTK_WIDGET_HAS_GRAB and GTK_WIDGET_HAS_FOCUS (but still allow them for
> GTK_COMPILATION).

The first patch actually deprecates the macros. However going through with that consequently would have meant I need to patch half of gtk. And I figured that wouldn't make reivewing easier.
I think instead of special casing for inside Gtk it would be much nicer if we could actually replace the macros everywhere. Of course, that would be the very last step after names are clear and review is done.
Comment 11 Christian Dywan 2008-08-15 10:44:27 UTC
(In reply to comment #9)
> I wonder if get_composite_child should be get_is_composite_child? Doesn't map
> directly to the property name but otherwise it sounds like it will return a
> composite child, not whether it is one.

I haven't made up my mind. However *if* we choose _get_is_composite_child I'd prefer to call the property "is-composite-child" as well since in my point of view the conflict is the very same there.
Comment 12 Tim Janik 2008-08-15 11:03:38 UTC
(In reply to comment #11)
> (In reply to comment #9)
> > I wonder if get_composite_child should be get_is_composite_child? Doesn't map
> > directly to the property name but otherwise it sounds like it will return a
> > composite child, not whether it is one.
> 
> I haven't made up my mind. However *if* we choose _get_is_composite_child I'd
> prefer to call the property "is-composite-child" as well since in my point of
> view the conflict is the very same there.

The implementation (specification) for what exactly "composite child" means was never completed/finished. I think it's best to simply deprecate this, i.e. recommend not actively using it for anything.
For 3.0, we might want to "reuse" the flag to generate warnings if people poke into composite widget internals (e.g. with bin_get_child or container_get_children).
Comment 13 Colin Walters 2008-09-29 19:09:21 UTC
*** Bug 554239 has been marked as a duplicate of this bug. ***
Comment 14 Sven Herzberg 2009-03-26 12:55:06 UTC
Shouldn't each of the patches do all these things to a flag?

1. Provide getter/setter
2. Deprecate old macro
3. Change old macro to call the new function

I'm especially wondering about #3 here.
Comment 15 Christian Dywan 2009-05-08 14:48:17 UTC
See comment number 10, I wanted to make review easier and get name suggestions out of the way before completing 2 and 3.

If we agree on the proposed replacements I will update the patches.
Comment 16 Christian Dywan 2009-07-17 15:27:48 UTC
Ping.
Comment 17 Michael Natterer 2009-07-17 19:22:43 UTC
Committed a modified version of this patch
(commit 74ca4e24827619428a375904312dc8ba39e8d4a8)
Comment 18 Michael Natterer 2009-07-17 19:54:45 UTC
Committed parts of the first patch:
get_app_paintable(), get_double_buffered() and the "double-bufferd" property
(commit 40408e74dde4615f56e8befdf4a8e71451eff0f0)
Comment 19 Michael Natterer 2009-08-07 06:57:26 UTC
Commited accessors for "visible", partly taken from the first patch.
(commit eb0a5721d96bc937daeb742c9c216549a9456eba)
Comment 20 Michael Natterer 2009-08-09 23:35:57 UTC
All accessors from the last attachment have been committed.
Comment 21 Michael Natterer 2009-08-28 14:29:17 UTC
Added accessors gtk_widget_is_toplevel() and gtk_widget_is_drawable().
Comment 22 Christian Dywan 2009-08-31 13:05:14 UTC
*** Bug 593671 has been marked as a duplicate of this bug. ***
Comment 23 Michael Natterer 2009-09-04 13:20:00 UTC
Add API for RECEIVES_DEFAULT:

commit b81079d898050dec2211abc96a0dcad332a7cb59
Author: Michael Natterer <mitch@gimp.org>
Date:   Fri Sep 4 14:50:35 2009 +0200

    Bug 69872 - GTK_WIDGET_SET_FLAGS should be deprecated
    
    Add gtk_widget_set_receives_default() and
    gtk_widget_get_receives_default() as accessors for
    GTK_RECEIVES_DEFAULT.

 gtk/gtk.symbols |    2 ++
 gtk/gtkwidget.c |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gtk/gtkwidget.h |    4 ++++
 3 files changed, 60 insertions(+), 0 deletions(-)
Comment 24 Jean Bréfort 2009-10-09 11:25:38 UTC
We also need a write accessor to HAS_FOCUS for used in widget code. gtk_widget_has_focus makes it readable only. It needs to be writeable as well.
Comment 25 Javier Jardón (IRC: jjardon) 2009-10-09 11:45:19 UTC
Hello Jean, from a Michael Natterer comment here: https://bugzilla.gnome.org/show_bug.cgi?id=593601#c4

GTK_WIDGET_SET_FLAGS (widget, GTK_HAS_FOCUS);
-> you should never need to use this
Comment 26 Jean Bréfort 2009-10-09 15:32:55 UTC
Seems some widgets do that:

gtk/gtktreeview.c:10200:    GTK_WIDGET_SET_FLAGS (widget, GTK_HAS_FOCUS);
gtk/gtkwindow.c:5246:    GTK_WIDGET_SET_FLAGS (widget, GTK_HAS_FOCUS);
Comment 27 Emmanuele Bassi (:ebassi) 2009-10-12 14:06:34 UTC
(In reply to comment #26)
> Seems some widgets do that:
> 
> gtk/gtktreeview.c:10200:    GTK_WIDGET_SET_FLAGS (widget, GTK_HAS_FOCUS);
> gtk/gtkwindow.c:5246:    GTK_WIDGET_SET_FLAGS (widget, GTK_HAS_FOCUS);

the only legal instance of that is the GtkWindow code; the GtkTreeView needs that for the search entry focusing, but that's evil and we should find a way to have public API for doing this correctly - also for applications using the same search entry semantics on TextView widgets (e.g. gedit).
Comment 28 Javier Jardón (IRC: jjardon) 2009-10-12 15:06:55 UTC
(In reply to comment #27)
> the GtkTreeView needs that for the search entry focusing, but that's evil and we should find a way to have public API for doing this correctly - also for applications using the same search entry semantics on TextView widgets (e.g. gedit).

Reopening bug #593671 after the comment #27 by Emmanuele Bassi
Comment 29 Christian Dywan 2009-12-03 14:10:50 UTC
Created attachment 149010 [details] [review]
WIP: Deprecate widget flags and don't use them inside GTK+

This is an unfinished patch deprecating object flags and not using them anymore inside GTK+. I will attach a more complete patch later.
Comment 30 Christian Dywan 2009-12-04 17:00:57 UTC
Created attachment 149099 [details] [review]
Deprecate some widget flags

Deprecate the following:

GTK_OBJECT_TYPE
GTK_OBJECT_TYPE_NAME
GTK_WIDGET_TYPE
GTK_WIDGET_STATE
GTK_WIDGET_SAVED_STATE
GTK_WIDGET_FLAGS
GTK_WIDGET_TOPLEVEL
GTK_WIDGET_NO_WINDOW
GTK_DISABLE_DEPRECATED

I did not remove all uses of GTK_WIDGET_NO_WINDOW, GTK_WIDGET_FLAGS and GTK_WIDGET_STATE because that makes the diff extremely long. Maybe it's better to do that in steps.
Comment 31 Matthias Clasen 2010-01-03 02:27:47 UTC
I don't think reviewing this in detail is feasible or necessary. Just commit it. I'll notice that patching gtkclist.c is not really the best use of resources, since it is deprecated and on the way out anyway.
Comment 32 André Klapper 2010-01-03 11:30:56 UTC
(When committing please update the "* Since: 2.14" values")
Comment 33 Javier Jardón (IRC: jjardon) 2010-01-04 02:45:44 UTC
Created attachment 150765 [details] [review]
Deprecate more widget flags

Deprecate the following:

GTK_WIDGET_APP_PAINTABLE
GTK_WIDGET_CAN_DEFAULT
GTK_WIDGET_DOUBLE_BUFFERED
GTK_WIDGET_HAS_DEFAULT
GTK_WIDGET_HAS_GRAB
GTK_WIDGET_RECEIVES_DEFAULT
Comment 34 Javier Jardón (IRC: jjardon) 2010-01-04 04:08:14 UTC
Created attachment 150767 [details] [review]
Deprecate widget flag: GTK_WIDGET_CAN_FOCUS
Comment 35 Christian Dywan 2010-01-04 07:43:48 UTC
I pushed my commit for "Deprecate some widget flags" and I pushed the update of GtkContainer, GtkEventBox and GtkFixed, including deprecating gtk_fixed_get/set_has_window.
Comment 36 Javier Jardón (IRC: jjardon) 2010-01-07 09:14:57 UTC
Comment on attachment 150765 [details] [review]
Deprecate more widget flags

commit 51e0dd9a82270ebfe8d4f5da0b3674c3c9ad0d57
Comment 37 Javier Jardón (IRC: jjardon) 2010-01-07 09:15:30 UTC
Comment on attachment 150767 [details] [review]
Deprecate widget flag: GTK_WIDGET_CAN_FOCUS

commit 56a893ca8cafc4b4d5cc32018795d8eda3fa1dbc
Comment 38 Javier Jardón (IRC: jjardon) 2010-01-08 01:58:56 UTC
Created attachment 151013 [details] [review]
Deprecate widget flag: GTK_WIDGET_DRAWABLE
Comment 39 Javier Jardón (IRC: jjardon) 2010-01-08 04:07:44 UTC
Created attachment 151020 [details] [review]
Deprecate widget flag: GTK_WIDGET_HAS_FOCUS
Comment 40 Javier Jardón (IRC: jjardon) 2010-01-09 05:02:39 UTC
Created attachment 151081 [details] [review]
Deprecate widget flag: GTK_WIDGET_IS_SENSITIVE
Comment 41 Javier Jardón (IRC: jjardon) 2010-01-09 05:03:52 UTC
Created attachment 151082 [details] [review]
Complete the deprecation of widget flag: GTK_WIDGET_NO_WINDOW
Comment 42 Javier Jardón (IRC: jjardon) 2010-01-09 05:33:09 UTC
Created attachment 151083 [details] [review]
Deprecate widget flag: GTK_WIDGET_SENSITIVE
Comment 43 Javier Jardón (IRC: jjardon) 2010-01-10 03:39:55 UTC
Created attachment 151109 [details] [review]
Deprecate widget flag: GTK_WIDGET_HAS_FOCUS.v2

Forgot to deprecate GTK_WIDGET_HAS_FOCUS in gtkwidget.h, here a new patch
Comment 44 Javier Jardón (IRC: jjardon) 2010-01-10 17:58:04 UTC
Created attachment 151134 [details] [review]
Deprecate widget flag: GTK_WIDGET_VISIBLE
Comment 45 Javier Jardón (IRC: jjardon) 2010-01-12 03:40:20 UTC
Created attachment 151218 [details] [review]
Deprecate widget flag: GTK_WIDGET_PARENT_SENSITIVE

Still remaining: GTK_WIDGET_RC_STYLE(), GTK_WIDGET_MAPPED() and GTK_WIDGET_REALIZED()
Comment 46 Javier Jardón (IRC: jjardon) 2010-01-18 16:17:58 UTC
Created attachment 151679 [details] [review]
Add gtk_widget_has_rc_style() and deprecate GTK_WIDGET_RC_STYLE()
Comment 47 Javier Jardón (IRC: jjardon) 2010-01-18 23:23:52 UTC
Comment on attachment 151679 [details] [review]
Add gtk_widget_has_rc_style() and deprecate GTK_WIDGET_RC_STYLE()

commit 6b808a7389689b3e6b89475c5a30cced2885bb1d
Comment 48 Javier Jardón (IRC: jjardon) 2010-02-03 01:35:24 UTC
*** Bug 562937 has been marked as a duplicate of this bug. ***
Comment 49 Stefan Sauer (gstreamer, gtkdoc dev) 2010-02-10 16:10:08 UTC
compiling my own code with "-DGSEAL_ENABLE" right now fails as my own widgets do

static void
abc_xyz_realize (GtkWidget *widget)
{
  AbcXyz *self = ABC_XYZ(widget);
  ...
  
  GTK_WIDGET_SET_FLAGS (widget, GTK_REALIZED | GTK_CAN_FOCUS);
  ...

this is the same code that I can also see in gtkbutton.c as of today (> 2.19.5). I can see what api I should use instead as there is no gtk_widget_set_flags().
Comment 50 Javier Jardón (IRC: jjardon) 2010-02-10 16:15:22 UTC
(In reply to comment #49)
 
>   GTK_WIDGET_SET_FLAGS (widget, GTK_REALIZED | GTK_CAN_FOCUS);

gtk_widget_set_can_focus (widget, TRUE);
gtk_widget_set_realized (widget, TRUE);
Comment 51 Stefan Sauer (gstreamer, gtkdoc dev) 2010-02-11 09:41:13 UTC
(In reply to comment #50)
> (In reply to comment #49)
> 
> >   GTK_WIDGET_SET_FLAGS (widget, GTK_REALIZED | GTK_CAN_FOCUS);
> 
> gtk_widget_set_can_focus (widget, TRUE);
> gtk_widget_set_realized (widget, TRUE);

okay, that was just added. But gtkbutton.c still uses
 GTK_WIDGET_SET_FLAGS (widget, GTK_REALIZED);

Do we change the api into fast api for internal widgets and cumbersome to use and slow api for external widgets? Can't we just keep fundamental stuff like FLAGS in the public struct. There is no hard requirement to seal *everything*.
Comment 52 Christian Dywan 2010-02-26 20:12:51 UTC
(In reply to comment #51)
> (In reply to comment #50)
> > (In reply to comment #49)
> > 
> > >   GTK_WIDGET_SET_FLAGS (widget, GTK_REALIZED | GTK_CAN_FOCUS);
> > 
> > gtk_widget_set_can_focus (widget, TRUE);
> > gtk_widget_set_realized (widget, TRUE);
> 
> okay, that was just added. But gtkbutton.c still uses
>  GTK_WIDGET_SET_FLAGS (widget, GTK_REALIZED);
> 
> Do we change the api into fast api for internal widgets and cumbersome to use
> and slow api for external widgets? Can't we just keep fundamental stuff like
> FLAGS in the public struct. There is no hard requirement to seal *everything*.

There is nothing cumbersome about the new functions. They are cleaner, since you have one function for each aspect and type safe and they work just like other setters we have. And apart from being an actual function call they are not slower.
For what I want, I think right now finishing the public API is more important than cleaning up internal uses, which we can still do later.
Comment 53 Stefan Sauer (gstreamer, gtkdoc dev) 2010-02-26 20:19:20 UTC
Okay, if the long run goal is to also use the functional internaly its better.

Still the functions are definitely slower as the macros:
- the do a gobject cast cehck
- you can only set one flag at a time.

But I can understand the proc and cons. I made a patch that removes some of the double calls and that is in, so I am fine.
Comment 54 Christian Dywan 2010-02-26 20:21:02 UTC
Review of attachment 151218 [details] [review]:

I think the Deprecated: line should say "Use gtk_widget_get_sensitive on the parent widget instead."
Comment 55 Christian Dywan 2010-02-26 20:28:40 UTC
Review of attachment 151082 [details] [review]:

Maybe we want to use the flag states directly inside gtkwidget.c, just like we continue to use GtkWidget struct members directly.
Comment 56 Christian Dywan 2010-02-26 20:30:52 UTC
Review of attachment 151083 [details] [review]:

Looks good to me.
Comment 57 Christian Dywan 2010-02-26 20:38:01 UTC
Comment on attachment 151081 [details] [review]
Deprecate widget flag: GTK_WIDGET_IS_SENSITIVE

Looks good to me, again, using flags directly in gtkwidget.c might be a good idea.
Comment 58 Christian Dywan 2010-02-26 20:41:26 UTC
Review of attachment 151109 [details] [review]:

Looks correct, again consider flags in gtkwidget.c.
Comment 59 Christian Dywan 2010-02-26 20:57:02 UTC
Created attachment 154796 [details] [review]
Elaborate on what to use instead of GTK_WIDGET_FLAGS
Comment 60 Murray Cumming 2010-02-26 22:12:12 UTC
Review of attachment 154796 [details] [review]:

Excellent. Thank you.
Comment 61 Javier Jardón (IRC: jjardon) 2010-02-27 05:12:28 UTC
(In reply to comment #55)
> Review of attachment 151082 [details] [review]:
> 
> Maybe we want to use the flag states directly inside gtkwidget.c, just like we
> continue to use GtkWidget struct members directly.

But you can't use it because It's deprecated code.
For example, if GTK_WIDGET_VISIBLE() is deprecated, It'll be removed from the source code, so you can't use it.

Or I'm missing something?
Comment 62 Christian Dywan 2010-02-27 06:50:07 UTC
(In reply to comment #61)
> (In reply to comment #55)
> > Review of attachment 151082 [details] [review] [details]:
> > 
> > Maybe we want to use the flag states directly inside gtkwidget.c, just like we
> > continue to use GtkWidget struct members directly.
> 
> But you can't use it because It's deprecated code.
> For example, if GTK_WIDGET_VISIBLE() is deprecated, It'll be removed from the
> source code, so you can't use it.

Yes. What I meant is, we might use widget->flags which is a member of GtkWidget.
Comment 63 Javier Jardón (IRC: jjardon) 2010-02-27 17:57:53 UTC
So in gtkwidget.c we should substitute:

GTK_WIDGET_VISIBLE -> ((GTK_OBJECT_FLAGS (wid) & GTK_VISIBLE) != 0) ?

I think is clearer

GTK_WIDGET_VISIBLE -> gtk_widget_get_visible()

Or maybe you are propossins something different?
Comment 64 Stefan Sauer (gstreamer, gtkdoc dev) 2010-02-27 19:29:02 UTC
(In reply to comment #63)
> So in gtkwidget.c we should substitute:
> 
> GTK_WIDGET_VISIBLE -> ((GTK_OBJECT_FLAGS (wid) & GTK_VISIBLE) != 0) ?
> 
> I think is clearer
> 
> GTK_WIDGET_VISIBLE -> gtk_widget_get_visible()
> 
> Or maybe you are propossins something different?

When the deprecated macros are removed in 3.0 you could move them from the header to the widget.c file.
Comment 65 Javier Jardón (IRC: jjardon) 2010-03-01 04:25:28 UTC
Comment on attachment 151013 [details] [review]
Deprecate widget flag: GTK_WIDGET_DRAWABLE

commit 4f78f70b15f861cca115f578b0f3f4ad3ad087e7
Comment 66 Javier Jardón (IRC: jjardon) 2010-03-01 04:33:19 UTC
Comment on attachment 151081 [details] [review]
Deprecate widget flag: GTK_WIDGET_IS_SENSITIVE

commit a27d5a2c9eba7af5b056de32ff9b2b4dd1eb97e1
Comment 67 Javier Jardón (IRC: jjardon) 2010-03-01 04:44:44 UTC
Comment on attachment 151082 [details] [review]
Complete the deprecation of widget flag: GTK_WIDGET_NO_WINDOW

commit f5bde06e82334995fca90ac6f103c8b2c7aa878a
Comment 68 Javier Jardón (IRC: jjardon) 2010-03-01 05:00:06 UTC
Comment on attachment 151083 [details] [review]
Deprecate widget flag: GTK_WIDGET_SENSITIVE

commit 64f526d34e1adc609944078e2fea38b2792f8230
Comment 69 Javier Jardón (IRC: jjardon) 2010-03-01 05:11:35 UTC
Comment on attachment 151109 [details] [review]
Deprecate widget flag: GTK_WIDGET_HAS_FOCUS.v2

commit 4232115e22c8ea41d6a3faf89bcaadfc9933d5e3
Comment 70 Javier Jardón (IRC: jjardon) 2010-03-01 06:53:38 UTC
Comment on attachment 151134 [details] [review]
Deprecate widget flag: GTK_WIDGET_VISIBLE

committed an improved version in commit  214a023e9160fa008db069f5ec10ce3a4999a4dc
Comment 71 Javier Jardón (IRC: jjardon) 2010-03-01 06:58:16 UTC
Comment on attachment 151218 [details] [review]
Deprecate widget flag: GTK_WIDGET_PARENT_SENSITIVE

commit b4b95d07f8b012cf29c0546f425ea3b78f2ad502
Comment 72 Javier Jardón (IRC: jjardon) 2010-03-02 04:27:04 UTC
Created attachment 155000 [details] [review]
Deprecate widget flag: GTK_WIDGET_MAPPED
Comment 73 Javier Jardón (IRC: jjardon) 2010-03-02 06:17:08 UTC
Created attachment 155004 [details] [review]
Deprecate widget flag: GTK_WIDGET_REALIZED
Comment 74 Christian Dywan 2010-03-03 10:02:13 UTC
Review of attachment 155000 [details] [review]:

Looks fine.
Comment 75 Christian Dywan 2010-03-03 10:17:30 UTC
Review of attachment 155004 [details] [review]:

Oh my, you improved a corner case in a return_if_fail in old editable. You don't really need to improve cruft :-)

Looks good.
Comment 76 Javier Jardón (IRC: jjardon) 2010-03-03 19:42:53 UTC
Comment on attachment 155000 [details] [review]
Deprecate widget flag: GTK_WIDGET_MAPPED

commit 1fe7d3cefd1aeabd6d8cab2a68673bb1f7876988
Comment 77 Javier Jardón (IRC: jjardon) 2010-03-03 19:43:27 UTC
Comment on attachment 155004 [details] [review]
Deprecate widget flag: GTK_WIDGET_REALIZED

commit 16a59ad9129c680833c584768e6a81b2ba365268
Comment 78 Javier Jardón (IRC: jjardon) 2010-03-03 22:29:39 UTC
Created attachment 155169 [details] [review]
Complete the deprecation of GTK_WIDGET_FLAGS
Comment 79 Christian Dywan 2010-03-05 17:38:42 UTC
Comment on attachment 155169 [details] [review]
Complete the deprecation of GTK_WIDGET_FLAGS

Changing all current macros in gtkwidget.h to use GTK_OBJECT_FLAGS seems unnecessary. Those are deprecated anyway. I'd rather leave those as they are.

Also, GTK_WIDGET_SET_FLAGS and GTK_WIDGET_UNSET_FLAGS are still left. Those need to be deprecated if this is supposed to be the final patch.
Comment 80 Javier Jardón (IRC: jjardon) 2010-03-06 12:26:42 UTC
Created attachment 155404 [details] [review]
Don't use GTK_WIDGET_*SET_FLAGS (wid, GTK_TOPLEVEL)
Comment 81 Javier Jardón (IRC: jjardon) 2010-03-06 12:27:22 UTC
Created attachment 155405 [details] [review]
Don't use GTK_WIDGET_*SET_FLAGS (wid, GTK_VISIBLE)
Comment 82 Javier Jardón (IRC: jjardon) 2010-03-06 12:28:26 UTC
Created attachment 155406 [details] [review]
Don't use GTK_WIDGET_*SET_FLAGS (wid, GTK_HAS_GRAB)
Comment 83 Javier Jardón (IRC: jjardon) 2010-03-06 12:29:21 UTC
Created attachment 155407 [details] [review]
Don't use GTK_WIDGET_*SET_FLAGS (wid, GTK_HAS_DEFAUL)
Comment 84 Javier Jardón (IRC: jjardon) 2010-03-06 12:30:00 UTC
Created attachment 155408 [details] [review]
Don't use GTK_WIDGET_*SET_FLAGS (wid, GTK_HAS_FOCUS)
Comment 85 Javier Jardón (IRC: jjardon) 2010-03-06 12:37:34 UTC
Created attachment 155409 [details] [review]
Complete the deprecation of GTK_WIDGET_FLAGS.v2

With this and the previous patches, I think we can close this bug :)
Comment 86 Christian Dywan 2010-03-08 18:38:43 UTC
Review of attachment 155404 [details] [review]:

We need a gtk_widget_set_is_toplevel it seems. It must be possible to write toplevel widgets with public API.
Comment 87 Javier Jardón (IRC: jjardon) 2010-03-08 18:39:35 UTC
Created attachment 155575 [details] [review]
Complete the deprecation of widget flag: GTK_WIDGET_STATE
Comment 88 Christian Dywan 2010-03-08 18:40:59 UTC
Review of attachment 155405 [details] [review]:

You should use gtk_widget_set_visible here.
Comment 89 Christian Dywan 2010-03-08 18:46:09 UTC
Review of attachment 155406 [details] [review]:

I think we could go for an internal function like _gtk_widget_set_has_grab here. It probably isn't something that would/ should be used outside of GTK+.
Comment 90 Christian Dywan 2010-03-08 18:51:26 UTC
Review of attachment 155407 [details] [review]:

Hm... this one is ugly because gtk_widget_grab_default is not enough to set the flag and there's an awkward interdepedency on GtkWindow. I guess we need an internal function here as well unless somebody can think of something more elegant.
Comment 91 Christian Dywan 2010-03-08 18:56:02 UTC
Review of attachment 155408 [details] [review]:

You can use gtk_widget_grab_focus to set the focus. But I don't think we should unset it. Looking at the GtkWidget::focus property, it's not possible to unset there either (it's a no-op to set it to FALSE). I'm not sure what the intention of unsetting is, if there is no other focus widget...
Comment 92 Christian Dywan 2010-03-08 19:02:15 UTC
To make sure it won't be forgotten: need to check with Kris about unsetting focus in the treeview. Apparently Gimp also does it, for an unknown reason.
Comment 93 Christian Dywan 2010-03-08 19:07:43 UTC
Review of attachment 155575 [details] [review]:

Looks good to me.
Comment 94 Christian Dywan 2010-03-08 19:14:05 UTC
Review of attachment 155409 [details] [review]:

Looks good.
Comment 95 Javier Jardón (IRC: jjardon) 2010-03-09 01:58:50 UTC
Comment on attachment 155575 [details] [review]
Complete the deprecation of widget flag: GTK_WIDGET_STATE

commit 32b9aeaadd6dbd084344d97c573b0289c1584923
Comment 96 Javier Jardón (IRC: jjardon) 2010-03-12 05:50:25 UTC
Comment on attachment 155409 [details] [review]
Complete the deprecation of GTK_WIDGET_FLAGS.v2

commit 2c043d33c2271aaf8db2e216325c7e855f048e00
Comment 97 Javier Jardón (IRC: jjardon) 2010-03-25 18:55:59 UTC
Created attachment 157094 [details] [review]
Deprecate GTK_WIDGET_*SET_FLAGS

Some internal uses need to be fixed, but we can do it later
Comment 98 Javier Jardón (IRC: jjardon) 2010-03-26 01:07:46 UTC
Created attachment 157132 [details] [review]
Add gtk_widget_set_is_toplevel()
Comment 99 Tristan Van Berkom 2010-03-26 01:53:18 UTC
Ok might as well drop my 2 cents as a user of GTK_WIDGET_SET_FLAGS (..IS_TOPLEVEL)

Basically what we do black magic by setting the toplevel flag to FALSE on
a GtkWindow so we can reparent it; our usage does not justify exporting
this API, and I dont know what use case does, furthermore its a rather
internal thing widget authors shouldn't need to think about.

I would wait for a seriously good argument before exposing it.
Comment 100 Christian Dywan 2010-03-26 09:20:36 UTC
(In reply to comment #99)
> Basically what we do black magic by setting the toplevel flag to FALSE on
> a GtkWindow so we can reparent it; our usage does not justify exporting
> this API, and I dont know what use case does, furthermore its a rather
> internal thing widget authors shouldn't need to think about.

Who is we and what is the window reparented to?

The obvious use case is implementing a new toplevel widget class.
Comment 101 Tristan Van Berkom 2010-03-26 23:38:45 UTC
(In reply to comment #100)
> (In reply to comment #99)
> > Basically what we do black magic by setting the toplevel flag to FALSE on
> > a GtkWindow so we can reparent it; our usage does not justify exporting
> > this API, and I dont know what use case does, furthermore its a rather
> > internal thing widget authors shouldn't need to think about.
> 
> Who is we and what is the window reparented to?

We is Glade, and we do it for the workspace, sorry for not clarifying that.

> 
> The obvious use case is implementing a new toplevel widget class.

The obvious way to implement a toplevel window is to subclass GtkWindow
and have it do what GTK+ decides to do internally when creating a toplevel.

What is a use case of a toplevel widget that an application would want
to create that is not a GtkWindow (or GtkInvisible) ?

I think the api is much simplified by not exposing the internal flag GTK+
uses to define toplevel GTK+ widgets, and there is something to be gained
in terms of api reliability as well by not exposing this flag.

Well, on the other hand if you do chose to expose this symbol Glade will
happily run with GTK+ 3.0 and continue to embed the GtkWindows...
Comment 102 Christian Dywan 2010-03-27 00:24:11 UTC
(In reply to comment #101)
> Well, on the other hand if you do chose to expose this symbol Glade will
> happily run with GTK+ 3.0 and continue to embed the GtkWindows...

I guess you do that to pretend there is a toplevel? How about using GtkOffscreenWindow instead?
Comment 103 Tristan Van Berkom 2010-03-27 01:53:47 UTC
The problem is only that Glade is a little dumb about what runtime
widget to create for a registered widget class, Glade still wants to
instantiate the exact GType that it introspected properties for.

But sure, some alternative hack could be done, the right thing
to do would be to teach Glade to use an alternate type in the
workspace than the actual edit type (then we can address combo-box
glitchy selection etc)...
Comment 104 Javier Jardón (IRC: jjardon) 2010-03-31 21:16:20 UTC
Ok, this bug is became too big to track the progress; let's discuss the remaining accessor functions:

GTK_WIDGET_*SET_FLAGS (wid, GTK_TOPLEVEL)
GTK_WIDGET_*SET_FLAGS (wid, GTK_VISIBLE)
GTK_WIDGET_*SET_FLAGS (wid, GTK_HAS_GRAB)
GTK_WIDGET_*SET_FLAGS (wid, GTK_HAS_DEFAULT)
GTK_WIDGET_*SET_FLAGS (wid, GTK_HAS_FOCUS)

in separte bugs (see blockers list).
When all the issue were corrected, we can apply the patch of comment #97 and finally close this bug.
Comment 105 Javier Jardón (IRC: jjardon) 2010-04-27 00:48:55 UTC
Comment on attachment 157094 [details] [review]
Deprecate GTK_WIDGET_*SET_FLAGS

commit d3d4cf0e841df6573a41ef4ebb609ff7b81dbb0f
Comment 106 Javier Jardón (IRC: jjardon) 2010-04-27 00:51:38 UTC
GTK_WIDGET_*SET_FLAGS() is deprecated as public api now, so we can close this bug.

There is still one remaining use of GTK_WIDGET_*SET_FLAGS (wid, GTK_VISIBLE) in internal code, you can track the progress of this remaining case in bug #614512