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 735375 - Does not come up when selected from Activities menu
Does not come up when selected from Activities menu
Status: RESOLVED FIXED
Product: gnome-screenshot
Classification: Core
Component: general
3.13.x
Other Linux
: Normal major
: ---
Assigned To: gnome-screenshot-maint
gnome-screenshot-maint
Depends on:
Blocks:
 
 
Reported: 2014-08-25 11:48 UTC by Ankur Sinha (FranciscoD)
Modified: 2014-09-06 07:36 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix launching from gnome-shell (11.40 KB, patch)
2014-09-01 18:53 UTC, Kalev Lember
reviewed Details | Review
Present the existing window when activated in UI mode (1.51 KB, patch)
2014-09-01 18:53 UTC, Kalev Lember
needs-work Details | Review
Start command line instances in non-unique mode (1.25 KB, patch)
2014-09-01 18:53 UTC, Kalev Lember
committed Details | Review
Fix launching from gnome-shell (12.62 KB, patch)
2014-09-04 23:47 UTC, Kalev Lember
committed Details | Review
Present the existing window when activated in UI mode (991 bytes, patch)
2014-09-04 23:48 UTC, Kalev Lember
committed Details | Review

Description Ankur Sinha (FranciscoD) 2014-08-25 11:48:34 UTC
Gnome-screenshot does not come up when selected from the Activities menu. Works just fine if I run gnome-screenshot --interactive from the terminal though. Not sure how to debug this. It works OK in gdb too. Even selecting "new window" on the right click doesn't bring up the window.

gnome-screenshot-3.13.90-1.fc21.x86_64
gnome-shell-3.13.90-1.fc21.x86_64
Comment 1 Michael Catanzaro 2014-09-01 15:28:22 UTC
Hey Matthias, I think you broke this.... When an app is activated by DBus, then the Exec line in the desktop file is ignored.  The --gapplication-service argument is incompatible with other arguments, so it's not a simple fix in the service file either.
Comment 2 Kalev Lember 2014-09-01 18:53:25 UTC
Created attachment 285052 [details] [review]
Fix launching from gnome-shell

Instead of starting in headless mode, start in the interactive mode and
bring up the UI when we're being DBus activated.

To make this possible, this patch separates the GSettings loading and
command line parsing, so we would only have to parse the command line
when started on the command line, and skip this in the DBus activation
case.
Comment 3 Kalev Lember 2014-09-01 18:53:31 UTC
Created attachment 285053 [details] [review]
Present the existing window when activated in UI mode
Comment 4 Kalev Lember 2014-09-01 18:53:34 UTC
Created attachment 285054 [details] [review]
Start command line instances in non-unique mode

This makes it possible to run non-interactive command line instances in
parallel to an existing UI instance. Activating the app from the
.desktop file launcher or starting with --interactive still brings up
the same, unique window.
Comment 5 Matthias Clasen 2014-09-02 14:28:34 UTC
Review of attachment 285054 [details] [review]:

I don't think this works. Or even if it does, it is not the right way to do it.
Comment 6 Allison Karlitskaya (desrt) 2014-09-03 16:42:05 UTC
Given the constraints of the current commandline API of gnome-screenshot, there is really only one way to do this properly:

 - create a TakeScreenshot action with a parameter field of some type

 - handle the commandline as so:

   - no args:            invoke the TakeScreenshot action
   - --delay, etc.:      invoke the action with the appropriate args
   - --interactive:      Activate the application

doing this would mean that further invocations (for example, while an interactive window was up or while we were in a delay) would forward the request to the primary instance for handling.

Because of the global-state nature of gnome-screenshot, this would probably require some amount of rewriting.  For that reason, the non-unique approach is maybe a good pragmatic solution for now.

Another consideration: perhaps there exist scripts that rely on gnome-screenshot exiting in a predictable way, particularly if used with --delay.  This is more argument towards either doing a full service/launcher split or using non-uniqueness for these cases.

Another solution could be to do a bit of a different kind of split: keep the gnome-screenshot commandline tool as the "take a screenshot now" app and split the interactive bit into a separate process which would be structured more like a typical Gtk application.  Modify the desktop file to use this other (normal) app as the Exec= line.  Invoking the (existing) commandline tool with --interactive would then just be an alias for starting that app.
Comment 7 Cosimo Cecchi 2014-09-04 20:14:20 UTC
Review of attachment 285052 [details] [review]:

Looks mostly good, some comments below.

::: src/screenshot-application.c
@@ +690,2 @@
     {
+      g_application_activate (app);

I think the previous behavior was a little different here, in that the delay, include_border and border_effect command line parameters were reflected as new defaults in the interactive dialog too. Shouldn't be too hard to put this behavior back here.

@@ +698,1 @@
+  if (window_arg && area_arg)

Can you split this parsing into a function like screenshot_config_parse_command_line()?

@@ +728,3 @@
+    screenshot_config->file = g_file_new_for_commandline_arg (file_arg);
+
+  screenshot_config->include_border = !disable_border_arg;

Not sure you need to repeat this code block here, as you already do it in screenshot_load_config()
Comment 8 Cosimo Cecchi 2014-09-04 20:17:01 UTC
Review of attachment 285053 [details] [review]:

::: src/screenshot-application.c
@@ +749,3 @@
+  window = screenshot_interactive_dialog_new ((CaptureClickedCallback) screenshot_start, self);
+  gtk_application_add_window (GTK_APPLICATION (g_application_get_default ()),
+                              GTK_WINDOW (window));

Is this really needed? screenshot_interactive_dialog_new() calls gtk_application_window_new() passing g_application_get_default() already, and I expect that to eventually call into gtk_application_add_window() already...
Comment 9 Cosimo Cecchi 2014-09-04 20:17:55 UTC
Review of attachment 285054 [details] [review]:

This looks OK until we get a more robust implementation like the one outlined by Ryan.
Comment 10 Kalev Lember 2014-09-04 23:47:53 UTC
Created attachment 285440 [details] [review]
Fix launching from gnome-shell

Instead of starting in headless mode, start in the interactive mode and
bring up the UI when we're being DBus activated.

To make this possible, this patch separates the GSettings loading and
command line parsing, so we would only have to parse the command line
when started on the command line, and skip this in the DBus activation
case.
Comment 11 Kalev Lember 2014-09-04 23:48:57 UTC
Created attachment 285441 [details] [review]
Present the existing window when activated in UI mode
Comment 12 Cosimo Cecchi 2014-09-04 23:52:32 UTC
Review of attachment 285440 [details] [review]:

Looks good with one minor nitpick

::: src/screenshot-config.h
@@ +61,3 @@
+							 const gchar *border_effect_arg,
+							 guint delay_arg,
+							 gboolean area_arg,

Weird indentation :-)
Comment 13 Cosimo Cecchi 2014-09-04 23:52:55 UTC
Review of attachment 285441 [details] [review]:

OK
Comment 14 Kalev Lember 2014-09-04 23:58:49 UTC
Thanks for the help and reviews everyone!

Attachment 285054 [details] pushed as cbdd3f5 - Start command line instances in non-unique mode
Attachment 285440 [details] pushed as dbc2099 - Fix launching from gnome-shell
Attachment 285441 [details] pushed as 71f3835 - Present the existing window when activated in UI mode
Comment 15 Ankur Sinha (FranciscoD) 2014-09-06 07:36:14 UTC
Thanks for the quick fix Kalev.  

gnome-screenshot-3.13.90-2.gitcbdd3f5.fc21.x86_64 seems to have fixed it.