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 661428 - Allow themes to know when a toplevel window appears unfocused
Allow themes to know when a toplevel window appears unfocused
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: .General
unspecified
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on: 661427
Blocks: 643217
 
 
Reported: 2011-10-11 03:29 UTC by Rui Matos
Modified: 2012-01-18 20:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add GTK_STATE_FLAG_BACKGROUND_TOPLEVEL to GtkStateFlags (6.67 KB, patch)
2011-10-11 03:29 UTC, Rui Matos
reviewed Details | Review
Add GTK_STATE_FLAG_WM_UNFOCUSED to GtkStateFlags (6.44 KB, patch)
2011-10-17 02:26 UTC, Rui Matos
needs-work Details | Review
Add GTK_STATE_FLAG_WM_UNFOCUSED to GtkStateFlags (6.44 KB, patch)
2011-10-20 22:21 UTC, Rui Matos
none Details | Review
Add a GTK_STATE_FLAG_WINDOW_UNFOCUSED widget state flag (8.62 KB, patch)
2011-10-21 22:37 UTC, Rui Matos
none Details | Review
Add a GTK_STATE_FLAG_WINDOW_UNFOCUSED widget state flag (8.52 KB, patch)
2011-10-26 17:56 UTC, Rui Matos
accepted-commit_now Details | Review
gdk: Add GDK_WINDOW_STATE_FOCUSED to GdkWindowState (1.63 KB, patch)
2011-10-28 22:50 UTC, Rui Matos
accepted-commit_now Details | Review
x11: Implement GDK_WINDOW_STATE_FOCUSED on top of _GNOME_WM_STATE_FOCUSED (3.97 KB, patch)
2011-10-28 22:51 UTC, Rui Matos
committed Details | Review
gtk: Add a GTK_STATE_FLAG_WINDOW_UNFOCUSED widget state flag (5.07 KB, patch)
2011-10-28 22:51 UTC, Rui Matos
needs-work Details | Review
gdk: Add GDK_WINDOW_STATE_FOCUSED to GdkWindowState (1.72 KB, patch)
2011-10-29 14:47 UTC, Rui Matos
committed Details | Review
gtk: Add a GTK_STATE_FLAG_WINDOW_UNFOCUSED widget state flag (4.80 KB, patch)
2011-10-29 14:57 UTC, Rui Matos
committed Details | Review
gtk: Add GTK_STYLE_CLASS_WINDOW_UNFOCUSED to GtkWindow (3.01 KB, patch)
2011-11-21 18:32 UTC, Rui Matos
none Details | Review
widget: Unset window-unfocused in gtk_widget_unparent() (999 bytes, patch)
2011-12-06 02:15 UTC, Rui Matos
committed Details | Review

Description Rui Matos 2011-10-11 03:29:10 UTC
The aim here is to be able to implement this design:
https://gitorious.org/gnome-design/gnome-design/blobs/master/mockups/theming/unfocused-window.png
.

I tried to do this in a way that shouldn't cause any behavior changes when
running under !X11 or under window managers that don't export the X property.

The mutter part is in bug 661427.
Comment 1 Rui Matos 2011-10-11 03:29:12 UTC
Created attachment 198755 [details] [review]
Add GTK_STATE_FLAG_BACKGROUND_TOPLEVEL to GtkStateFlags
Comment 2 Rui Matos 2011-10-11 03:35:17 UTC
Cosimo helped me on this but all the bad code is my fault ;-)
Comment 3 Cosimo Cecchi 2011-10-11 19:57:55 UTC
Review of attachment 198755 [details] [review]:

Nice! Conceptually looks good to me...I inlined some comments below.

::: gtk/gtkcssprovider.c
@@ +1998,3 @@
     { "focused",      0, GTK_STATE_FLAG_FOCUSED },
     { "focus",        0, GTK_STATE_FLAG_FOCUSED },
+    { "background_toplevel", 0, GTK_STATE_FLAG_BACKGROUND_TOPLEVEL },

background_toplevel is quite a bad name for this, but I'm having a hard time coming up with a better name; :inactive would sound a lot better, but then it might be easy to think it's the opposite of :active, which is really not the case (active in this context is interpreted as the toggled state of the widget, e.g. GtkToggleButton:active). :unfocused suffers the same problem, since :focused tracks the keyboard focus and not the WM focus.
Other ideas for this name:
- :behind
- :background-window
Or we could make the WM focus relation explicit in the name and go for
- :wm-inactive
- :wm-unfocused

