GNOME Bugzilla – Bug 309721
Lockdown: Can't prevent panel move
Last modified: 2009-01-25 21:55:20 UTC
Although the whole panel can be locked, there doesn't seem to be a way to just prevent users from dragging the panel to a different screen edge. This causes confusion for a lot of users, and looks freakish when moved to a vertical edge.
maybe a duplicate of #114160 ?
I don't think this is a duplicate of bug 114160. It's simply to lock the panel in a way that you can't move it on the side of the screen. Bug 114160 is about locking all the applets at once but here we want to lock the panel itself, not what's on it.
What's the use case for allowing users to add/remove things from panels, but not move them? That seems spectacularly (bizarrely) finegrained to me? [But yes, confirming that it does not exist.]
You've gone and meade a mess of my simple little bug. This is about (not) moving the whole panel.
This is an important feature request and I hope that it gets addressed. For me, this bug is annoying, especially on my laptop. For my mom it is a catastrophe! Here is another description of the problem: - Occasionally when using gnome, especially if you are using a laptop touchpad or you are not an advanced computer user, the gnome panel gets dragged from the top over to the side. - The entire panel gets dragged and because the height of the screen is usually less than the width, the layout out of the panel gets messed up. Moving it back requires me to manually re-align all the panel elements again. - For new computer users or if a mac or windows user is using my computer to "check their email", this is extremely disorienting and bizarre. Why would the main panel suddenly appear on the side of the screen? Moreover, because they had not intended to do it, they don't know how to rectify the situation. Even if they figure out how to move the panel back, they may not be advanced enough to manually unlock, re-align, and relock their applets. The solution? Allow the panel to be fixed in position (top, left, right, etc.). In fact, this should be the default setting. I think shifting the main panel would be a very rare case, not a common one and therefore should be found in the preferences menu or have a "lock the panel" option similiar to how we can lock applets onto the panel now. To clarify for S. Bacher and L. Villa: this has nothing at all to do with applets being locked onto the panel or not. It has nothing to do with moving applets on the panel. It refers to the panel as a whole and how we should be able to lock the panel with regards to the screen.
I think that the panel should not be moved by dragging (just like MSWindows does). In the rare case the user would like to change the panel position he may just go to the panel properties.
*** Bug 345460 has been marked as a duplicate of this bug. ***
Ubuntu bug about that: https://launchpad.net/gnome-panel/+bug/83286
Created attachment 84881 [details] [review] gnome-panel-disable-movement.patch Novell/SUSE have a patch to add a lock-position menu item, defaulting to locked. Here is the patch, via Dan Winship. Personally, I'd maybe prefer either: - show a confirmation dialog after dragging. But people will click OK without reading it. - only start dragging after a "move panel via drag" menu item has been chosen. But then we'd still need a lockdown option, so maybe this is the best choice.
The Novell bug is https://bugzilla.novell.com/show_bug.cgi?id=72924 by the way.
Ok, let's get this applied already. :) It's really frustrating to see this happen to my non-geek girlfriend over and over. Dan's patch seems to apply mostly cleanly to 2.21. It only needed some minor modifications to panel-context-menu.c to work. Attaching an updated version that applies nicely against trunk, with some minor coding (mostly fitting things to 120 character lines) + doc fixes ("it has been know" -> "it has been known") . Can we get this applied to trunk?
Created attachment 101692 [details] [review] Updated disable movement patch for 2.21
Created attachment 101715 [details] [review] Updated disable movement patch for 2.21 that uses a combo box Was thinking about this some more, and I think it makes more sense to use a combo box in the context menu for the option, just like the "lock to panel" option for the individual applets. Having the messaging change depending on the state of the toggle is a bit confusing IMO.
> Was thinking about this some more, and I think it makes more sense to use a > combo box in the context menu for the option A combo box in a menu? I don't think I've ever seen something like that? Or do you mean a radio-menu sub-menu?
I meant a check box. I'll attach a screen shot to show how it looks.
Created attachment 102070 [details] Screenshot of panel popup with lockdown option
That's fine for me (not a panel maintainer), though I would still prefer to just have a confirmation dialog when moving the panel. That way, the problem would be fixed even for everyone, instead of just being fixed for people who find the option. Thanks.
The default of the option is to lock the panel in place, so the user would have to go out of their way to uncheck that item in order to move their panel involuntarily. People tend to just click through confirmation dialogs, especially ones they don't understand (and I can't imagine any wording for such a dialog that wouldn't be confusing to the non-technical). If anything, I would tend to agree with the earlier commenter who advocated disabling moving the panel with the mouse altogether. Unfortunately that breaks behaviour that some people have come to expect: given an odd setup of smaller sized panels, you might well want to position them with a mouse. IMO, Dan's patch is the best solution for the problem given the need to maintain a consistent interface.
> The default of the option is to lock the panel in place That seems ideal, and better than any dialog.
So can a maintainer please review/approve this patch?
Discussed this further on ubuntu-desktop. See here for the whole thread: https://lists.ubuntu.com/archives/ubuntu-desktop/2008-February/001446.html Anyway, what fell out of that is the patch above is somewhat suboptimal, since it adds a permanent "mode" to the panel. Users get confused by modes. A better solution would be to only allow the panel to be moved with the mouse if the "ALT" key is depressed, mimicking the behaviour of metacity with respect to normal windows. See here for the full rationale: https://lists.ubuntu.com/archives/ubuntu-desktop/2008-February/001455.html I coded up new patches to the panel and metacity, which I'll attach here. There's still some unresolved issues though: 1. Metacity has a special gconf setting (/apps/metacity/general/mouse_button_modifier) so that the user can specify which key they'd like to use as a modifier for making just the window move. Should gnome-panel respect this setting, even though it's only supposed to apply to metacity? 2. In the spirit of removing modes, would it also be reasonable to remove the "locked" mode on each panel applet... and make it so _they_ can only be moved with the mouse when the modifier key is pressed? I should probably ask these questions on desktop-devel or something...
Created attachment 105816 [details] [review] Patch to make panel only movable with mouse when alt key (MOD1) is depressed
Created attachment 105817 [details] [review] Patch to metacity to still pass mouse click events down to panel when alt key is pressed Required for patch #105816 to work correctly.
Created attachment 106871 [details] [review] Patch to make panel only movable with mouse when metacity modifier key is depressed Updated patch that uses the metacity gconf key to determine which modifier key should be used.
Added a new patch that uses the metacity setting for the modifier key (falling back to MOD1 if metacity isn't running). I'm not thrilled about using metacity keys inside the panel, but the panel keyboard bindings are doing this already-- one more use is hardly making things that much worse. The super key doesn't work, but I think that's a problem with GTK+ not understanding mod keys, or something. Requires more investigation.
One minor problem with this: If you set the "super" key as being the one to move windows (and the panel), you can no longer move the panel with the mouse. I think this is because the super key isn't treated as a modifier, see bug 165343.
It seems like the metacity developers agree in principle with the approach I took in my patch: http://mail.gnome.org/archives/metacity-devel-list/2008-April/msg00000.html
Well done and thanks.
Was the patch ever committed to metacity? If no, maybe you should open a metacity bug. And what about other WM? Does it work with compiz?
Belated response. Let's see if we can get this bug resolved before the end of the year. ;-) No, the patch was never committed to metacity AFAIK. I'm not sure what the point of opening a metacity bug is if the patch to the panel won't be applied. Shouldn't we just do them together? I just tested the patched panel with compiz and it worked fine with no modifications (using gnome 2.22, but it's not like things have changed that much since then).
Vuntz suggested that the metacity must be applied first and seperately before we can address the panel issue. Filed bug 554428 to make this happen (hopefully).
Created attachment 119651 [details] [review] Slightly further cleaned up version of panel movement patch Slightly cleaned up version of previous patch (in anticipation of it being actually applied). Just ignore gconf errors (as the rest of the panel does), clean up formatting, and add a comment.
The metacity patch has been committed, removing the only objection to applying the panel patch. Would be nice to get this in for the next GNOME cycle...
Comment on attachment 119651 [details] [review] Slightly further cleaned up version of panel movement patch Patch looks good, can you commit it after some style changes: >--- gnome-panel-2.20.1/gnome-panel/panel-toplevel.c 2007-10-15 17:00:47.000000000 -0300 >+++ gnome-panel-2.20.1.patched/gnome-panel/panel-toplevel.c 2008-09-30 10:56:03.000000000 -0300 >@@ -3173,12 +3173,35 @@ > GdkEventButton *event) > { > PanelToplevel *toplevel; >+ GConfClient *client; >+ char *modifier_str = NULL; >+ guint modifier_keysym; >+ guint modifier_keymask; > GtkWidget *event_widget; Make this: PanelToplevel *toplevel; + GConfClient *client; + char *modifier_str; + guint modifier_keysym; + guint modifier_keymask; GtkWidget *event_widget; > g_return_val_if_fail (PANEL_IS_TOPLEVEL (widget), FALSE); > > toplevel = PANEL_TOPLEVEL (widget); > >+ /* Get the mouse-button modifier from metacity (falling back to mod1), don't let >+ * the movement event through unless it is modified (prevents users from unintentionally >+ * moving the panel). >+ **/ Warning: mixing spaces and tabs :-) Also just close the comment with "*/" -- no need for "**/" >+ client = panel_gconf_get_client (); >+ modifier_str = gconf_client_get_string (client, "/apps/metacity/general/mouse_button_modifier", Probably worth defining the gconf key at the top of the file: we try to avoid putting various gconf paths in various places and try to regroup them at the top of files in some #define. >+ NULL); >+ >+ if (modifier_str) { >+ gtk_accelerator_parse (modifier_str, &modifier_keysym, &modifier_keymask); >+ g_free(modifier_str); Missing space: g_free (modifier_str); >+ } >+ else { On one line: } else { >+ modifier_keymask = GDK_MOD1_MASK; (I'm assuming this is the same as the default in metacity schema) >+ } >+ >+ if ((event->state & gtk_accelerator_get_default_mod_mask ()) != modifier_keymask) >+ return FALSE; >+ > if (event->button != 1 && event->button != 2) > return FALSE; > I'd actually move all the new code you did after the "if (event->button != 1 && event->button != 2)" and "if (toplevel->priv->animating)" so that we don't have to fetch the value of gconf key without any reason. Thanks, and sorry for being so long...
Actually, I rewrote the patch to move some stuff around. It's in trunk now. Thanks!