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 741301 - Offer Print Settings when job stops
Offer Print Settings when job stops
Status: RESOLVED OBSOLETE
Product: gnome-settings-daemon
Classification: Core
Component: printers
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Marek Kašík
gnome-settings-daemon-maint
Depends on:
Blocks:
 
 
Reported: 2014-12-09 17:00 UTC by Marek Kašík
Modified: 2019-03-20 11:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Offer Print Settings to user when job stops (5.12 KB, patch)
2014-12-09 17:00 UTC, Marek Kašík
needs-work Details | Review
the notification (41.59 KB, image/png)
2014-12-09 17:02 UTC, Marek Kašík
  Details
the notification when activated by cursor (93.95 KB, image/png)
2014-12-09 17:02 UTC, Marek Kašík
  Details
Offer Print Settings to user when job stops (5.06 KB, patch)
2015-01-08 15:29 UTC, Marek Kašík
reviewed Details | Review
Offer Print Settings to user when job stops (4.75 KB, patch)
2015-01-16 13:49 UTC, Marek Kašík
needs-work Details | Review
Add function for creating/updating notification (3.73 KB, patch)
2015-07-16 07:03 UTC, mhatina
none Details | Review
Use funtion for updating notification (9.59 KB, patch)
2015-07-16 07:04 UTC, mhatina
none Details | Review
Use funtion for creating notification (5.47 KB, patch)
2015-07-16 07:04 UTC, mhatina
none Details | Review
Add/use functions for creating/updating notification (15.36 KB, patch)
2015-07-20 08:46 UTC, mhatina
none Details | Review
Add/use functions for manipulation with notifications (15.59 KB, patch)
2015-10-22 08:54 UTC, mhatina
none Details | Review
Add/use functions for manipulation with notifications (15.64 KB, patch)
2015-10-26 10:27 UTC, mhatina
none Details | Review
Add/use functions for manipulation with notifications (15.67 KB, patch)
2015-10-26 10:29 UTC, mhatina
none Details | Review
Add/use functions for manipulation with notifications (14.16 KB, patch)
2015-11-10 09:38 UTC, mhatina
none Details | Review

Description Marek Kašík 2014-12-09 17:00:51 UTC
Created attachment 292396 [details] [review]
Offer Print Settings to user when job stops

If a print job stops it is usually because of a problem with printer because of which user has to go to Printers panel of gnome-control-center. Adding action "Print Settings", which would open Printers panel, to the notification about stopped job would make this easier for user.

This was proposed by Tim Waugh here https://bugzilla.redhat.com/show_bug.cgi?id=1171418#c2 and I like the idea.

Attached patch adds the action to the notification.
Comment 1 Marek Kašík 2014-12-09 17:02:15 UTC
Created attachment 292397 [details]
the notification
Comment 2 Marek Kašík 2014-12-09 17:02:54 UTC
Created attachment 292399 [details]
the notification when activated by cursor
Comment 3 Rui Matos 2015-01-05 14:01:03 UTC
UI review would be nice but it looks fine to me FWIW
Comment 4 Marek Kašík 2015-01-05 14:06:41 UTC
Comment on attachment 292396 [details] [review]
Offer Print Settings to user when job stops

We've discussed this with Allan and I'll make some changes to the patch, I just haven't got to it yet :).
Comment 5 Marek Kašík 2015-01-08 15:29:58 UTC
Created attachment 294103 [details] [review]
Offer Print Settings to user when job stops

This version implements what we've agreed on with Allan. It does add the opening of print settings as a default action of the notification which shows up when a print job stops.
This means that only the first screenshot is correct now. User won't see the button because the notification is the button :).
Comment 6 Rui Matos 2015-01-08 18:41:57 UTC
Review of attachment 294103 [details] [review]:

I'm not familiar with cups here: if we get a bunch of cups notifications for stopped jobs are we going to create as many user visible notifications?

If yes, then we should probably keep a singleton NotifyNotification object and batch the various messages into it otherwise think might get annoying for users.

::: plugins/print-notifications/gsd-print-notifications-manager.c
@@ +868,3 @@
+                        gchar *control_center;
+
+                        control_center = g_find_program_in_path ("gnome-control-center");

This function is deprecated and in any case this isn't needed, g-c-c not being available isn't really something we should expect to happen.

If we wanted to get fancy we'd use GdkAppLaunchContext but libnotify doesn't give us a timestamp anyway so just spawning g-c-c should be fine.
Comment 7 Marek Kašík 2015-01-16 13:49:08 UTC
Created attachment 294687 [details] [review]
Offer Print Settings to user when job stops