[ in any case, the CSS convention for pseudo-classes seems to be using a dash and not an underscore to separate multiple words ]

::: gtk/gtkwindow.c
@@ +481,3 @@
+static void     set_background_toplevel                   (GtkWidget        *widget);
+
+static const gchar *appears_focused_atom_name = "_MUTTER_APPEARS_FOCUSED";

I don't like the idea of hardcoding mutter here.
I think the property looks generically useful enough to be addressed at the WM spec level, if it doesn't provide anything like that already (but I guess you will get more comments on this point in the bug you opened against mutter).

@@ +602,3 @@
+#ifdef GDK_WINDOWING_X11
+  widget_class->property_notify_event = gtk_window_property_notify_event;
+#endif

To keep the #ifdefs as low as possible, you could unconditionally wire this and the GDK_PROPERTY_CHANGE_MASK addition, and #ifdef the set_background_toplevel() call in there.

@@ +9652,3 @@
+
+  if (!gtk_widget_is_toplevel (widget))
+    return FALSE;

You should move this check in set_background_toplevel() to remove the duplication between here and gtk_window_property_notify_event()
Comment 4 Rui Matos 2011-10-17 02:26:43 UTC
Created attachment 199167 [details] [review]
Add GTK_STATE_FLAG_WM_UNFOCUSED to GtkStateFlags

(In reply to comment #3)
> - :wm-unfocused

I like this one. It even makes the code align nicely :-)

Addressed all the other comments except for the X property name for now.
Comment 5 Matthias Clasen 2011-10-18 17:33:17 UTC
Review of attachment 199167 [details] [review]:

::: gtk/gtkcssselector.c
@@ +155,3 @@
         "inconsistent",
+        "focus",
+        "wm-unfocused"

Could also do 'inactive-window', or 'window-unfocused' or so.
Mentioning a 'wm' here seems a little off to me.

::: gtk/gtkenums.h
@@ +855,3 @@
   GTK_STATE_FLAG_INCONSISTENT = 1 << 4,
+  GTK_STATE_FLAG_FOCUSED      = 1 << 5,
+  GTK_STATE_FLAG_WM_UNFOCUSED = 1 << 6

Hmm, but 'wm-unfocused' is not a generic widget state, it is something that only affects toplevel windows.

::: gtk/gtkwindow.c
@@ +9698,3 @@
+
+  return FALSE;
+}

Hmm. I'm not happy about dumping so much X11-specific code in here.
Also, this needs to be written with multi-backend in mind.
Ie, you have to check GDK_IS_X11_WINDOW before doing things like GDK_DISPLAY_XDISPLAY.
Comment 6 Rui Matos 2011-10-20 22:21:16 UTC
Created attachment 199591 [details] [review]
Add GTK_STATE_FLAG_WM_UNFOCUSED to GtkStateFlags

(In reply to comment #5)
> Review of attachment 199167 [details] [review]:
>
> ::: gtk/gtkcssselector.c
> @@ +155,3 @@
>          "inconsistent",
> +        "focus",
> +        "wm-unfocused"
>
> Could also do 'inactive-window', or 'window-unfocused' or so.
> Mentioning a 'wm' here seems a little off to me.

I agree. This patch I'm attaching now doesn't change this yet. I will change
it last, after we are sure the remaining of the patch is OK and everyone
agrees on a name.

> ::: gtk/gtkenums.h
> @@ +855,3 @@
>    GTK_STATE_FLAG_INCONSISTENT = 1 << 4,
> +  GTK_STATE_FLAG_FOCUSED      = 1 << 5,
> +  GTK_STATE_FLAG_WM_UNFOCUSED = 1 << 6
>
> Hmm, but 'wm-unfocused' is not a generic widget state, it is something that
> only affects toplevel windows.

Yes, I agree that this is a bit of a stretch as a generic widget state. But
how else can we make this so that it's easy for themes to apply a different
style to any widget depending on this info?

> ::: gtk/gtkwindow.c
> @@ +9698,3 @@
> +
> +  return FALSE;
> +}
>
> Hmm. I'm not happy about dumping so much X11-specific code in here.

