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 738972 - wayland: port area selection to gnome-shell api
wayland: port area selection to gnome-shell api
Status: RESOLVED FIXED
Product: gnome-screenshot
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-screenshot-maint
gnome-screenshot-maint
Depends on:
Blocks:
 
 
Reported: 2014-10-21 22:08 UTC by Matthias Clasen
Modified: 2014-10-23 17:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Port screenshot area selection to GNOME Shell's DBus API (3.72 KB, patch)
2014-10-23 16:13 UTC, Kalev Lember
reviewed Details | Review
Port screenshot area selection to GNOME Shell's DBus API (3.96 KB, patch)
2014-10-23 17:06 UTC, Kalev Lember
committed Details | Review

Description Matthias Clasen 2014-10-21 22:08:31 UTC
The area selection code does not work under Wayland. Instead, we should just call into gnome-shell, using

org.gnome.Shell.Screenshot.
      SelectArea(out i x,
                 out i y,
                 out i width,
                 out i height);

Which works fine when I tried it out using gdbus under wayland.
Comment 1 Kalev Lember 2014-10-23 16:13:42 UTC
Created attachment 289213 [details] [review]
Port screenshot area selection to GNOME Shell's DBus API

... to make screenshot selection work under Wayland.

In order to keep gnome-screenshot working with other window managers,
this keeps the old, X11 based area selection code around as a fallback
when the DBus interface is unavailable.
Comment 2 Emmanuele Bassi (:ebassi) 2014-10-23 16:55:01 UTC
Review of attachment 289213 [details] [review]:

::: src/screenshot-area-selection.c
@@ +306,3 @@
+
+  ret = g_dbus_connection_call_finish (G_DBUS_CONNECTION (source_object), res, &error);
+  if (error != NULL)

what happens when the user cancels the operation? do we get an error or a [0,0,0,0] area?
Comment 3 Emmanuele Bassi (:ebassi) 2014-10-23 16:57:31 UTC
even if we get a [0,0,0,0] area from the shell, then we should set .aborted = true, so that the screenshot app can eventually show the interactive dialog again if needed.
Comment 4 Kalev Lember 2014-10-23 17:06:03 UTC
Just tested it and we get an error when the user cancels it -- I'll amend the patch to handle that case, thanks Emmanuele!
Comment 5 Kalev Lember 2014-10-23 17:06:56 UTC
Created attachment 289215 [details] [review]
Port screenshot area selection to GNOME Shell's DBus API

... to make screenshot selection work under Wayland.

In order to keep gnome-screenshot working with other window managers,
this keeps the old, X11 based area selection code around as a fallback
when the DBus interface is unavailable.
Comment 6 Emmanuele Bassi (:ebassi) 2014-10-23 17:09:07 UTC
Review of attachment 289215 [details] [review]:

looks good to me; if Cosimo does not object, I think this is good to be merged.
Comment 7 Cosimo Cecchi 2014-10-23 17:31:16 UTC
Review of attachment 289215 [details] [review]:

Looks good with one minor comment.

::: src/screenshot-area-selection.c
@@ +354,3 @@
+                          NULL,
+                          G_DBUS_CALL_FLAGS_NONE,
+  g_variant_get (ret, "(iiii)",

You probably want no timeout here instead of the default - selecting the area could take a while.
Comment 8 Emmanuele Bassi (:ebassi) 2014-10-23 17:37:28 UTC
(In reply to comment #7)

> You probably want no timeout here instead of the default - selecting the area
> could take a while.

hopefully, not more than 25 seconds — but nice catch.
Comment 9 Kalev Lember 2014-10-23 17:51:39 UTC
Thanks for the quick review, guys! Pushed with the timeout change.

Attachment 289215 [details] pushed as 5606572 - Port screenshot area selection to GNOME Shell's DBus API