Merge lp:~macslow/unity-notifications/fix-1295762 into lp:unity-notifications

[Mirco Müller]

Added a snap-decisions-specific hint to allow control over the timeout of a notification.

Added a snap-decisions-specific hint to allow control over the timeout of a notification. If the default 60-seconds timeout is too short for a specific snap-decisions notification, an app can use the "x-canonical-snap-decisions-timeout" (integer) hint to state a longer timeout. The value is interpreted as milliseconds. This fixes LP #1295762 and LP #1315419.

Since this is meant to only address the timeout of snap-decisions notifications, the standard libnotify-way of stating a custom timeout - applied to any type of notification - has intentionally not been chosen.

* Are there any related MPs required for this MP to build/function as expected? Please list.
No.

* Did you perform an exploratory manual test run of your code change and any related functionality?
Compiled code, verified new feature works as expected on desktop, phone and tablet.

* Did you make sure that your branch does not contain spurious tags?
Yes.

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
na

* If you changed the UI, has there been a design review?
na

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:205
http://jenkins.qa.ubuntu.com/job/unity-notifications-ci/60/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-notifications-utopic-amd64-ci/1
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-notifications-utopic-armhf-ci/1
        deb: http://jenkins.qa.ubuntu.com/job/unity-notifications-utopic-armhf-ci/1/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-notifications-utopic-i386-ci/1

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-notifications-ci/60/rebuild

review: Approve (continuous-integration)

[Antti Kaijanmäki (kaijanmaki)]

LGTM.

review: Approve

[Antti Kaijanmäki (kaijanmaki)]

Tested this on n4. now the indicator-network dialogs do not expire after 60 seconds and the rest of the snap decisions do. So. TESTING DONE.

[Lars Karlitski (larsu)]

> Since this is meant to only address the timeout of snap-decisions
> notifications, the standard libnotify-way of stating a custom timeout -
> applied to any type of notification - has intentionally not been chosen.

A snap decision is just a normal notification? Why can't we just reuse the standard expiration timeout, which is not used for anything now.

As for the bug that is supposedely fixing: shouldn't indicator-network simply pass 0 as timeout to get have the bubble stay on screen until the user interacts with it?

review: Disapprove

[Jussi Pakkanen (jpakkane)]

We don't use that because design explicitly told us not to.

[Lars Karlitski (larsu)]

> We don't use that because design explicitly told us not to.

What? Since when does design care about parameters to D-Bus methods?

[Jussi Pakkanen (jpakkane)]

Senders of snap notifications are not allowed to affect how long they are shown. This behaviour is defined only in the notification service. Thus the only way to do that is to ignore the timeout argument.

[Antti Kaijanmäki (kaijanmaki)]

> Senders of snap notifications are not allowed to affect how long they are
> shown. This behaviour is defined only in the notification service. Thus the
> only way to do that is to ignore the timeout argument.

Jussi probably meant here to say "Senders of regular notifications are not.."

Anyway, we have certain scenarios where we need the snap decisions to never expire (see the bug description) and this branch fixes that.

[Lars Karlitski (larsu)]

> Senders of snap notifications are not allowed to affect how long they are
> shown. This behaviour is defined only in the notification service. Thus the
> only way to do that is to ignore the timeout argument.

But the proposed patch allows senders to affect the timeout, doesn't it? Why do we have the hint if we don't allow setting it?

[Lars Karlitski (larsu)]

> > Senders of snap notifications are not allowed to affect how long they are
> > shown. This behaviour is defined only in the notification service. Thus the
> > only way to do that is to ignore the timeout argument.
>
> Jussi probably meant here to say "Senders of regular notifications are not.."

That makes more sense. But we could just ignore the timeout parameter for non-snap-decision notifications.

> Anyway, we have certain scenarios where we need the snap decisions to never
> expire (see the bug description) and this branch fixes that.

Are there any snap decisions that _do_ expire? Maybe we can simply define them as never expiring?

=== modified file 'examples/example.py'
--- examples/example.py	2013-12-04 17:38:52 +0000
+++ examples/example.py	2014-05-07 15:39:33 +0000
@@ -49,6 +49,7 @@
 				'x-canonical-private-icon-only':     False,
 				'x-canonical-truncation':            False,
 				'x-canonical-snap-decisions':        False,
+				'x-canonical-snap-decisions-timeout':False,
 				'x-canonical-switch-to-application': False,
 				'x-canonical-secondary-icon':        False,
 				'x-canonical-private-button-tint':   False,
