this is described in more detail in:

http://mail.gnome.org/archives/gtk-devel-list/2005-December/msg00136.html

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.

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

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

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

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

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 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).

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 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.

(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.

(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).

*** Bug 554239 has been marked as a duplicate of this bug. ***

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.

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.

Ping.

Committed a modified version of this patch
(commit 74ca4e24827619428a375904312dc8ba39e8d4a8)

Committed parts of the first patch:
get_app_paintable(), get_double_buffered() and the "double-bufferd" property
(commit 40408e74dde4615f56e8befdf4a8e71451eff0f0)

Commited accessors for "visible", partly taken from the first patch.
(commit eb0a5721d96bc937daeb742c9c216549a9456eba)

All accessors from the last attachment have been committed.

Added accessors gtk_widget_is_toplevel() and gtk_widget_is_drawable().

*** Bug 593671 has been marked as a duplicate of this bug. ***

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(-)

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.

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

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);

(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).

(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

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.

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.

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.

(When committing please update the "* Since: 2.14" values")

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

Created attachment 150767 [details] [review]
Deprecate widget flag: GTK_WIDGET_CAN_FOCUS

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 on attachment 150765 [details] [review]
Deprecate more widget flags

commit 51e0dd9a82270ebfe8d4f5da0b3674c3c9ad0d57

Comment on attachment 150767 [details] [review]
Deprecate widget flag: GTK_WIDGET_CAN_FOCUS

commit 56a893ca8cafc4b4d5cc32018795d8eda3fa1dbc

Created attachment 151013 [details] [review]
Deprecate widget flag: GTK_WIDGET_DRAWABLE

Created attachment 151020 [details] [review]
Deprecate widget flag: GTK_WIDGET_HAS_FOCUS

Created attachment 151081 [details] [review]
Deprecate widget flag: GTK_WIDGET_IS_SENSITIVE

Created attachment 151082 [details] [review]
Complete the deprecation of widget flag: GTK_WIDGET_NO_WINDOW

Created attachment 151083 [details] [review]
Deprecate widget flag: GTK_WIDGET_SENSITIVE

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

Created attachment 151134 [details] [review]
Deprecate widget flag: GTK_WIDGET_VISIBLE

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()

Created attachment 151679 [details] [review]
Add gtk_widget_has_rc_style() and deprecate GTK_WIDGET_RC_STYLE()

Comment on attachment 151679 [details] [review]
Add gtk_widget_has_rc_style() and deprecate GTK_WIDGET_RC_STYLE()

commit 6b808a7389689b3e6b89475c5a30cced2885bb1d

*** Bug 562937 has been marked as a duplicate of this bug. ***

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().

(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);

(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*.

(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.

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.

Review of attachment 151218 [details] [review]:

I think the Deprecated: line should say "Use gtk_widget_get_sensitive on the parent widget instead."

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.

Review of attachment 151083 [details] [review]:

Looks good to me.

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.

Review of attachment 151109 [details] [review]:

Looks correct, again consider flags in gtkwidget.c.

Created attachment 154796 [details] [review]
Elaborate on what to use instead of GTK_WIDGET_FLAGS

Review of attachment 154796 [details] [review]:

Excellent. Thank you.

(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?

(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.

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?

(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 on attachment 151013 [details] [review]
Deprecate widget flag: GTK_WIDGET_DRAWABLE

commit 4f78f70b15f861cca115f578b0f3f4ad3ad087e7

Comment on attachment 151081 [details] [review]
Deprecate widget flag: GTK_WIDGET_IS_SENSITIVE

commit a27d5a2c9eba7af5b056de32ff9b2b4dd1eb97e1

Comment on attachment 151082 [details] [review]
Complete the deprecation of widget flag: GTK_WIDGET_NO_WINDOW

commit f5bde06e82334995fca90ac6f103c8b2c7aa878a

Comment on attachment 151083 [details] [review]
Deprecate widget flag: GTK_WIDGET_SENSITIVE

commit 64f526d34e1adc609944078e2fea38b2792f8230

Comment on attachment 151109 [details] [review]
Deprecate widget flag: GTK_WIDGET_HAS_FOCUS.v2

commit 4232115e22c8ea41d6a3faf89bcaadfc9933d5e3

Comment on attachment 151134 [details] [review]
Deprecate widget flag: GTK_WIDGET_VISIBLE

committed an improved version in commit  214a023e9160fa008db069f5ec10ce3a4999a4dc

Comment on attachment 151218 [details] [review]
Deprecate widget flag: GTK_WIDGET_PARENT_SENSITIVE

commit b4b95d07f8b012cf29c0546f425ea3b78f2ad502

Created attachment 155000 [details] [review]
Deprecate widget flag: GTK_WIDGET_MAPPED

Created attachment 155004 [details] [review]
Deprecate widget flag: GTK_WIDGET_REALIZED

Review of attachment 155000 [details] [review]:

Looks fine.

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 on attachment 155000 [details] [review]
Deprecate widget flag: GTK_WIDGET_MAPPED

commit 1fe7d3cefd1aeabd6d8cab2a68673bb1f7876988

Comment on attachment 155004 [details] [review]
Deprecate widget flag: GTK_WIDGET_REALIZED

commit 16a59ad9129c680833c584768e6a81b2ba365268

Created attachment 155169 [details] [review]
Complete the deprecation of GTK_WIDGET_FLAGS

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.

Created attachment 155404 [details] [review]
Don't use GTK_WIDGET_*SET_FLAGS (wid, GTK_TOPLEVEL)

Created attachment 155405 [details] [review]
Don't use GTK_WIDGET_*SET_FLAGS (wid, GTK_VISIBLE)

Created attachment 155406 [details] [review]
Don't use GTK_WIDGET_*SET_FLAGS (wid, GTK_HAS_GRAB)

Created attachment 155407 [details] [review]
Don't use GTK_WIDGET_*SET_FLAGS (wid, GTK_HAS_DEFAUL)

Created attachment 155408 [details] [review]
Don't use GTK_WIDGET_*SET_FLAGS (wid, GTK_HAS_FOCUS)

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 :)

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.

Created attachment 155575 [details] [review]
Complete the deprecation of widget flag: GTK_WIDGET_STATE

Review of attachment 155405 [details] [review]:

You should use gtk_widget_set_visible here.

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+.

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.

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...

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.

Review of attachment 155575 [details] [review]:

Looks good to me.

Review of attachment 155409 [details] [review]:

Looks good.

Comment on attachment 155575 [details] [review]
Complete the deprecation of widget flag: GTK_WIDGET_STATE

commit 32b9aeaadd6dbd084344d97c573b0289c1584923

Comment on attachment 155409 [details] [review]
Complete the deprecation of GTK_WIDGET_FLAGS.v2

commit 2c043d33c2271aaf8db2e216325c7e855f048e00

Created attachment 157094 [details] [review]
Deprecate GTK_WIDGET_*SET_FLAGS

Some internal uses need to be fixed, but we can do it later

Created attachment 157132 [details] [review]
Add gtk_widget_set_is_toplevel()

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.

(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.

(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...

(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?

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)...

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 on attachment 157094 [details] [review]
Deprecate GTK_WIDGET_*SET_FLAGS

commit d3d4cf0e841df6573a41ef4ebb609ff7b81dbb0f

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