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 723307 - window: Add "is-interesting" property
window: Add "is-interesting" property
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks: 723308
 
 
Reported: 2014-01-30 18:15 UTC by Florian Müllner
Modified: 2014-01-31 12:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
window: Add "is-interesting" property (6.50 KB, patch)
2014-01-30 18:15 UTC, Florian Müllner
reviewed Details | Review
window: Add "skip-taskbar" property (3.00 KB, patch)
2014-01-30 21:56 UTC, Florian Müllner
committed Details | Review

Description Florian Müllner 2014-01-30 18:15:29 UTC
See patch.

Some windows change their "interesting" property while mapped. It used to be kind of OK to miss that, as we "only" ended up showing apps/windows erroneously, but it is now the underlying cause of crashes (see https://bugzilla.redhat.com/show_bug.cgi?id=1057933 for instance). We could work around those quite easily, but let's actually fix the underlying bug ...
Comment 1 Florian Müllner 2014-01-30 18:15:31 UTC
Created attachment 267664 [details] [review]
window: Add "is-interesting" property

GNOME Shell has had the concept of "interesting" windows for ages,
meaning windows that should be presented as user-visible parts of
an application. Up to now we have treated the property as static,
ignoring that the underlying properties may actually change while
the window is mapped. In order to track these changes, a proper
GObject property on MetaWindow is a lot more comfortable than
anything bolted on top externally, so just move the code here.
Comment 2 Giovanni Campagna 2014-01-30 18:40:41 UTC
Review of attachment 267664 [details] [review]:

Looks good to me, because it just copies the code over from mutter, so it can't regress, but maybe the update should be in recalc_window_features()?
Also, can we consolidate this with the filtering that is done in display.c for tab list?
Comment 3 Jasper St. Pierre (not reading bugmail) 2014-01-30 20:15:56 UTC
Review of attachment 267664 [details] [review]:

How is this different from META_WINDOW_IN_NORMAL_TAB_CHAIN ?
Comment 4 Jasper St. Pierre (not reading bugmail) 2014-01-30 20:17:15 UTC
My local patch also called this meta_window_should_show_in_taskbar, which I think is a much more descriptive name than "is_interesting"
Comment 5 Giovanni Campagna 2014-01-30 20:25:50 UTC
(In reply to comment #4)
> My local patch also called this meta_window_should_show_in_taskbar, which I
> think is a much more descriptive name than "is_interesting"

But also more confusing if you compare it to meta_window_skip_taskbar()
Comment 6 Jasper St. Pierre (not reading bugmail) 2014-01-30 20:39:30 UTC
(In reply to comment #5)
> But also more confusing if you compare it to meta_window_skip_taskbar()

The fact that that function is a giant liar doesn't make it great. We could rename it to meta_window_has_skip_taskbar_hint() or something, and add a doc comment about how this is only about the _NET_WM_STATE_SKIP_TASKBAR hint.
Comment 7 Florian Müllner 2014-01-30 21:04:46 UTC
(In reply to comment #6)
> We could rename it to meta_window_has_skip_taskbar_hint() or something, and 
> add a doc comment about how this is only about the _NET_WM_STATE_SKIP_TASKBAR hint.

True, though overwriting _NET_WM_STATE_SKIP_TASKBAR as set by the application is arguably a bug and against the EWMH.

Still, I don't mind just notifying on skip-taskbar changes and changing the other patches to use the existing is_skip_taskbar() ...
Comment 8 Jasper St. Pierre (not reading bugmail) 2014-01-30 21:13:59 UTC
Erm, I'm not suggesting to overwrite anything set by the application.

I'm saying to rename the accessor method, "meta_window_is_skip_taskbar()" to "meta_window_has_skip_taskbar_hint()" to make the intent more clear, and add a new method. My local copy has:

/**
 * meta_window_should_show_in_taskbar:
 * @window: A #MetaWindow
 *
 * Returns whether a window should appear in a taskbar, alt-tab, or
 * anything that needs to enumerate over "application windows".
 *
 * This is distinct from meta_window_is_skip_taskbar() because it also
 * returns %FALSE for windows which can't accept keyboard focus, along
 * with windows with special types such as %META_WINDOW_DESKTOP and
 * %META_WINDOW_DOCK.
 */
gboolean
meta_window_should_show_in_taskbar (MetaWindow *window)
{
  return META_WINDOW_IN_NORMAL_TAB_CHAIN (window);
}
Comment 9 Florian Müllner 2014-01-30 21:17:42 UTC
(In reply to comment #8)
> Erm, I'm not suggesting to overwrite anything set by the application.

It's what we are doing right now. We do force skip_taskbar (the local property) for some window types, and then export it to _NET_WM_SKIP_TASKBAR when calling set_net_wm_state().
Comment 10 Jasper St. Pierre (not reading bugmail) 2014-01-30 21:39:32 UTC
Ugh, I see what you mean, but I'd prefer if it was the same exact style as the Alt-Tab code, which uses IN_TAB_CHAIN. If you want to fix IN_TAB_CHAIN, let's do that later, but I don't see the value in having two methods that are very, very slightly different about what they accept (input/take_focus), using one for one case and another for another case.
Comment 11 Florian Müllner 2014-01-30 21:56:30 UTC
Created attachment 267677 [details] [review]
window: Add "skip-taskbar" property

We currently only have a method to query the skip-taskbar hint.
Add a corresponding property to allow listening for change
notifications.


(In reply to comment #10)
> I don't see the value in having two methods that are very,
> very slightly different about what they accept (input/take_focus)

I'm open to changing that later, but for now I don't like the idea of changing the code on the stable branch in subtle ways. The current shell_window_tracker_is_window_interesting() is indeed the same as !meta_window_is_skip_taskbar(), so let's re-use that for now - it's also nice to only need to notify in a single place and not monitor/combine 4 separate properties.
Comment 12 Jasper St. Pierre (not reading bugmail) 2014-01-30 22:26:43 UTC
Review of attachment 267677 [details] [review]:

OK.
Comment 13 Florian Müllner 2014-01-31 12:34:38 UTC
Attachment 267677 [details] pushed as 52c24c5 - window: Add "skip-taskbar" property