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 638674 - [PATCH] Allow moving attached dialogs
[PATCH] Allow moving attached dialogs
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: Florian Müllner
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2011-01-04 17:22 UTC by Ron
Modified: 2011-03-21 21:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Allow moving attached dialogs (2.26 KB, patch)
2011-01-04 17:22 UTC, Ron
committed Details | Review

Description Ron 2011-01-04 17:22:34 UTC
Created attachment 177498 [details] [review]
Allow moving attached dialogs

This patch allows moving an attached dialog either per ALT+MouseButton1 combo or dragging the title bar. Might close Bug #635557.
Comment 1 Florian Müllner 2011-03-21 15:47:52 UTC
Review of attachment 177498 [details] [review]:

Code looks good (except for some style suggestions), but the commit message needs improvement:
 - the subject is misleading - it implies that it allows moving the dialog despite being
   attached to its parent, rather than using it to move the parent
 - no body! See http://lists.cairographics.org/archives/cairo/2008-September/015092.html

I'd suggest something like:

display: Grab attached dialogs' parent

Currently, attached modal dialog can be grabbed (either by the
title bar, or using Alt+Button1), though they won't move when
dragged. To address this, grab the parent in that case, which
allows to move both the parent and the attached dialog.

::: src/core/display.c
@@ +3551,3 @@
 
+  grab_window = window;
+  if (meta_prefs_get_attach_modal_dialogs () &&

Could use a short comment along the lines of
/* If window is a modal dialog attached to its parent, grab the parent instead */

@@ +3556,3 @@
+      parent = meta_window_get_transient_for (window);
+      if (parent && parent != window)
+        grab_window = parent;

Maybe a bit more elegant to write this as

MetaWindow *grab_window = NULL;

...

if (meta_prefs_get_attach_modal_dialogs() &&
    window->type == META_WINDOW_MODAL_DIALOG)
  grab_window = meta_window_get_transient_for (window);

if (grab_window == NULL)
  grab_window = window;
Comment 2 Florian Müllner 2011-03-21 17:50:51 UTC
Review of attachment 177498 [details] [review]:

::: src/core/display.c
@@ +3552,3 @@
+  grab_window = window;
+  if (meta_prefs_get_attach_modal_dialogs () &&
+      window->type == META_WINDOW_MODAL_DIALOG)

Another issue I noted:
changing the grab window to the parent should only be done for move operations - doing it for resizing feels really awkward ...
Comment 3 Florian Müllner 2011-03-21 21:48:23 UTC
Pushed with the suggested fixes (as "display: Grab attached dialogs' parent").