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 644167 - windowManager.js: Accounting of dimmedWindows is screwy
windowManager.js: Accounting of dimmedWindows is screwy
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Maxim Ermilov
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2011-03-08 02:37 UTC by Owen Taylor
Modified: 2011-03-16 16:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
windowManager: fix up accounting of dimmed windows (7.29 KB, patch)
2011-03-08 03:12 UTC, Owen Taylor
committed Details | Review

Description Owen Taylor 2011-03-08 02:37:41 UTC
Looking at the backtraces in bug 642787, it's clear that we are somehow getting destroyed actors stuck in WindowManager._dimmedWindows; none of the fixes discussed there really address that - we need to correct the logic as well.

I think there is a basic error that is making it really hard to track dimmedWindows correctly - dimmedWindows is stored as the dialogs that have parents that are dimmed, while it would be way easier to make the logic understandable if dimmedWindows was the dimmed parent windows themselves.
Comment 1 Owen Taylor 2011-03-08 03:12:56 UTC
Created attachment 182795 [details] [review]
windowManager: fix up accounting of dimmed windows

Simplify the accounting of which windows we should dim by checking
the current state of windows rather than trying to track changes,
and by keeping a list of dimmed windows rather than a list of windows
with a dimmed parent. Remove windows from the list of dimmed windows
when they are destroyed.

This should fix problems where destroyed windows could end up in
the list of dimmed windows.
Comment 2 Owen Taylor 2011-03-08 16:34:39 UTC
Maxim - can you check this patch and see if it looks like it is correct to you?
Comment 3 Maxim Ermilov 2011-03-09 23:03:17 UTC
Review of attachment 182795 [details] [review]:

looks good.

::: js/ui/windowManager.js
@@ +251,3 @@
+            window._dimmed = true;
+            this._dimmedWindows.push(window);
+            this._dimWindow(window, true);

It will dim windows in overview.
should be:
if (!Main.overview.visible)
    this._dimWindow(window, true);
Comment 4 Owen Taylor 2011-03-16 16:31:52 UTC
>  It will dim windows in overview.
> should be:
> if (!Main.overview.visible)
>    this._dimWindow(window, true);

Good catch. Pushed with that (and the undim case) fixed.

Attachment 182795 [details] pushed as c6170ed - windowManager: fix up accounting of dimmed windows