GNOME Bugzilla – Bug 633620
override-redirect windows (ex. menus) can appear below the chrome layer (ex. top panel)
Last modified: 2014-12-07 22:11:17 UTC
This is mostly visibile with Evince's zoom menu. If you select a high enough zoom percentage (say the last one in the list, 400%), and click again on the combo box, the top of the list will appear below the top panel, which hides the first two items and prevents selecting them. There's no workaround to this, except choosing the first visible item, and then clicking again on the combo box to choose the item you wanted, which is now visible. But users are not likely to understand that these items exist but are hidden, and will rather be confused. I guess this bug has something to do with the Shell always being on top, and reporting a size of the screen that doesn't exclude the top panel. Is this something the Shell can do?
Created attachment 185091 [details] Screenshot which demonstrates the problem. Screenshot of the problem happening with xchat.
Thanks for the bug report. This particular bug has already been reported into our bug tracking system, but please feel free to report any further bugs you find. *** This bug has been marked as a duplicate of bug 638904 ***
Reversing duplicate link, because my report describes a fully reproducible problem, and we have the screenshot too. ;-)
*** Bug 638904 has been marked as a duplicate of this bug. ***
*** Bug 614040 has been marked as a duplicate of this bug. ***
from bug 582653: The issue is that the panel is drawn in a separate layer from the composited windows, and the code assumes there will not be partial overlap; either the window completely overlaps the panel (fullscreen mode), or it doesn't overlap at all. There are actually two connected problems (whose pixels get drawn there and who receives clicks there). The solution that is most in line with what we are currently doing would be to update the stage_input_mask-handling code to subtract out the shape of the overlapping window, and additionally clip the chrome layer to the shape of the stage input mask so that the drawing works correctly too. (We'll assume for the moment that you aren't going to have shaped windows overlapping the panel...) An alternative solution would be to move the chrome layer into window_group and stack it appropriately based on the other windows. Then we wouldn't have to do any clipping to make it draw correctly, though we'd still have to fiddle with the input mask manually. I remember we talked about doing this when fixing bug 571827, but I don't remember if there was any particular reason we decided against it, or if it just turned out to be easier the other way. Another possibility is to make mutter itself more friendly to the idea of having an arbitrary stacking of composited windows and mutter-internal ClutterActors. From what I understand of mutter's event processing, this might involve creating InputOnly Windows for all actors that aren't MutterWindows, ensuring that the X and Clutter stacking stay in sync, and arranging for events from the InputOnly windows to be translated to ClutterEvents in the correct actors.
This is really a gtk bug: bug 641999
*** Bug 647058 has been marked as a duplicate of this bug. ***
*** Bug 647451 has been marked as a duplicate of this bug. ***
647451 (implicitly) points out that fixing gtk won't completely fix things, since pop-up menus in non-gtk apps will still have this problem
*** Bug 647811 has been marked as a duplicate of this bug. ***
*** Bug 649541 has been marked as a duplicate of this bug. ***
*** Bug 649718 has been marked as a duplicate of this bug. ***
*** Bug 649835 has been marked as a duplicate of this bug. ***
*** Bug 650034 has been marked as a duplicate of this bug. ***
*** Bug 646382 has been marked as a duplicate of this bug. ***
*** Bug 651071 has been marked as a duplicate of this bug. ***
*** Bug 631127 has been marked as a duplicate of this bug. ***
*** Bug 651963 has been marked as a duplicate of this bug. ***
*** Bug 652006 has been marked as a duplicate of this bug. ***
Created attachment 189652 [details] [review] main: move pointer-sync-on-restack code here We call global.sync_pointer() on MetaScreen::restack as a hack to try to fix up the hover state after a pointer grab. Previously we were doing this in chrome.js, since there was already a ::restack handler there anyway, but this isn't really related to the chrome at all, so move it to main.js instead.
Created attachment 189653 [details] [review] chrome: drop visibleInOverview Every place that called chrome.addActor was specifying visibleInOverview:true, and no existing designs call for chrome that disappears when you enter the overview, so just drop that as an option.
Created attachment 189654 [details] [review] chrome: fix various stacking problems by using a real window Rather than trying to manually figure out whether the chrome is supposed to be visible or not, create two GtkWindow (of type GDK_WINDOW_TYPE_DOCK) to represent the chrome layer (one for "normal" chrome, one for visibleInFullscreen chrome), reparent the chrome actors into those windows' MetaWindowActors, and rewrite input events received by those windows to look like they came from the stage instead. The end result is that the chrome is now stacked correctly automatically, regardless of what else is going on, including being able to be partially overlapped by override-redirect windows.
The last patch is not quite finished: - DND into the overview doesn't work (and I don't really know much about how DND works; hints appreciated) - If you're in the overview, and you move to the hotspot and stay there, it will leave the overview and then immediately bounce back in (because we're reparenting the chrome, causing a leave and enter. although this doesn't happen in the other direction...) - it's messing up focus-stealing-prevention somehow (apps started from terminals generally end up unfocused. not sure why yet). but it *mostly* works, and I've tested that pop-up menus pop up over it rather than under it
(In reply to comment #24) > The last patch is not quite finished: > > - DND into the overview doesn't work (and I don't really know much > about how DND works; hints appreciated) Well two things come to mind after reading your last patch: 1) In _shell_global_check_xdnd_event() you'd have to adjust the check to not filter anything not being stage/cow out but probably != chrome (something like !_shell_global_is_chrome_window (global, xev->xany.window) ) 2) In shell_global_init_xdnd() we set XAtoms XdndAware on the stage window and XdndProxy on the cow to get any events at all. You'd probably would have to make your chrome window (displaying the panel) the proxy instead.
After testing it it seems that normal DND (app internal) doesn't work as well, which I don't really have an explanation for.
Review of attachment 189652 [details] [review]: This makes sense.
And FWIW it breaks legacy icons at the top (like the old nm applet; just shows up as a white square and cannot be interacted with) .. not sure how much we care about them though ...
(In reply to comment #28) > And FWIW it breaks legacy icons at the top (like the old nm applet; just shows > up as a white square and cannot be interacted with) .. not sure how much we > care about them though ... I'm afraid while Fedora defaults to showing the (legacy) IM selector icon there we'll have to care ...
(In reply to comment #28) > And FWIW it breaks legacy icons at the top (like the old nm applet; just shows > up as a white square and cannot be interacted with) .. not sure how much we > care about them though ... I have a patch in bug 651299 to kill the legacy icon area. (Meaning all trayicons get sent to the message tray.) (In reply to comment #29) > I'm afraid while Fedora defaults to showing the (legacy) IM selector icon there > we'll have to care ... But that is supposed to be fixed for 3.2 (and is even more likely to be if there is no choice otherwise :-).
*** Bug 652536 has been marked as a duplicate of this bug. ***
*** Bug 653203 has been marked as a duplicate of this bug. ***
Comment on attachment 189652 [details] [review] main: move pointer-sync-on-restack code here Attachment 189652 [details] pushed as d5f37fa - main: move pointer-sync-on-restack code here
Created attachment 190613 [details] [review] stack: tweak layer names a bit and add a new layer Rename META_LAYER_TOP to META_LAYER_ABOVE, and META_LAYER_BOTTOM to META_LAYER_BELOW, both to match the EWMH hints, and because they are not the top and bottom layers. Rename the (unused) META_LAYER_FOCUSED_WINDOW to META_LAYER_TOP. This comes between FULLSCREEN and OVERRIDE_REDIRECT, and can be used for the shell's visible-in-fullscreen chrome layer. Put windows in this layer if they are both META_WINDOW_DOCK and wm_state_above. Remove the hole in layer numbering left by an old bugfix.
Created attachment 190614 [details] [review] window: don't deny focus due to overlap with shaped window Mutter will deny focus to a new window if there is a wm_state_above window that will block it from view. Ignore shaped windows in this comparison, since the bounds of the window don't actually tell us whether the windows will overlap in that case. ==== possibly a gross kludge... without this, everything always gets focus- stealing-prevented because of the invisible fullscreen chrome window.
Created attachment 190615 [details] [review] shell-global: remove some cruft Some of the bookkeeping associated with the ShellGlobal::screen-size-changed signal didn't get removed when that signal did.
Created attachment 190616 [details] [review] chrome: drop visibleInOverview Every place that called chrome.addActor was specifying visibleInOverview:true, and no existing designs call for chrome that disappears when you enter the overview, so just drop that as an option.
Created attachment 190617 [details] [review] chrome: fix various stacking problems by using real X windows ==== fixes the drag-and-drop problems. still has the problem where hitting the hotspot while in the overview will bounce you out and then back in again, because the pointer leaves and then re-enters the chrome window when things get restacked on overview.hide, and I haven't yet figured out a way to suppress that event without suppressing other real enters/leaves.
Review of attachment 190615 [details] [review]: Looks good.
Review of attachment 190616 [details] [review]: Yes please.
Attachment 190615 [details] pushed as bfec396 - shell-global: remove some cruft Attachment 190616 [details] pushed as 82a8ac1 - chrome: drop visibleInOverview
(In reply to comment #37) > Created an attachment (id=190616) [details] [review] > chrome: drop visibleInOverview > > Every place that called chrome.addActor was specifying > visibleInOverview:true, and no existing designs call for chrome that > disappears when you enter the overview, so just drop that as an > option. This broke the dock extension, and possibly other. Is it really necessary to remove it, in order to fix this bug?
(In reply to comment #42) > This broke the dock extension, and possibly other. Is it really necessary to > remove it, in order to fix this bug? It's not necessary, but it simplifies the code. Extensions can just connect to Main.overview's signals themselves if they want to hide when it's visible.
*** Bug 654413 has been marked as a duplicate of this bug. ***
Created attachment 191977 [details] [review] chrome: fix various stacking problems by using real X windows The Activities button double-bounce is now fixed, by using a pointer grab to avoid getting leave/enter notifications. So... I think this works.
*** Bug 655673 has been marked as a duplicate of this bug. ***
*** Bug 655946 has been marked as a duplicate of this bug. ***
*** Bug 655978 has been marked as a duplicate of this bug. ***
*** Bug 656009 has been marked as a duplicate of this bug. ***
*** Bug 658547 has been marked as a duplicate of this bug. ***
Review of attachment 191977 [details] [review]: Should this one be for 3.2? This is a pretty embarassing bug.
(In reply to comment #51) > Should this one be for 3.2? This is a pretty embarassing bug. it's not *that* embarrassing. we survived 3.0 with it. I feel like this is going to need a bunch of real-world testing after it lands, so it would be better to do it early in 3.4 rather than late in 3.2. One of the changes for bug 657986 was to make each chrome actor be a separate child of Main.uiGroup. That sets us up to have each one be a separate override-redirect window here, and as Owen suggested in bug 654625 (the mutter side of this bug), we should have an explicitly mutter API for that, rather than the current hackery of this patch.
*** Bug 659596 has been marked as a duplicate of this bug. ***
*** Bug 664235 has been marked as a duplicate of this bug. ***
*** Bug 664696 has been marked as a duplicate of this bug. ***
Comment on attachment 191977 [details] [review] chrome: fix various stacking problems by using real X windows This will need a bunch of changes to deal with the new plan for bug 654625. A few other notes (since I'm probably not going to end up being the one who rewrites this): >@@ -100,12 +99,15 @@ CtrlAltTabManager.prototype = { >+ let self = Shell.get_pid(); > let windows = display.get_tab_list(Meta.TabList.DOCKS, screen, screen.get_active_workspace ()); >+ if (windows[i].get_pid() == self) >+ continue; This was only needed because in this set of patches, chrome windows had the DOCK wm type, so it shouldn't be needed under the new scheme. >+++ b/js/ui/lookingGlass.js Stuff equivalent to this got committed as part of something else >+++ b/js/ui/xdndHandler.js >- let pickedActor = global.stage.get_actor_at_pos(Clutter.PickMode.ALL, x, y); >+ let pickedActor = global.stage.get_actor_at_pos(Clutter.PickMode.REACTIVE, x, y); This is not needed if chrome isn't fullscreen, but I think it's still correct. >@@ -305,6 +308,32 @@ gnome_shell_plugin_xevent_filter (MetaPlugin *plugin, >+ if (_shell_global_is_chrome_window (shell_plugin->global, xev->xany.window)) >+ { >+ switch (xev->type) >+ { >+ case KeyPress: ... >+ xev->xany.window = shell_plugin->stage_window; The set of events that get translated there seemed to work in basic testing, but I never bothered to figure out if all of them were necessary, or if any other events need to be translated too. Chat bubbles may need xkb events, etc.
*** Bug 665811 has been marked as a duplicate of this bug. ***
*** Bug 665970 has been marked as a duplicate of this bug. ***
There is at least bug 656009 which has been marked as a duplicate of this bug without acutally being fixed. This bug here seems to be fixed (appart from bug 652536). At least non-qt menus on my 3.4.2 installation appear above the message tray icons and below the top panel.
*** Bug 680858 has been marked as a duplicate of this bug. ***
Dan, does the patch you were working on above still make sense? (In reply to comment #59) > There is at least bug 656009 which has been marked as a duplicate of this bug > without acutally being fixed. This bug here seems to be fixed (appart from bug > 652536). At least non-qt menus on my 3.4.2 installation appear above the > message tray icons and below the top panel. See duplicate bug 680858 for a screenshot of a menu that still appears below the top bar in GIMP. The problem with Evince's zoom menu I originally reported is now fixed, but not that one. And as you said, there's also bug 652536 for Qt apps.
(In reply to comment #61) > Dan, does the patch you were working on above still make sense? yes, modulo the notes in comment 56. (In reply to comment #59) > This bug here seems to be fixed (apart from bug > 652536). At least non-qt menus on my 3.4.2 installation appear above the > message tray icons and below the top panel. gtk3 was changed to avoid overlapping the chrome (bug 641999), but the shell itself is not doing anything to avoid this problem (which is why it still shows up for non-gtk3 menus).
*** Bug 688649 has been marked as a duplicate of this bug. ***
*** Bug 688758 has been marked as a duplicate of this bug. ***
(In reply to comment #62) > (In reply to comment #61) > > Dan, does the patch you were working on above still make sense? > > yes, modulo the notes in comment 56. > > (In reply to comment #59) > > This bug here seems to be fixed (apart from bug > > 652536). At least non-qt menus on my 3.4.2 installation appear above the > > message tray icons and below the top panel. > > gtk3 was changed to avoid overlapping the chrome (bug 641999), but the shell > itself is not doing anything to avoid this problem (which is why it still shows > up for non-gtk3 menus). So... nothing will be made to fix the issue for not so older applications? where should we look to understand the problem, and maybe solve it for gtk2 apps? i mean, it's quite annoying to get the work blocked because this bug.
*** Bug 688735 has been marked as a duplicate of this bug. ***
*** Bug 690280 has been marked as a duplicate of this bug. ***
In Bug 690280, said: """ I have worked on a patch which brings the cinnamon modification in the gnome 3.6.1 code base. Information is there in the following URL http://gaps-blog.blogspot.com/2012/12/gnome-shell-popup-menu-problem-solved.html I will try to attach patch files into this bug which fixed this problem soon. """ (copy/pasted for visibility, as it was in a duplicate)
Created attachment 231725 [details] patch for gnome-shell 3.6.1 tag based branch. (Deleted since a wrong patch)
Created attachment 231729 [details] [review] patch for gnome-shell 3.6.1 tag based branch.
Created attachment 231730 [details] [review] patch for mutter 3.6.1 tag based branch.
Created attachment 231731 [details] [review] patch for mutter 3.6.1 tag based branch (missed header file update).
Mutter patched must be added first before compiling the gnome-shell patch changes. The credit for this work must go into the developers in Cinnamon project who fixed this issue in the first place.
These patches need lots of cleanup first as there is unrelated stuff included, such as diff --git a/.project b/.project new file mode 100644 index 0000000..356efdd --- /dev/null +++ b/.project @@ -0,0 +1,11 @@ +<?xml version="1.0" encoding="UTF-8"?> +<projectDescription> + <name>mutter</name> + <comment></comment> + <projects> + </projects> + <buildSpec> + </buildSpec> + <natures> + </natures> +</projectDescription>
...plus exact names of authors welcome so they can get credit. Easiest way would be using ""git format-patch"" as described on https://live.gnome.org/Git/Developers#Contributing_patches , also because the 3.6.1 codebase is already two months old.
Sorry guys this my first time in gnome project. i will try cleanup the patches as well as you have suggested and re attache them.
Created attachment 231883 [details] [review] 0001-patch-from-cinnamon-Popup-menus-appear-on-top-on-the.patch
Comment on attachment 231883 [details] [review] 0001-patch-from-cinnamon-Popup-menus-appear-on-top-on-the.patch Orginal Commit in Cinnamon Project : https://github.com/linuxmint/Cinnamon/commit/f8c7922777b102dc574bb3695fc649b89879ebab
Created attachment 231885 [details] [review] 0001-patch-from-muffin-Put-popup-menus-into-a-separate-wi.patch Original Commits from muffin : https://github.com/linuxmint/muffin/commit/9e45c4c8f1cc2f92f266a5f8e1c4e83d4ce70ef1 https://github.com/linuxmint/muffin/commit/0e575275078d4b8b518978448bba60f404b3746d
First of all, let me thank you for taking time with this long-standing bug. You're effort is appreciated. Secondly, a friendly advice: at https://live.gnome.org/GnomeShell/Development/WorkingWithPatches you can find a step by step guide to produce and attach patches for gnome-shell, which should also ease your workflow with automated commands. I'll review them in their current form, but it would be very helpful if you could reformat them.
Review of attachment 231885 [details] [review]: ::: src/compositor/meta-window-actor.c @@ +1509,3 @@ meta_window_set_compositor_private (window, G_OBJECT (self)); + if (window->type == META_WINDOW_POPUP_MENU){ This should check for window->layer == META_LAYER_OVERRIDE_REDIRECT
Review of attachment 231883 [details] [review]: ::: js/ui/layout.js @@ +1076,3 @@ } + if (global.top_window_group.get_children().length == 0) The logic is correct, but updateRegions may not be called often enough. You need to add a check for this condition inside _windowRestacked, and call _queueUpdateRegions if changed.
Created attachment 232235 [details] [review] Merged the fix for popup menus appearing below shell panel from mutter.
Created attachment 232236 [details] [review] Merged the fix for popup menus appearing below shell panel from cinnamon.
Added suggested changes and attached the patched using git bz script.
Review of attachment 232235 [details] [review]: ::: src/compositor/meta-window-actor.c @@ +1509,3 @@ meta_window_set_compositor_private (window, G_OBJECT (self)); + if (window->layer == META_LAYER_OVERRIDE_REDIRECT){ Fix the coding style. The curly brace should be on the next line and indented with two spaces i.e if (...) { do_somthing } @@ +1513,2 @@ CLUTTER_ACTOR (self)); + }else{ Same the else should be in a separate line (just look at the other code).
Review of attachment 232236 [details] [review]: It seems you forgot the other fix from the first patch? ::: js/ui/main.js @@ +132,3 @@ global.overlay_group.reparent(uiGroup); global.stage.add_actor(uiGroup); + global.top_window_group.reparent(global.stage); Reparent this to the uiGroup, please.
Comment on attachment 191977 [details] [review] chrome: fix various stacking problems by using real X windows Cleaning up...
You also need a better commit message, formatted as a single line of subject and a longer description.
(In reply to comment #87) > Review of attachment 232236 [details] [review]: > > It seems you forgot the other fix from the first patch? > > ::: js/ui/main.js > @@ +132,3 @@ > global.overlay_group.reparent(uiGroup); > global.stage.add_actor(uiGroup); > + global.top_window_group.reparent(global.stage); > > Reparent this to the uiGroup, please. I Didn't get you jasper, just rechecked the first patch branch i was working on to see whether i missed any thing, but i guess it contains all the changes. Do i still missing any thing ?
It doesn't have the code that sets the stage input mode, etc.
(In reply to comment #91) > It doesn't have the code that sets the stage input mode, etc. Ohhh i moved that logic as follows inside _windowsRestacked function for input i got for that patch. if ((wasInFullscreen[i] != this._monitors[i].inFullscreen) || (global.top_window_group.get_children().length > 0)) { changed = true; break; }
Giovanni was suggesting to add that to ensure that the region is set in more cases. Simply setting "changed" to true is not sufficient, from my understanding of the code.
Created attachment 232267 [details] [review] layout: Fixed popup menus going behind the panel. Added a new window group on top of existing group and made popup menus appear in this new window group so that the popup menus will stay on top of the top panel. This fix depends on the mutter change which introduce top window group. The original fix is from the cinnamon desktop.
Created attachment 232268 [details] [review] compositor: Introduced new top window group Added a new window group on top of existing group and made popup menus appear in this new window group so that they appears on top of others. The original fix is from the muffin.
(In reply to comment #93) > Giovanni was suggesting to add that to ensure that the region is set in more > cases. Simply setting "changed" to true is not sufficient, from my > understanding of the code. I'm not a expert in this code, actually i rate my self as a novice. i'm really sorry if i'm wasting the time of you guys. According to my understanding the updateRegions is called in a asynchronous mode by the _queueUpdateRegions and the _windowsRestacked is a observer method of the restacked signal from the global.screen which is actually fired from the mutter code. And isn't this signal is fired when the popup menu is shown since it need to stack the windows ? And isn't it enough to execute this logic only when a restack is performed. Because for my understanding the updateRegions method will be called for updated which not only cause by re stacking windows.
Guys any updated on this issue ?
(In reply to comment #97) > Guys any updated on this issue ? Sorry for bad english :(, i mean "Guys any updates on this issue ?"
Sorry for the late review. Note that I'm not entirely happy with this alternate, and I'd prefer if the panel were stacked like X windows, like with Dan's original approach, but that was never completed, so let's go with this approach. However, the patch here has serious issues if landed directly as-is. (In reply to comment #96) > According to my understanding the updateRegions is called in a asynchronous > mode by the _queueUpdateRegions and the _windowsRestacked is a observer method > of the restacked signal from the global.screen which is actually fired from the > mutter code. And isn't this signal is fired when the popup menu is shown since > it need to stack the windows ? And isn't it enough to execute this logic only > when a restack is performed. Because for my understanding the updateRegions > method will be called for updated which not only cause by re stacking windows. The regions defined in set_stage_input_region are the regions where you are allowed to click on the stage, at the expense of being able to click on windows. If you set it so that the list of regions is empty, that means that you can only click on X windows, and cannot click on the Shell UI. In normal Shell usage, the set of input regions is only the top panel, as there's usually no other UI that takes over part of the screen. While your patches make a top_window_group to allow the appearance of some X windows to go on top of the uiGroup (and other shell UI), they don't allow the panel to be clicked, because the set of input regions is still the same. The patch before accomplished this by setting the set of input regions to be empty if there are any override-redirect windows in the set, but you removed this part of the patch. Note that there's still an issue -- opening up any override-redirect window should not give us an empty input region. While we could do accurate tracking of the input region, we could instead check to make sure that all override-redirect windows use are of the correct type: function isWindowMenu(actor) { switch (actor.meta_window.get_window_type()) { case Meta.WindowType.DROPDOWN_MENU: case Meta.WindowType.POPUP_MENU: case Meta.WindowType.COMBO: return true; default: return false; } } ... // Windows of these types should take pointer grabs, // making it safe to empty the input region. if (global.top_window_group.get_children().some(isWindowMenu)) { global.set_stage_input_rects([]); } else { global.set_stage_input_rects(rects); } Would you like me to update your patches to fix this?
Thank you very much for taking time explaining the code Jasper :). So if i understood you correct rather than blindly making the global.set_stage_input_rects to empty by looking at if the new window group have any children, its better to check if the children in the group require us to change the input rects right ? And also the suggestion in comment 82 by Giovanni is that not to remove what i had in the patch but add another check to the windowsRestacked and call queueUpdateRegions so that the input region will be updated. I would like to take another chance of changing the patch with the help you have provided. Will that be ok Jasper ? If i fail again i think its better you fix the patched so that this bug can be resolved.
(In reply to comment #100) > Thank you very much for taking time explaining the code Jasper :). So if i > understood you correct rather than blindly making the > global.set_stage_input_rects to empty by looking at if the new window group > have any children, its better to check if the children in the group require us > to change the input rects right ? And also the suggestion in comment 82 by > Giovanni is that not to remove what i had in the patch but add another check to > the windowsRestacked and call queueUpdateRegions so that the input region will > be updated. Right. What happens is that every so often, we need to double-check if the input regions we set are accurate, and if they're not, we need to make sure we recalculate them. If the windows are restacked and the top window group doesn't have any menus or combo box windows any more, we need to make sure we recalculate. That's what Giovanni was saying. > I would like to take another chance of changing the patch with the help you > have provided. Will that be ok Jasper ? Go for it!
Created attachment 233948 [details] [review] layout: Fixed popup menus going behind the panel. Added a new window group on top of existing group and made popup menus appear in this new window group so that the popup menus will stay on top of the top panel. This fix depends on the mutter change which introduce top window group. The original fix is from the cinnamon desktop.
(In reply to comment #101) > (In reply to comment #100) > > Thank you very much for taking time explaining the code Jasper :). So if i > > understood you correct rather than blindly making the > > global.set_stage_input_rects to empty by looking at if the new window group > > have any children, its better to check if the children in the group require us > > to change the input rects right ? And also the suggestion in comment 82 by > > Giovanni is that not to remove what i had in the patch but add another check to > > the windowsRestacked and call queueUpdateRegions so that the input region will > > be updated. > > Right. What happens is that every so often, we need to double-check if the > input regions we set are accurate, and if they're not, we need to make sure we > recalculate them. > > If the windows are restacked and the top window group doesn't have any menus or > combo box windows any more, we need to make sure we recalculate. That's what > Giovanni was saying. > > > I would like to take another chance of changing the patch with the help you > > have provided. Will that be ok Jasper ? > > Go for it! I attached a new patch with the information you provided. And also i had to handle a problem which i found while testing where when the input region is set to empty and once the popup up menu is closed is not set to top panel rects which causes mouse events not to be sent to top panel. But i managed to fix it by keeping track of whether we need to reset the input region back to original.
(In reply to comment #82) > Review of attachment 231883 [details] [review]: > > ::: js/ui/layout.js > @@ +1076,3 @@ > } > > + if (global.top_window_group.get_children().length == 0) > > The logic is correct, but updateRegions may not be called often enough. You > need to add a check for this condition inside _windowRestacked, and call > _queueUpdateRegions if changed. Actually, I don't understand why this should ever happen. Windows are never restacked into the window_group (which could be a bug if a window happens to change its type to a popup menu, which is unlikely), they're simply added/removed from their own window group.
Review of attachment 233948 [details] [review]: I'm going to wait for Giovanni's comment on windowsRestacked. ::: js/ui/layout.js @@ +684,2 @@ this._trackedActors = []; + this._resetInputRegion = false; I'd prefer if this was named this._isPopupMenuVisible @@ +968,3 @@ + // We mark as changed if region was emptied by updateRegion before or + // the top_window_group has a popup meta window. + if (!changed && (this._resetInputRegion || if (!changed && (this._isPopupMenuVisible != global.top_window_group.get_children().some(isPopupMetaWindow))) changed = true; @@ +1103,3 @@ }); +function isPopupMetaWindow(actor) { This utility function should go at the top. ::: js/ui/main.js @@ +132,3 @@ global.overlay_group.reparent(uiGroup); global.stage.add_actor(uiGroup); + global.top_window_group.reparent(global.stage); And as I said in a previous review, top_window_group should be reparented to the uiGroup. Put this line below the reparenting of the window_group, too.
(In reply to comment #104) > Actually, I don't understand why this should ever happen. Windows are never > restacked into the window_group (which could be a bug if a window happens to > change its type to a popup menu, which is unlikely), they're simply > added/removed from their own window group. Yes, windows are not restacked. What I was referring to is that windowRestacked is the best approximation of "contents of window_group/top_window_group" changed. Otherwise, updateRegions is only called when the panel moves, overview opened/closed, fullscreen changes, n-workspace changes or keyboard opened/closed.
Ah, OK.
(In reply to comment #105) > Review of attachment 233948 [details] [review]: > > I'm going to wait for Giovanni's comment on windowsRestacked. > > ::: js/ui/layout.js > @@ +684,2 @@ > this._trackedActors = []; > + this._resetInputRegion = false; > > I'd prefer if this was named this._isPopupMenuVisible > > @@ +968,3 @@ > + // We mark as changed if region was emptied by updateRegion before or > + // the top_window_group has a popup meta window. > + if (!changed && (this._resetInputRegion || > > if (!changed && (this._isPopupMenuVisible != > global.top_window_group.get_children().some(isPopupMetaWindow))) > changed = true; > > @@ +1103,3 @@ > }); > > +function isPopupMetaWindow(actor) { > > This utility function should go at the top. > > ::: js/ui/main.js > @@ +132,3 @@ > global.overlay_group.reparent(uiGroup); > global.stage.add_actor(uiGroup); > + global.top_window_group.reparent(global.stage); > > And as I said in a previous review, top_window_group should be reparented to > the uiGroup. Put this line below the reparenting of the window_group, too. "I'd prefer if this was named this._isPopupMenuVisible" I just thought "isPopupMetaWindow" would be good since we are checking for window type which is one of the types shown as popups. If still needs to be changed then it's ok i can change it with other changes.
(In reply to comment #108) > (In reply to comment #105) > > Review of attachment 233948 [details] [review] [details]: > > > > I'm going to wait for Giovanni's comment on windowsRestacked. > > > > ::: js/ui/layout.js > > @@ +684,2 @@ > > this._trackedActors = []; > > + this._resetInputRegion = false; > > > > I'd prefer if this was named this._isPopupMenuVisible > > > > @@ +968,3 @@ > > + // We mark as changed if region was emptied by updateRegion before or > > + // the top_window_group has a popup meta window. > > + if (!changed && (this._resetInputRegion || > > > > if (!changed && (this._isPopupMenuVisible != > > global.top_window_group.get_children().some(isPopupMetaWindow))) > > changed = true; > > > > @@ +1103,3 @@ > > }); > > > > +function isPopupMetaWindow(actor) { > > > > This utility function should go at the top. > > > > ::: js/ui/main.js > > @@ +132,3 @@ > > global.overlay_group.reparent(uiGroup); > > global.stage.add_actor(uiGroup); > > + global.top_window_group.reparent(global.stage); > > > > And as I said in a previous review, top_window_group should be reparented to > > the uiGroup. Put this line below the reparenting of the window_group, too. > > "I'd prefer if this was named this._isPopupMenuVisible" I just thought > "isPopupMetaWindow" would be good since we are checking for window type which > is one of the types shown as popups. If still needs to be changed then it's ok > i can change it with other changes. Ignore my question, i was referring to a wrong comment there, i totally agree with the name suggested when looking at the variable name i have used. But i would like if i can name it as _isPopupWindowVisible, what do you guys think ?
> ::: js/ui/main.js > @@ +132,3 @@ > global.overlay_group.reparent(uiGroup); > global.stage.add_actor(uiGroup); > + global.top_window_group.reparent(global.stage); > > And as I said in a previous review, top_window_group should be reparented to > the uiGroup. Put this line below the reparenting of the window_group, too. This didn't work, when i change the code as you suggested the popup menu goes under the top panel. I wonder the top panel is in the stage and when we re-parent our new top_window_group to that then it will be same as what we have now. Just a wild guess :)
(In reply to comment #105) > Review of attachment 233948 [details] [review]: > > > +function isPopupMetaWindow(actor) { > > This utility function should go at the top. > What you ment by top ? is it to the same level we have updateRegion and windowsRestacked methods ,which are in Chrome ?
How can this not be confirmed? It affects all users, and every app that is maximized as well.
We do not differentiate between the UNCONFIRMED and NEW statuses.
(In reply to comment #110) > > ::: js/ui/main.js > > @@ +132,3 @@ > > global.overlay_group.reparent(uiGroup); > > global.stage.add_actor(uiGroup); > > + global.top_window_group.reparent(global.stage); > > > > And as I said in a previous review, top_window_group should be reparented to > > the uiGroup. Put this line below the reparenting of the window_group, too. > > This didn't work, when i change the code as you suggested the popup menu goes > under the top panel. I wonder the top panel is in the stage and when we > re-parent our new top_window_group to that then it will be same as what we have > now. Just a wild guess :) You need to reparent top_window_group after LayoutManager adds panelBox there. (If top_window_group is not reparented to uiGroup, windows there will not be visible when using the magnifier). You probably want to take the patch from bug 682429, that moves the stage hierarchy to LayoutManager as a whole, for cleanliness.
Ok even without the patch if i move global.top_window_group.reparent(uiGroup); just after the layoutManager.init(); it should be ok right ? cause after init method the paneBoxl is already added into the uiGroup.
Yes, that would do.
Created attachment 234742 [details] [review] layout: Fixed popup menus going behind the panel. Added a new window group on top of existing group and made popup menus appear in this new window group so that the popup menus will stay on top of the top panel. This fix depends on the mutter change which introduce top window group. The original fix is from the cinnamon desktop.
*** Bug 693503 has been marked as a duplicate of this bug. ***
Created attachment 235918 [details] [review] compositor: Add a new window group for override-redirect windows Put override redirect windows such as menus into a separate window group stacked above everything else. This will allow us to visually put these above other compositior chrome. Based on a patch from Muffin.
Review of attachment 235918 [details] [review]: ::: src/compositor/compositor.c @@ +607,2 @@ info->window_group = meta_window_group_new (screen); + info->top_window_group = meta_window_group_new (screen); Shouldn't we add this to the stage like we do for all other groups?
Review of attachment 234742 [details] [review]: ::: js/ui/layout.js @@ +24,3 @@ + // above top panel. + switch(actor.meta_window.get_window_type()) { + case Meta.WindowType.DROPDOWN_MENU: Indentation is broken. ::: js/ui/main.js @@ +152,2 @@ layoutManager.init(); + global.top_window_group.reparent(uiGroup); We should hide the group when going to the overview and show it once we leave it.
Created attachment 235943 [details] [review] compositor: Add a new window group for override-redirect windows Put override redirect windows such as menus into a separate window group stacked above everything else. This will allow us to visually put these above other compositior chrome. Based on a patch from Muffin.
Review of attachment 235943 [details] [review]: Looks good.
Created attachment 235946 [details] [review] layout: Don't use reparent reparent() defines the new actor stacking order based on the existing depth of the actor, which is flat out wrong. Simply remove the actor from its old parent and add the new one in.
Created attachment 235947 [details] [review] layout: Restructure input region and struts code Have two branches, one for input region and one for struts. This makes it easier to skip one of the branches, like in the case where we want to skip input regions if we have a popup menu visible.
Created attachment 235948 [details] [review] Place popup menus and other override-redirect windows on top of the panel
*** Bug 693785 has been marked as a duplicate of this bug. ***
Review of attachment 235946 [details] [review]: OK, reparent is deprecated anyway.
Review of attachment 235947 [details] [review]: Ok.
Review of attachment 235948 [details] [review]: Hide the top_window_group when going to the overview.
Comment on attachment 235943 [details] [review] compositor: Add a new window group for override-redirect windows Attachment 235943 [details] pushed as 6b5cf2e - compositor: Add a new window group for override-redirect windows
Attachment 235946 [details] pushed as 978ac65 - layout: Don't use reparent Attachment 235947 [details] pushed as a4e70ba - layout: Restructure input region and struts code Attachment 235948 [details] pushed as 22ddec4 - Place popup menus and other override-redirect windows on top of the panel Pushed with suggested change.
*** Bug 698596 has been marked as a duplicate of this bug. ***
Is this now fixed? And if so in which version of Gnome3? Just so I know whether to expect the same issues in Fedora 19 when it is released. (And apologies in advance if this is clear - I'm not too familiar with Gnome3 development nor bugzilla).
As mentioned in bug 698596: It's fixed in 3.8.x, which will be included with Fedora 19.
*** Bug 700765 has been marked as a duplicate of this bug. ***
What about deban whith it x11? you make for wayland but not for X11
The fix has nothing to do with Wayland - the issue is fixed on X11 just as well.
(In reply to comment #138) > The fix has nothing to do with Wayland - the issue is fixed on X11 just as > well. compositor-private.h it is affect to Wayland. I can't find this file in my desktop with X11
(In reply to comment #139) > (In reply to comment #138) > > The fix has nothing to do with Wayland - the issue is fixed on X11 just as > > well. > > compositor-private.h it is affect to Wayland. No its not. > I can't find this file in my desktop with X11 The file is not one that gets installed its only used during compilation. Trust us we know what we are talking about ;) Its fixed in gnome-shell 3.8.x+ if you are running an older version you don't have the fix. Debian stable still runs 3.4 which is too old. But as Florian said this has nothing to do with wayland.
ok, I am sorry. How can I fix it in older?
I find this solution https://www.ailis.de/~k/archives/67-Workaround-for-borderless-Java-Swing-menus-on-Linux.html it is work, but it have bad appearance
Is it have some property padding-top? it will be a nice:) sorry for my English and for disturbance
Sorry but Bugzilla is not a support forum. Please ask your distributor or a forum of your distribution for backfixes or how you can upgrade.