(In reply to comment #6)
> Review of attachment 294103 [details] [review]:
> 
> I'm not familiar with cups here: if we get a bunch of cups notifications for
> stopped jobs are we going to create as many user visible notifications?

Yes, we create new notification for each event.


> If yes, then we should probably keep a singleton NotifyNotification object and
> batch the various messages into it otherwise think might get annoying for
> users.

I don't assume that users generates so many events that we would need to batch the notifications for them.


> ::: plugins/print-notifications/gsd-print-notifications-manager.c
> @@ +868,3 @@
> +                        gchar *control_center;
> +
> +                        control_center = g_find_program_in_path
> ("gnome-control-center");
> 
> This function is deprecated and in any case this isn't needed, g-c-c not being
> available isn't really something we should expect to happen.
> 
> If we wanted to get fancy we'd use GdkAppLaunchContext but libnotify doesn't
> give us a timestamp anyway so just spawning g-c-c should be fine.

I just removed the check.
Comment 8 Rui Matos 2015-01-16 14:18:21 UTC
(In reply to comment #7)
> I don't assume that users generates so many events that we would need to batch
> the notifications for them.

I think a user submitting a bunch of print jobs and they all erroring out a few seconds later is something that could definitely happen and in that case we'd get as many notifications all telling the same thing. Doesn't sound like a polished UX to me.
Comment 9 Bastien Nocera 2015-01-16 16:29:53 UTC
Review of attachment 294687 [details] [review]:

Rui is correct, we need to reuse the same notification for this. At worst, there should be one notification per printer, not per job.
Comment 10 mhatina 2015-07-16 07:03:13 UTC
Created attachment 307528 [details] [review]
Add function for creating/updating notification
Comment 11 mhatina 2015-07-16 07:04:25 UTC
Created attachment 307529 [details] [review]
Use funtion for updating notification
Comment 12 mhatina 2015-07-16 07:04:53 UTC
Created attachment 307530 [details] [review]
Use funtion for creating notification
Comment 13 Bastien Nocera 2015-07-16 08:35:08 UTC
Review of attachment 307528 [details] [review]:

This should probably be merged into the next patch which uses those functions.

::: plugins/print-notifications/gsd-print-notifications-manager.c
@@ +245,3 @@
 
+NotifyNotification *
+notification_new (const char *summary,

Pass GsdPrintNotificationsManager instead to the function.

@@ +259,3 @@
+
+NotifyNotification *
+notification_update (const char *summary,

Ditto.

@@ +263,3 @@
+                     const char *hint)
+{
+  GsdPrintNotificationsManager *manager = GSD_PRINT_NOTIFICATIONS_MANAGER (manager_object);

Because you shouldn't really access the global.
Comment 14 Bastien Nocera 2015-07-16 08:36:38 UTC
Review of attachment 307529 [details] [review]:

::: plugins/print-notifications/gsd-print-notifications-manager.c
@@ +539,2 @@
                         case IPP_JOB_ABORTED:
+                                if (job_state == IPP_JOB_STOPPED)

This seems to defeat the point of using a switch statement.

@@ +588,2 @@
                         case IPP_JOB_ABORTED:
+                                if (job_state == IPP_JOB_STOPPED)

And again.
Comment 15 Bastien Nocera 2015-07-16 08:38:15 UTC
Review of attachment 307530 [details] [review]:

This could probably be merged into the previous patches.

Also, your commit messages don't explain what your changes do for users, which they should.
Comment 16 mhatina 2015-07-20 08:46:26 UTC
Created attachment 307732 [details] [review]
Add/use functions for creating/updating notification

Fixed.
Comment 17 mhatina 2015-10-22 08:54:55 UTC
Created attachment 313849 [details] [review]
Add/use functions for manipulation with notifications
Comment 18 Bastien Nocera 2015-10-22 10:30:16 UTC
Review of attachment 313849 [details] [review]:

::: plugins/print-notifications/gsd-print-notifications-manager.c
@@ +92,3 @@
         gint                          last_notify_sequence_number;
         guint                         start_idle_id;
+        NotifyNotification           *stopped_job_notification;

Where is it you set that?

@@ +274,3 @@
+  else
+    {
+      notify_notification_update (notification,

You can't ever reach this branch because you never set ->stopped_job_notification.
Comment 19 mhatina 2015-10-26 10:27:54 UTC
Created attachment 314127 [details] [review]
Add/use functions for manipulation with notifications
Comment 20 mhatina 2015-10-26 10:29:05 UTC
Created attachment 314128 [details] [review]
Add/use functions for manipulation with notifications
Comment 21 Bastien Nocera 2015-10-29 10:36:49 UTC
Review of attachment 314128 [details] [review]:

::: plugins/print-notifications/gsd-print-notifications-manager.c
@@ +535,2 @@
                         case IPP_JOB_ABORTED:
+                                switch (job_state)

I'd really rather you used a function to simplify that, instead of defeating the switch statement.

@@ +584,2 @@
                         case IPP_JOB_ABORTED:
+                                switch (job_state)

And again here.
Comment 22 mhatina 2015-11-10 09:38:10 UTC
Created attachment 315177 [details] [review]
Add/use functions for manipulation with notifications
Comment 23 GNOME Infrastructure Team 2019-03-20 11:21:37 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gnome-settings-daemon/issues/261.