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 309721 - Lockdown: Can't prevent panel move
Lockdown: Can't prevent panel move
Status: RESOLVED FIXED
Product: gnome-panel
Classification: Other
Component: panel
2.17.x
Other Linux
: Normal enhancement
: ---
Assigned To: Panel Maintainers
Panel Maintainers
: 345460 (view as bug list)
Depends on: 554428
Blocks:
 
 
Reported: 2005-07-07 16:16 UTC by Murray Cumming
Modified: 2009-01-25 21:55 UTC
See Also:
GNOME target: ---
GNOME version: Unversioned Enhancement


Attachments
gnome-panel-disable-movement.patch (8.51 KB, patch)
2007-03-19 15:36 UTC, Murray Cumming
none Details | Review
Updated disable movement patch for 2.21 (9.80 KB, patch)
2007-12-27 19:53 UTC, William Lachance
none Details | Review
Updated disable movement patch for 2.21 that uses a combo box (9.72 KB, patch)
2007-12-28 03:05 UTC, William Lachance
none Details | Review
Screenshot of panel popup with lockdown option (24.76 KB, image/jpeg)
2008-01-03 21:19 UTC, William Lachance
  Details
Patch to make panel only movable with mouse when alt key (MOD1) is depressed (450 bytes, patch)
2008-02-23 14:02 UTC, William Lachance
none Details | Review
Patch to metacity to still pass mouse click events down to panel when alt key is pressed (1.32 KB, patch)
2008-02-23 14:04 UTC, William Lachance
none Details | Review
Patch to make panel only movable with mouse when metacity modifier key is depressed (1.39 KB, patch)
2008-03-08 21:20 UTC, William Lachance
none Details | Review
Slightly further cleaned up version of panel movement patch (1.21 KB, patch)
2008-09-30 13:58 UTC, William Lachance
none Details | Review

Description Murray Cumming 2005-07-07 16:16:08 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.
Comment 1 Sebastien Bacher 2005-07-10 13:01:22 UTC
maybe a duplicate of #114160 ?
Comment 2 Thierry Moisan 2005-07-21 14:27:49 UTC
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.
Comment 3 Luis Villa 2005-07-21 21:37:01 UTC
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.]
Comment 4 Murray Cumming 2005-07-21 21:38:46 UTC
You've gone and meade a mess of my simple little bug. This is about (not) moving
the whole panel.
Comment 5 Emilie 2006-12-26 19:21:11 UTC
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.
Comment 6 Eduardo Pérez Ureta 2007-01-16 14:59:49 UTC
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.
Comment 7 Sebastien Bacher 2007-02-07 17:51:08 UTC
*** Bug 345460 has been marked as a duplicate of this bug. ***
Comment 8 Sebastien Bacher 2007-02-07 17:51:27 UTC
Ubuntu bug about that: https://launchpad.net/gnome-panel/+bug/83286
Comment 9 Murray Cumming 2007-03-19 15:36:15 UTC
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.
Comment 10 Federico Mena Quintero 2007-09-28 16:47:06 UTC
The Novell bug is https://bugzilla.novell.com/show_bug.cgi?id=72924 by the way.
Comment 11 William Lachance 2007-12-27 19:52:58 UTC
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?
Comment 12 William Lachance 2007-12-27 19:53:47 UTC
Created attachment 101692 [details] [review]
Updated disable movement patch for 2.21
Comment 13 William Lachance 2007-12-28 03:05:07 UTC
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.
Comment 14 Murray Cumming 2008-01-03 21:12:13 UTC
> 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?
Comment 15 William Lachance 2008-01-03 21:16:59 UTC
I meant a check box. I'll attach a screen shot to show how it looks.
Comment 16 William Lachance 2008-01-03 21:19:51 UTC
Created attachment 102070 [details]
Screenshot of panel popup with lockdown option
Comment 17 Murray Cumming 2008-01-03 21:28:57 UTC
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.
Comment 18 William Lachance 2008-01-03 21:44:18 UTC
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.
Comment 19 Murray Cumming 2008-01-03 21:48:48 UTC
> The default of the option is to lock the panel in place

That seems ideal, and better than any dialog.
Comment 20 Murray Cumming 2008-02-02 12:07:59 UTC
So can a maintainer please review/approve this patch?
Comment 21 William Lachance 2008-02-23 14:01:14 UTC
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...
Comment 22 William Lachance 2008-02-23 14:02:34 UTC
Created attachment 105816 [details] [review]
Patch to make panel only movable with mouse when alt key (MOD1) is depressed
Comment 23 William Lachance 2008-02-23 14:04:17 UTC
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.
Comment 24 William Lachance 2008-03-08 21:20:08 UTC
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.
Comment 25 William Lachance 2008-03-08 21:22:07 UTC
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.
Comment 26 William Lachance 2008-03-09 18:14:17 UTC
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.
Comment 27 William Lachance 2008-04-30 18:15:14 UTC
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
Comment 28 Murray Cumming 2008-04-30 19:13:21 UTC
Well done and thanks.
Comment 29 Vincent Untz 2008-08-04 02:35:56 UTC
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?
Comment 30 William Lachance 2008-09-28 19:55:18 UTC
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).
Comment 31 William Lachance 2008-09-30 13:12:53 UTC
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).
Comment 32 William Lachance 2008-09-30 13:58:25 UTC
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.
Comment 33 William Lachance 2008-10-20 15:28:30 UTC
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 34 Vincent Untz 2009-01-07 22:13:22 UTC
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...
Comment 35 Vincent Untz 2009-01-20 06:19:02 UTC
Actually, I rewrote the patch to move some stuff around. It's in trunk now.
Thanks!