@@ -107,6 +108,8 @@
 		print "\tx-canonical-truncation"
 	if capabilities['x-canonical-snap-decisions']:
 		print "\tx-canonical-snap-decisions"
+	if capabilities['x-canonical-snap-decisions-timeout']:
+		print "\tx-canonical-snap-decisions-timeout"
 	if capabilities['x-canonical-switch-to-application']:
 		print "\tx-canonical-switch-to-application"
 	if capabilities['x-canonical-secondary-icon']:

=== modified file 'examples/sd-example-password-entry.py'
--- examples/sd-example-password-entry.py	2013-10-31 12:55:18 +0000
+++ examples/sd-example-password-entry.py	2014-05-07 15:39:33 +0000
@@ -118,9 +118,10 @@
 	n.set_hint ("x-canonical-private-menu-model", menu_model_paths.end ());
 
 	# indicate to the notification-daemon, that we want to use snap-decisions
-	n.set_hint_string ("x-canonical-snap-decisions", "true");
-	n.set_hint_string ("x-canonical-private-button-tint", "true");
-	n.set_hint_string ("x-canonical-non-shaped-icon", "true");
+	n.set_hint ("x-canonical-snap-decisions", GLib.Variant.new_string("true"));
+	n.set_hint ("x-canonical-snap-decisions-timeout", GLib.Variant.new_int32(90000));
+	n.set_hint ("x-canonical-private-button-tint", GLib.Variant.new_string("true"));
+	n.set_hint ("x-canonical-non-shaped-icon", GLib.Variant.new_string("true"));
 
 	Gio.bus_own_name(Gio.BusType.SESSION, APPLICATION_ID, 0, bus_acquired, None, None)
 

=== modified file 'examples/sd-example-user-auth.py'
--- examples/sd-example-user-auth.py	2013-10-31 12:55:18 +0000
+++ examples/sd-example-user-auth.py	2014-05-07 15:39:33 +0000
@@ -132,9 +132,10 @@
 	n.set_hint ("x-canonical-private-menu-model", menu_model_paths.end ());
 
 	# indicate to the notification-daemon, that we want to use snap-decisions
-	n.set_hint_string ("x-canonical-snap-decisions", "true");
-	n.set_hint_string ("x-canonical-private-button-tint", "true");
-	n.set_hint_string ("x-canonical-non-shaped-icon", "true"); 
+	n.set_hint ("x-canonical-snap-decisions", GLib.Variant.new_string("true"));
+	n.set_hint ("x-canonical-snap-decisions-timeout", GLib.Variant.new_int32 (90000));
+	n.set_hint ("x-canonical-private-button-tint", GLib.Variant.new_string("true"));
+	n.set_hint ("x-canonical-non-shaped-icon", GLib.Variant.new_string("true")); 
 
 	Gio.bus_own_name(Gio.BusType.SESSION, APPLICATION_ID, 0, bus_acquired, None, None)
 

=== modified file 'include/notify-backend.h.in'
--- include/notify-backend.h.in	2013-11-28 13:03:46 +0000
+++ include/notify-backend.h.in	2014-05-07 15:39:33 +0000
@@ -66,5 +66,6 @@
 #define BUTTON_TINT_HINT "x-canonical-private-button-tint"
 #define TRUNCATION_HINT "x-canonical-truncation"
 #define APPEND_HINT "x-canonical-append"
+#define TIMEOUT_HINT "x-canonical-snap-decisions-timeout"
 
 #endif

=== modified file 'src/NotificationServer.cpp'
--- src/NotificationServer.cpp	2013-11-28 13:03:46 +0000
+++ src/NotificationServer.cpp	2014-05-07 15:39:33 +0000
@@ -56,6 +56,7 @@
     capabilities.push_back(NON_SHAPED_ICON_HINT);
     capabilities.push_back(MENU_MODEL_HINT);
     capabilities.push_back(INTERACTIVE_HINT);
+    capabilities.push_back(TIMEOUT_HINT);
 
     return capabilities;
 }
@@ -77,7 +78,13 @@
         expireTimeout = 3000;
         ntype = Notification::Type::Confirmation;
     } else if (hints.find(SNAP_HINT) != hints.end()) {
-        expireTimeout = 60000;
+        QVariant u = hints[TIMEOUT_HINT].variant();
+        if(!u.canConvert(QVariant::Int)) {
+            expireTimeout = 60000;
+        } else {
+            expireTimeout = u.toInt();
+        }
+
         ntype = Notification::Type::SnapDecision;
     } else if(hints.find(INTERACTIVE_HINT) != hints.end()) {
         ntype = Notification::Type::Interactive;