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 763611 - Restart and install dialog for OS upgrades
Restart and install dialog for OS upgrades
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
3.21.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2016-03-14 12:26 UTC by Allan Day
Modified: 2016-06-30 09:44 UTC
See Also:
GNOME target: 3.20
GNOME version: ---


Attachments
endSessionDialog: Rename a variable (1.14 KB, patch)
2016-03-22 19:22 UTC, Kalev Lember
committed Details | Review
endSessionDialog: Use new PackageKit DBus API (2.72 KB, patch)
2016-03-22 19:22 UTC, Kalev Lember
committed Details | Review
endSessionDialog: Add support for system upgrades (4.85 KB, patch)
2016-03-22 19:22 UTC, Kalev Lember
none Details | Review
endSessionDialog: Add support for system upgrades (5.09 KB, patch)
2016-06-21 14:22 UTC, Kalev Lember
committed Details | Review

Description Allan Day 2016-03-14 12:26:52 UTC
Software now supports installing operating system upgrades - a system modal confirmation dialog for restarting and performing the upgrade is therefore required.

The dialog should be similar to the dialog for restarting and installing updates, with some differences. A countdown until the reboot should not be used in this case: OS upgrades are too time consuming and too serious a change to install without explicit user confirmation.

Suggested text for the dialog:

"Restart & Install Upgrade"

"<distro> <version> will be installed after restart. Upgrade installation can take a long time: ensure that you have backed up and that the computer is plugged in."
Comment 1 Kalev Lember 2016-03-16 20:56:08 UTC
Something like https://kalev.fedorapeople.org/shutdown-dialog-system-upgrade.png ?
Comment 2 Matthias Clasen 2016-03-17 17:03:33 UTC
Looks good to me, modulo spacing between the title and the body.

Do you have a patch ?
Comment 3 Allan Day 2016-03-18 13:51:06 UTC
Looks good to me too. I know that spacing in these dialogs can be a bit inconsistent and tricky from a theme point of view, so wouldn't block on that personally.
Comment 4 Kalev Lember 2016-03-22 19:21:53 UTC
Sorry for the delay. Here are the gnome-shell patches; we also need some new PackageKit API for this, those are in a branch in https://github.com/kalev/PackageKit/tree/shell-system-upgrades
Comment 5 Kalev Lember 2016-03-22 19:22:27 UTC
Created attachment 324555 [details] [review]
endSessionDialog: Rename a variable
Comment 6 Kalev Lember 2016-03-22 19:22:34 UTC
Created attachment 324556 [details] [review]
endSessionDialog: Use new PackageKit DBus API
Comment 7 Kalev Lember 2016-03-22 19:22:43 UTC
Created attachment 324557 [details] [review]
endSessionDialog: Add support for system upgrades
Comment 8 Kalev Lember 2016-03-22 19:48:24 UTC
This adds two new strings, "Restart & Install Upgrade" and "%s %s will be installed after restart. Upgrade installation can take a long time: ensure that you have backed up and that the computer is plugged in."

I wonder if it would make sense to put these on the 3.20 branch now and ship them in the .1 release to give translators a bit of time to translate the new strings?
Comment 9 Florian Müllner 2016-03-22 20:22:51 UTC
Review of attachment 324555 [details] [review]:

LG, though it's not quite obvious why this is a separate patch
Comment 10 Florian Müllner 2016-03-22 20:23:13 UTC
Review of attachment 324556 [details] [review]:

Doesn't this break logging out when using PackageKit from 3.20.0?
Comment 11 Florian Müllner 2016-03-22 20:23:21 UTC
Review of attachment 324557 [details] [review]:

The concerns from the other patch regarding API breakage during a stable cycle apart, looks good to me with a string freeze exception

::: js/ui/endSessionDialog.js
@@ +738,3 @@
+        else if (this._type == DialogType.RESTART &&
+            this._pkOfflineProxy.UpgradeTriggered)
+            this._type = DialogType.UPGRADE_RESTART;

IMHO easier to read as:

if (this._type == DialogType.RESTART) {
    if (this._pkOfflineProxy.UpdateTriggered)
        this._type = DialogType.UPDATE_RESTART;
    else if (this._pkOfflineProxy.UpgradeTriggered)
        this._type = DialogType.UPGRADE_RESTART;
}
Comment 12 Kalev Lember 2016-03-23 09:38:47 UTC
hughsie is suggesting to use an a{sv} PreparedUpgrade property so that we wouldn't need separate PreparedUpgradeName and PreparedUpgradeVersion properies.
Comment 13 Kalev Lember 2016-06-21 14:21:34 UTC
OK, here's an updated patch that fixes the thing from comment #11 and adapts it to use the slightly modified PK API from https://github.com/hughsie/PackageKit/commit/305fca956dafd0c97897e35eee908e12f7f23de2
Comment 14 Kalev Lember 2016-06-21 14:22:11 UTC
Created attachment 330134 [details] [review]
endSessionDialog: Add support for system upgrades
Comment 15 Florian Müllner 2016-06-21 21:48:24 UTC
Review of attachment 330134 [details] [review]:

::: js/ui/endSessionDialog.js
@@ +142,3 @@
+        return _("%s %s will be installed after restart. Upgrade installation can take a long time: ensure that you have backed up and that the computer is plugged in.").format(distroName, distroVersion);
+    },
+    disableTimer: true,

Not sure about the "disable == true" pattern, could be split out in a separate patch

@@ +191,3 @@
         <arg type="s" name="action" direction="in"/> \
     </method> \
+    <method name="TriggerUpgrade"> \

This is unused
Comment 16 Kalev Lember 2016-06-22 07:34:09 UTC
(In reply to Florian Müllner from comment #15)
> Review of attachment 330134 [details] [review] [review]:
> 
> ::: js/ui/endSessionDialog.js
> @@ +142,3 @@
> +        return _("%s %s will be installed after restart. Upgrade
> installation can take a long time: ensure that you have backed up and that
> the computer is plugged in.").format(distroName, distroVersion);
> +    },
> +    disableTimer: true,
> 
> Not sure about the "disable == true" pattern, could be split out in a
> separate patch

Sure, I could instead do 'showTimer: true' in all others dialogs and 'showTimer: false' in this one, if you think that would be better?
Comment 17 zagortenay333 2016-06-27 10:22:58 UTC
Is there any way to show these dialogs arbitrarily? For themeing purposes. Same for that audio-jack dialog.
Comment 18 Kalev Lember 2016-06-30 09:41:50 UTC
(In reply to Florian Müllner from comment #15)
> Review of attachment 330134 [details] [review] [review]:

> @@ +191,3 @@
>          <arg type="s" name="action" direction="in"/> \
>      </method> \
> +    <method name="TriggerUpgrade"> \
> 
> This is unused

Ah indeed, thanks! Fixed.
Comment 19 Kalev Lember 2016-06-30 09:43:53 UTC
Attachment 324555 [details] pushed as db8f6b4 - endSessionDialog: Rename a variable
Attachment 324556 [details] pushed as 58a733d - endSessionDialog: Use new PackageKit DBus API
Attachment 330134 [details] pushed as ab68360 - endSessionDialog: Add support for system upgrades