Right. So, I've re-read the wm-spec and noticed that I had overlooked that the
_NET_ACTIVE_WINDOW property on the root window already tells us which window
is active. I then spent quite some time reading through gdk's code and
actually started hacking it to track root window's PropertyNotifies but this
approach would make all gtk+ applications wake up and process events whenever
any property on the root window changes. Also, since _NET_ACTIVE_WINDOW holds
an XID, each gtk+ app would have to do a non-trivial lookup to know which of
its toplevels, if any, is active. All of this sounds ugly to me.

I've renamed the property getter for now in a way that it could even be put in
gdkwindow-x11.c if we really want. But I don't think we do, I'm told that API
to track individual X properties might be coming and at that point we could
re-implement all this in a backwards compatible way and ditch this code.

What I'm saying is that _GNOME_WM_APPEARS_FOCUSED would be a temporary private
means to implement this feature and at some later point we would re-implement
it on top of _NET_ACTIVE_WINDOW.

> Also, this needs to be written with multi-backend in mind.
> Ie, you have to check GDK_IS_X11_WINDOW before doing things like
> GDK_DISPLAY_XDISPLAY.

This is addressed now.
Comment 7 Florian Müllner 2011-10-21 00:51:19 UTC
(In reply to comment #6)
> Yes, I agree that this is a bit of a stretch as a generic widget state. But
> how else can we make this so that it's easy for themes to apply a different
> style to any widget depending on this info?

Isn't something like

GtkWindow:window-unfocused MyWidget {}

supposed to work?
Comment 8 Matthias Clasen 2011-10-21 03:02:12 UTC
(In reply to comment #7)
> (In reply to comment #6)
> > Yes, I agree that this is a bit of a stretch as a generic widget state. But
> > how else can we make this so that it's easy for themes to apply a different
> > style to any widget depending on this info?
> 
> Isn't something like
> 
> GtkWindow:window-unfocused MyWidget {}
> 
> supposed to work?

Right. Cosimo convinced my that this does in fact need to be a widget state.
Comment 9 Matthias Clasen 2011-10-21 03:50:55 UTC
(In reply to comment #6)

> >
> > Hmm. I'm not happy about dumping so much X11-specific code in here.
> 
> Right. So, I've re-read the wm-spec and noticed that I had overlooked that the
> _NET_ACTIVE_WINDOW property on the root window already tells us which window
> is active. I then spent quite some time reading through gdk's code and
> actually started hacking it to track root window's PropertyNotifies but this
> approach would make all gtk+ applications wake up and process events whenever
> any property on the root window changes. Also, since _NET_ACTIVE_WINDOW holds
> an XID, each gtk+ app would have to do a non-trivial lookup to know which of
> its toplevels, if any, is active. All of this sounds ugly to me.
> 
> I've renamed the property getter for now in a way that it could even be put in
> gdkwindow-x11.c if we really want. But I don't think we do, I'm told that API
> to track individual X properties might be coming and at that point we could
> re-implement all this in a backwards compatible way and ditch this code.
> 
> What I'm saying is that _GNOME_WM_APPEARS_FOCUSED would be a temporary private
> means to implement this feature and at some later point we would re-implement
> it on top of _NET_ACTIVE_WINDOW.

Well, this is all fine. But I still think that the implementation should live in gdk/x11, even if it is temporary and will be replaced by a better one at some point in the future.
Comment 10 Rui Matos 2011-10-21 22:37:26 UTC
Created attachment 199706 [details] [review]
Add a GTK_STATE_FLAG_WINDOW_UNFOCUSED widget state flag

Ok, here's something that can be formally reviewed.

(In reply to comment #9)
> Well, this is all fine. But I still think that the implementation should live
> in gdk/x11, even if it is temporary and will be replaced by a better one at
> some point in the future.

Ok, I've done this albeit being a bit wary of exposing new API for somethink
that could be done privately for now. Is there even a way to have a
private-to-gtk gdk function?
Comment 11 Matthias Clasen 2011-10-25 03:56:43 UTC
Review of attachment 199706 [details] [review]:

::: gtk/gtkwindow.c
@@ +9656,3 @@
+{
+  if (event->atom == gdk_atom_intern_static_string ("_GNOME_WM_APPEARS_FOCUSED"))
+    set_window_unfocused (widget);

Hmm. The fact that the property name appears here kinda spoils the moving of the X11-specific code to gdk.

The really right way to do this would be to make this a window state, and extend GdkWindowState. Then we could just hook this up with GdkEventState. But that is much more invasive, and would require an ewmh addition.
Comment 12 Matthias Clasen 2011-10-26 16:46:14 UTC
Looking at http://standards.freedesktop.org/wm-spec/wm-spec-1.3.html#id2507241
it is actually allowed to use private state atoms in _NET_WM_STATE, so maybe doing it properly is not so hard after all...
Comment 13 Rui Matos 2011-10-26 17:56:13 UTC
Created attachment 200048 [details] [review]
Add a GTK_STATE_FLAG_WINDOW_UNFOCUSED widget state flag

(In reply to comment #12)
> Looking at http://standards.freedesktop.org/wm-spec/wm-spec-1.3.html#id2507241
> it is actually allowed to use private state atoms in _NET_WM_STATE, so maybe
> doing it properly is not so hard after all...

Yep, been working on it this afternoon. Done now! And it's not much more
invasive either. Thanks for the hint Mattias.
Comment 14 Matthias Clasen 2011-10-26 21:13:10 UTC
Review of attachment 200048 [details] [review]:

I like this a lot better. If you have the mutter part of this ready to go, and it they work ok together, I'm fine with it.
Comment 15 Rui Matos 2011-10-28 22:50:57 UTC
Created attachment 200219 [details] [review]
gdk: Add GDK_WINDOW_STATE_FOCUSED to GdkWindowState

This state means that the toplevel window is presented as focused to the user,
i.e with active decorations under an X11 window manager.
Comment 16 Rui Matos 2011-10-28 22:51:35 UTC
Created attachment 200220 [details] [review]
x11: Implement GDK_WINDOW_STATE_FOCUSED on top of _GNOME_WM_STATE_FOCUSED

_GNOME_WM_STATE_FOCUSED is a new _NET_WM_STATE hint which allows us to
implement a meaningful GDK_WINDOW_STATE_FOCUSED under X11. If the window
manager doesn't support this hint we keep GDK_WINDOW_STATE_FOCUSED set since
that is what gtk+ implicitly assumed historically.
Comment 17 Rui Matos 2011-10-28 22:51:43 UTC
Created attachment 200221 [details] [review]
gtk: Add a GTK_STATE_FLAG_WINDOW_UNFOCUSED widget state flag

This allows themes to style widgets differently according to whether the
toplevel window they are in is presented as focused.
Comment 18 Rui Matos 2011-10-28 22:55:37 UTC
I've split the patch and changed the _NET_WM_STATE hint to _GNOME_WM_STATE_FOCUSED after talking with Owen.
Comment 19 Matthias Clasen 2011-10-29 03:01:04 UTC
Review of attachment 200219 [details] [review]:

Commit message could perhaps mention that this is an optional hint, and will always be set if the wm doesn't support it.
Comment 20 Matthias Clasen 2011-10-29 03:03:06 UTC
Review of attachment 200220 [details] [review]:

Ah, here is the comment I was asking in the other patch...
Comment 21 Matthias Clasen 2011-10-29 03:09:00 UTC
Review of attachment 200221 [details] [review]:

::: gtk/gtkwidget.c
@@ +309,3 @@
 #define	INIT_PATH_SIZE	(512)
 
+#define GTK_STATE_FLAGS_BITS 7

Would it make sense to move this next to the GtkStateFlags enum, to keep things in sync ?

::: gtk/gtkwindow.c
@@ +595,3 @@
   widget_class->state_changed = gtk_window_state_changed;
   widget_class->style_updated = gtk_window_style_updated;
+  widget_class->window_state_event = gtk_window_window_state_event;

I must say I'm puzzled by this addition. If you look only one line further up, you see that we already have a window_state_event handler.
Comment 22 Rui Matos 2011-10-29 14:47:27 UTC
Created attachment 200254 [details] [review]
gdk: Add GDK_WINDOW_STATE_FOCUSED to GdkWindowState

This state means that the toplevel window is presented as focused to the user,
i.e with active decorations under an X11 window manager.

If the GDK backend doesn't implement this flag, it will just remain set after
mapping the window.

--

• Just expanded the commit message as pointed.
Comment 23 Rui Matos 2011-10-29 14:57:16 UTC
Created attachment 200255 [details] [review]
gtk: Add a GTK_STATE_FLAG_WINDOW_UNFOCUSED widget state flag

--

• Added the documentation comment for the state flag.

(In reply to comment #21)
> +#define GTK_STATE_FLAGS_BITS 7
>
> Would it make sense to move this next to the GtkStateFlags enum, to keep things
> in sync ?

It would make it public though and I haven't found any other examples of such
a number of bits definition anywhere on the tree. I don't even like the name
actually.

> ::: gtk/gtkwindow.c
> @@ +595,3 @@
>    widget_class->state_changed = gtk_window_state_changed;
>    widget_class->style_updated = gtk_window_style_updated;
> +  widget_class->window_state_event = gtk_window_window_state_event;
>
> I must say I'm puzzled by this addition. If you look only one line further up,
> you see that we already have a window_state_event handler.

*Brown paper bag*

That explains why my menus suddenly have resize grips...

Thanks for the review.
Comment 24 Matthias Clasen 2011-11-05 05:35:03 UTC
Review of attachment 200255 [details] [review]:

Looks good.
Comment 25 Matthias Clasen 2011-11-05 05:35:25 UTC
Review of attachment 200254 [details] [review]:

Looks good.
Comment 26 Rui Matos 2011-11-08 20:06:01 UTC
Attachment 200254 [details] pushed as 43f1b5a - gdk: Add GDK_WINDOW_STATE_FOCUSED to GdkWindowState
Attachment 200255 [details] pushed as 70f87b8 - gtk: Add a GTK_STATE_FLAG_WINDOW_UNFOCUSED widget state flag

s/GNOME/NET the atom's name on attachment 200220 [details] [review] since there seems to be
defacto acceptance of it in the wm-spec mailing list.
Comment 27 Benjamin Otte (Company) 2011-11-20 13:57:39 UTC
So, this patch is crap for multiple reasons.

- Please cite the CSS spec that defines the :window-unfocused pseudoclass. http://dev.w3.org/csswg/selectors3/ surely doesn't.
- It is not clear to me why it's a good idea that the state of the toplevel should be a widget state that must be kept up to date in _all_ children at all times.
- Widgets not inside a toplevel are considered to be inside a focused or an unfocused window? Ie should the property be set on a newly-created widget or not?
- The patch does not properly reset the window-unfocused state on widgets 
that get removed/added from/to containers in (un)focused toplevels
- Why on earth didn't we just use a style class like ".toplevel-focus" on the toplevel like we agreed on so many times before?

GtkStateFlags are a rather crappy thing by themselves due to the fact that they require proper propagation and that leads to a huge mess all the time, so they should be avoided like the plague. So unless somebody is crying very loudly, we should back that out again and use style classes instead.
Comment 28 Rui Matos 2011-11-20 16:39:15 UTC
(In reply to comment #27)
> - Please cite the CSS spec that defines the :window-unfocused pseudoclass.
> http://dev.w3.org/csswg/selectors3/ surely doesn't.

It's not. I was under the assumption that gtk+ wasn't in the business of following web standards even where it might make sense to differ/expand on them. In any case, I agree that we shouldn't part away from them without good reason and as you say below, there seems to be an alternative that allows us to achieve the same goal without adding custom stuff.

> - It is not clear to me why it's a good idea that the state of the toplevel
> should be a widget state that must be kept up to date in _all_ children at all
> times.

It just seemed logic after I decided to do this as a widget state flag.

> - Widgets not inside a toplevel are considered to be inside a focused or an
> unfocused window? Ie should the property be set on a newly-created widget or
> not?

It should be unset since that's what gtk+ historically assumed. Old themes should just continue to work as they did before this.

> - The patch does not properly reset the window-unfocused state on widgets 
> that get removed/added from/to containers in (un)focused toplevels

I don't have my laptop with me this weekend to make sure, but I'm almost sure that gtk_widget_set_parent() takes care of that automatically.

> - Why on earth didn't we just use a style class like ".toplevel-focus" on the
> toplevel like we agreed on so many times before?

Well, I never discussed this with you and the people I discussed it with didn't bring that up, although some concerns were raised about the state flag at first as you can see on this report.

> GtkStateFlags are a rather crappy thing by themselves due to the fact that they
> require proper propagation and that leads to a huge mess all the time, so they
> should be avoided like the plague. So unless somebody is crying very loudly, we
> should back that out again and use style classes instead.

Sounds like a plan, I'll get to work on it tomorrow!

I apologize for not discussing this up more widely before commiting and not researching as thoroughly as I should. I guess I was just too excited about finally contributing a new feature to gtk+ beyond bug fixes. We are still a long way from a stable release anyway, so we're still on time to fix things properly.

Thanks for the feedback!
Comment 29 Benjamin Otte (Company) 2011-11-20 17:07:22 UTC
I'm sorry. It was not meant as a critique on you Rui, I should have made this more clear. It was directed at the gtk developers who reviewed this - in this case Cosimo and Matthias. I would have expected them to at least poke me about it on IRC before. So again, sorry that you felt my wrath.

Before you get to work on this, I'd like to argue the best approach on IRC first, it might be that there's something I've missed that make this approach better. And I don't want to cause needless rework for you.
Comment 30 Benjamin Otte (Company) 2011-11-20 17:22:45 UTC
One note I want to make, because I think it's important that people know this (and tell everyone else about it):

(In reply to comment #28)
> I was under the assumption that gtk+ wasn't in the business of
> following web standards even where it might make sense to differ/expand on
> them.
>
In general, I try (and I try to convince other GTK developers) to follow CSS as closely as possible. My reasoning for this is that I want to be able to profit from advances to the CSS standard and I fear that if we go different paths, we cannot profit from them. Things like CSS 3 selectors, transitions and all these fancy things make a number of assumptions about how things work, and if we do things the right way, it makes it a lot easier to implement these things.
Also, there is the hope that new people get a head start on working with the theming code if they have used CSS in the browser before.

Of course, we need to get our features working with CSS. But I try to do this by using the same mechanisms that CSS provides to the XML documents it styles. So far this is pretty much limited to style classes. Other possibilities that we haven't touched yet would include attributes like "foo[attr=value] { color: blue }" or @media.
Comment 31 Matthias Clasen 2011-11-20 20:58:58 UTC
(In reply to comment #30)
> One note I want to make, because I think it's important that people know this
> (and tell everyone else about it):
> 
> (In reply to comment #28)
> > I was under the assumption that gtk+ wasn't in the business of
> > following web standards even where it might make sense to differ/expand on
> > them.
> >
> In general, I try (and I try to convince other GTK developers) to follow CSS as
> closely as possible. My reasoning for this is that I want to be able to profit
> from advances to the CSS standard and I fear that if we go different paths, we
> cannot profit from them. Things like CSS 3 selectors, transitions and all these
> fancy things make a number of assumptions about how things work, and if we do
> things the right way, it makes it a lot easier to implement these things.
> Also, there is the hope that new people get a head start on working with the
> theming code if they have used CSS in the browser before.


I'm with Rui here. CSS is not a windowing system. So it is to be expected that
there are some things that we want to do for which there is no direct css equivalent.

And I don't agree with your assessment that state flags are 'crap'.
Comment 32 Benjamin Otte (Company) 2011-11-20 22:43:43 UTC
(In reply to comment #31)
> And I don't agree with your assessment that state flags are 'crap'.
>
State flags is a bunch of caches that have no clear definition about how they work. They are set from the parent when a parent is set, they are not unset when a widget is unparented. Setting a state flag may or may not cause that state flag to be set on children. Unsetting a state flag may or may not cause that flag to be unset on children. Setting a state flag on your own widget may or may not cause a reset of that state flag when you change some random property on the widget or on a parent or when you reparent a widget. Some state flags are tried to be kept in sync with other state flags automatically, others are not. Unsetting a flag may cause the flag to still be set, depending on flag. The code to set flags takes an extra flag to say if a flag is going to be set, unset or toggled. It's a huge mess.

And I'm pretty sure about this because I spent a lot of time in the 3.0.x days fixing various bugs with insensitivity and other states not propagating through the hierarchy as expected because people broke things in the transition to 3.0.

So far nobody has convinced me, let alone given me a single reason, why state flags could be a good idea the way they exist today.
Comment 33 Rui Matos 2011-11-21 18:32:07 UTC
Created attachment 201845 [details] [review]
gtk: Add GTK_STYLE_CLASS_WINDOW_UNFOCUSED to GtkWindow

--

Ok, so this turned out to be pretty simple to do as a style class. I propose
that we commit something like this after reverting
70f87b8bd555068baa14df38d74b47daf0b50764.
Comment 34 Rui Matos 2011-11-21 18:35:45 UTC
See patch in bug 664493 for the theme changes.
Comment 35 Matthias Clasen 2011-11-23 19:49:12 UTC
Having widget state not depend on details of containers far up in the hierarchy was an explicit goal of many of the things we did in the run up to 3.0.

Having unfocused state propagate down to individual widgets seems like a natural fit for that...
Comment 36 Benjamin Otte (Company) 2011-11-23 21:51:24 UTC
This is not about widget state, this is about drawing. And the drawing code right now depends on widgets far up in the hierarchy already, because CSS selectors allow you to select based on parents (and even siblings).

That said, I like a widget state for it. What I don't like is that such a widget flag is not defined well enough. Some questions to think about in the implementation:
- Should the flag be set on focused or unfocused widgets?
- What is the value of a widget not inside a toplevel?
- How does that interact with Socket/Plug?
- Should random widgets be allowed to toggle the flag? It sounds useful to be able to do this for theme editors or glade.
- Should there even be a gtk_widget_set_foo() that toggles this flag, so that certain parts of the window hierarchy can toggle this flag to (de)emphasize parts of the UI?
- For such an API, should "focused" or "unfocused" be the default state?

Some questions about the concept:
- What is the desired effect here? In particular, in what situations do we expect this to happen and where don't we expect it to happen?
- How does this look different from insensitive? 
- How does this work with multi-window applications?
- How does this work with modal dialogs? Is the transient window of a focused modal dialog unfocused or not?
- How do we expect window managers' styling to interact with this?
- What widgets do we expect to be styled and in what way based on this flag?
- Do applications need to declare certain areas or windows as applicable for recoloring on unfocus or is GTK supposed to do it by itself?
- How does GTK/do themes know which parts of an application to recolor?
- How does this work with windows that aren't the active window but get interacted with?
- In particular, what happens when widgets of inactive windows get prelighted? Should prelight maybe even stop working?
- Or what about DND?
- What about focus-follows-mouse? Could that suddenly cause lots of flicker?
Comment 37 Emmanuele Bassi (:ebassi) 2011-11-23 21:54:10 UTC
let's get the input of the design team as well.
Comment 38 Jakub Steiner 2011-11-24 18:32:00 UTC
>What is the desired effect here? In particular, in what situations do we
expect this to happen and where don't we expect it to happen?

The intent is to get a flat look that communicates you aren't really interacting with widgets in this window until you focus it. Along with window decorations it should strengten the difference between focused and unfocused window:

http://gitorious.org/gnome-design/gnome-design/blobs/raw/master/mockups/theming/unfocused-window.png

One area where this might differ is that unfocused-window widgets might have a :prelight state as you can interact with them with the mouse, a click will focus.

>How does this look different from insensitive? 
I am picturing pretty much the same styling as what we have for insensitive. In terms of the theme I am imagining just adding new selectors to the existing rules for insensitive states. I don't know if we need to apply this globally on all widgets though. I definitely want the continuation of the flatness of teh WM titlebar to the toolbar below it.

>How does this work with multi-window applications?

For regular multi-window apps like gedit or epiphany, the behavior applies normally. What we might not want is utility windows, like GIMP's docks to have a flat look. But that applies to window decorations as well. I don't think the toolbox & layer docks should become unfocused when you are interacting with an image.

>How does this work with modal dialogs? Is the transient window of a focused
modal dialog unfocused or not?
I think it should become unfocused as you can't really interact with it.

>How do we expect window managers' styling to interact with this?
The whole purpose of this was to follow WM behavior. If a window is not in focus, tone it down to communicate you aren't interacting with it. But we might want to change the WM behavior wrt to utility windows as described above. This state is given from the WM, it's not like widget authors could be setting a widget to this state.

>- What widgets do we expect to be styled and in what way based on this flag?
As mentioned above, toning down menubars and toolbars was the initial drive. I don't think it's necessary to tone down all widgets globally. It may be beneficial to see insensitive widgets on an unfocused window. But we might might want to tone down the toned down there. Will try this out.

>- Do applications need to declare certain areas or windows as applicable for
recoloring on unfocus or is GTK supposed to do it by itself?

I was going to say no, this state comes from the WM. But it might be good to have a chance to override this for the utility window case which we don't want toned down.

> How does GTK/do themes know which parts of an application to recolor?
I'm affraid I don't understand the question here.

>How does this work with windows that aren't the active window but get
interacted with?
>In particular, what happens when widgets of inactive windows get prelighted?
Should prelight maybe even stop working?

Can you give an example? I don't think :prelight excludes :window-unfocused

>Or what about DND?
Nothing changes here. Drop targets should be highlighted, but the target window remains unfocused (and thus the widgets inside it).

>What about focus-follows-mouse? Could that suddenly cause lots of flicker?

No special casing for focus-follows-mouse. Unfocused windows should become flat.
Comment 39 Rui Matos 2011-11-24 19:20:19 UTC
(In reply to comment #36)
> This is not about widget state, this is about drawing. And the drawing code
> right now depends on widgets far up in the hierarchy already, because CSS
> selectors allow you to select based on parents (and even siblings).
>
> That said, I like a widget state for it. What I don't like is that such a
> widget flag is not defined well enough. Some questions to think about in the
> implementation:

Now you prefer the widget state flag approach? I'm ok with either
FWIW. I'll assume the widget state flag on my take below:

> - Should the flag be set on focused or unfocused widgets?

I've done this as set for unfocused because historically GTK+
only had the focused state implicitly. So to not break old themes
I think setting the flag for unfocused is the way to go.

> - What is the value of a widget not inside a toplevel?

This is quite arbitrary but, again for better backwards
compatibility, I'd say it shouldn't have the unfocused flag. With
the code as it is on master, the flag is already unset on widget
construction, and will eventually get it propagated from the
parent when it gets one.

What's missing is unseting the flag when the widget is
unparented. I'll do a patch for that.

> - How does that interact with Socket/Plug?

Does this really matter much? What's inside the Plug might not
even be GTK+...

> - Should random widgets be allowed to toggle the flag? It sounds useful to be
> able to do this for theme editors or glade.

Glade has a preview mode which creates a toplevel window. That
should be enough? Applications may also just call
gtk_widget_[un]set_state_flags() themselves.

> - Should there even be a gtk_widget_set_foo() that toggles this flag, so that
> certain parts of the window hierarchy can toggle this flag to (de)emphasize
> parts of the UI?

I don't think so. I see this as a theming decision. The
application could ship a custom .css file to override it.

> - For such an API, should "focused" or "unfocused" be the default state?

If such an API is created, again as I said before for backwards
compatibility, the default should be focused.
Comment 40 Rui Matos 2011-11-24 19:44:40 UTC
(In reply to comment #38)
> What we might not want is utility windows, like GIMP's docks to have
> a flat look. But that applies to window decorations as well. I don't think the
> toolbox & layer docks should become unfocused when you are interacting with an
> image.

> But we might
> want to change the WM behavior wrt to utility windows as described above. This
> state is given from the WM, it's not like widget authors could be setting a
> widget to this state.

Totally agree. Actually the wm-spec allows for this:

"_NET_WM_WINDOW_TYPE_UTILITY indicates a small persistent utility window, such as a palette or toolbox. [...] Windows of this type may set the WM_TRANSIENT_FOR hint indicating the main application window."

So, gimp would need to be patched to set its utility windows transient for the active main window and mutter would need to draw focused decorations for utility windows when their WM_TRANSIENT_FOR window is focused. I think this isn't implemented in mutter currently but it's very doable.

> >How does this work with modal dialogs? Is the transient window of a focused
> modal dialog unfocused or not?
> I think it should become unfocused as you can't really interact with it.

Currently gnome-shell draws the parent toplevel as focused along with the dialog but applies a shader effect to dim it. We could just change that if needed.
Comment 41 Rui Matos 2011-12-06 02:15:25 UTC
Created attachment 202892 [details] [review]
widget: Unset window-unfocused in gtk_widget_unparent()

Widgets without a parent aren't inside a toplevel window so we must remove
window-unfocused as it doesn't make sense outside a toplevel.
Comment 42 Matthias Clasen 2011-12-06 03:37:46 UTC
Review of attachment 202892 [details] [review]:

Makes sense.
Comment 43 Rui Matos 2011-12-07 22:46:17 UTC
Comment on attachment 202892 [details] [review]
widget: Unset window-unfocused in gtk_widget_unparent()

Attachment 202892 [details] pushed as 7a6babf - widget: Unset window-unfocused in gtk_widget_unparent()
Comment 44 Cosimo Cecchi 2012-01-18 20:57:21 UTC
I think we can safely close this now. Other bugs in separate reports.