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 740115 - InstallPrinterDrivers dialog is not modal for Printers panel
InstallPrinterDrivers dialog is not modal for Printers panel
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: Printers
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Marek Kašík
Control-Center Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-11-14 11:54 UTC by Marek Kašík
Modified: 2014-11-14 13:33 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
Make driver installation dialog modal (3.23 KB, patch)
2014-11-14 11:54 UTC, Marek Kašík 		reviewed Details | Review
Make driver installation dialog modal (1.67 KB, patch)
2014-11-14 13:19 UTC, Marek Kašík 		committed Details | Review
Use correct type for window id (2.04 KB, patch)
2014-11-14 13:20 UTC, Marek Kašík 		committed Details | Review

Description Marek Kašík 2014-11-14 11:54:26 UTC 
Created attachment 290705 [details] [review]
Make driver installation dialog modal

Dialog brought up by gnome-package's method InstallPrinterDrivers gets wrong XID and hence it shows in the centre of the screen instead of in the centre of the Printer panel. It gets XID of the New Printer Dialog instead of the Printers panel itself.
Attached patch fixes this + it retypes the window_id to guint as it should be.

I also looked at possibilities how to make this work in wayland but I don't know how yet.

Marek
Comment 1 Rui Matos 2014-11-14 13:00:56 UTC 
Review of attachment 290705 [details] [review]:

The changes in the first file are obviously correct

::: panels/printers/pp-new-printer.c
@@ +339,3 @@
+                       "WindowID",
+                       "Window ID of parent window",
+                       0, G_MAXUINT, 631,

631 ?! I know it was there already and in practice it doesn't really matter but it's totally bogus and should be fixed.

I would prefer that the changes in this file were in a different patch as they don't really relate to the bug here.
Comment 2 Marek Kašík 2014-11-14 13:19:19 UTC 
Created attachment 290709 [details] [review]
Make driver installation dialog modal

Hi,

thank you for the review.

(In reply to comment #1)
> Review of attachment 290705 [details] [review]:
> 
> The changes in the first file are obviously correct

Thanks.


> ::: panels/printers/pp-new-printer.c
> @@ +339,3 @@
> +                       "WindowID",
> +                       "Window ID of parent window",
> +                       0, G_MAXUINT, 631,
> 
> 631 ?! I know it was there already and in practice it doesn't really matter but
> it's totally bogus and should be fixed.

You are right this was a copy paste error.


> I would prefer that the changes in this file were in a different patch as they
> don't really relate to the bug here.

I've moved the changes of the second file to another patch.
Comment 3 Marek Kašík 2014-11-14 13:20:17 UTC 
Created attachment 290710 [details] [review]
Use correct type for window id

Use correct type for the window id and initialize the property holding it to 0.
Comment 4 Rui Matos 2014-11-14 13:22:27 UTC 
Review of attachment 290710 [details] [review]:

++
Comment 5 Rui Matos 2014-11-14 13:23:00 UTC 
Review of attachment 290709 [details] [review]:

yep
Comment 6 Marek Kašík 2014-11-14 13:33:52 UTC 
Thank you for the re-reviews. I've pushed the patches to master.