introduse the ability to postpone a downloaded update #4263

nvaccessAuto · 2014-07-11T04:43:46Z

Reported by zahari_bgr on 2014-07-11 04:43
Hi,
currently, when one downloads an update when using installed copy, they have no other choice except to run the update within the current session.
This is not always the desired behaviour, cause the user may want to do other stuff before that and may need to restart either NVDA or the system. Redownloading is not very nice, cause not everyone have a persistant internet connection, not to menssion the bandwith and traffic.
I propose to be introduced a mechanism for postponing the install of the update and executing it on user's demand.

nvaccessAuto · 2014-07-11T04:59:06Z

Comment 1 by zahari_bgr on 2014-07-11 04:59
My proposal is, when download is finished, instead of the current notification, asking the user wether they want to install the update now or postpone it. A pending update then could be run through the last (otherwise hidden) option in the new Exit dialog (and if it is disabled, from a next to last item in the NVDA menu with the same name - "Run pending updates", which is also hidden if there aren't any pending updates).
I currently store the pending update files in the user config directory and delete them if there is a new pending update. They are also deleted after successfull installation - as usual.
This could be made configurable later, as requested in ticket #2255
To achieve this, I extracted the code for update execution from _downloadSuccess() to a separate function outside any class, and created two more functions askInstallUpdateNow() and isPendingUpdate(). isPendingUpdate() returns the path to a pending update (if any) and None otherwise. The path to a pending update is stored in the state list. executeUpdate() starts the actual update (either passed as a parameter or taken with call to isPendingUpdate()).
askInstallUpdateNow() returns the rezult of the message box asking the user wether they want to start the update now. Its text depends wether the exit dialog is enabled or not.
In _downloadSuccess(), I askInstallUpdate(), and if positive - executeUpdate(), else preserv the update installer file (If there is an error - I cover that) and delete old pending update file (if any).
Apart of the GUI elements, on startup, after NVDA's initialization, I notify the user that there is a pending update with askInstallUpdateNow(), and if yes - I do it.
Do you think this startup notification is not approprid and should be removed, or there should be an option in General settings "Notify for updates ready to be installed on startup", or to leave it as it is?
I currently don't cover the situation when an update is pending and the user does manual check for updates, i.e. I don't check the versions. However, an automatic check should not duplicate reports, since last downloaded update is also in state['dontRemindVersion']. In some situations one could want intentionaly to redownload the update though, so that should be also allowed.

I've created a branch named "t4263" in the following repository:
https://bitbucket.org/zahyur/nvda.git

This ticket originates from ticket #4057 and this branch is based on t4057. In ticket #4057 you will find an attached diff with the initial changes for this ticket.

nvaccessAuto · 2014-07-11T09:18:42Z

Comment 3 by briang1 on 2014-07-11 09:18
I like this idea. What happens if another update comes down while one is still pending? Does the new one over ride the old update, or would we have a kind of queue situation?
As i say I like the idea. I've often also wanted to perhaps share an update to save having to manually do it on several machines I might have. If its known where its stored it would be easy to simply grab the downloaded file and push it over the network to other machines.

nvaccessAuto · 2014-07-17T08:36:09Z

Comment 4 by zahari_bgr on 2014-07-17 08:36
Hi,
I've made some improvements here (I hope the following also answers to the @briang1's questions):

added an option for enabling/disabling the startup notification for pending updates. It is also disabled when NVDA was started with --launcher.
added an option for choosing the directory to store updates (defaults to userConfig/updates, which is created the first time you import this module)
-- the preserved updates are moved with changing the path
-- the preserved updates are not copyed into systemConfig along with other files in userConfig when copying userConfig to systemConfig
-- When setting the path in General Settings, you are forced to choose a valid, writable directory (you could also reset to default); if at the moment of moving the downloaded file it is not writable - you have the option of choosing another one; at last, the pending file is left in the temp directory.
added an option keepUpdatesFor which determins after how many days preserved updates should be deleted. This excludes the update which is currently pending to be installed. Defaults to 0, which meens that every applyed or discarded update will be deleted.

I've also added some documentation for the above options.

I now check the pending version against the new version when performing an update check. I've also changed a little the update notification GUI:

When there is a new version that is different than the pending version (or there is no pending update): "Download and install update" is now just "Download update"
When the latest version is already pending: You can install it from here or re-download.
The StaticText in this dialog corresponds to the current situation.

Finaly, I removed the checks for isInstalledCopy() and let UpdateDownloader() to be used for all update downloads - I think it is now safe and it will eliminate some confusion and browser issues.

I've created a test build from my testing branch with this and my other changes, which could be downloaded from:
https://bitbucket.org/zahyur/nvda/downloads/nvda_testing-2.exe
or
http://zahari.tk/nvda_testing-2.exe

When you check for updates from this snapshot, you will get nvda_testing-3.exe, which is exactly the same, except for the version number.

nvaccessAuto · 2014-07-18T04:43:44Z

Attachment pendingUpdates.diff added by zahari_bgr on 2014-07-18 04:43
Description:

nvaccessAuto · 2014-10-03T11:56:23Z

Comment 5 by blindbhavya on 2014-10-03 11:56
Hi.
Great idea, hope the code may be reviewed.
I will be CCing myself to this one for updates on this ticket's progress.

nvaccessAuto · 2015-05-29T20:19:07Z

Comment 6 by driemer.riemer@... on 2015-05-29 20:19
Editing again because I submitted early on accident.

Code review:

It looks like you disable everything but the edit box for a directory in secure mode. Shouldn't this be disabled as well?

                           _("The directory for storing downloaded NVDA updates not exists or is not writable. Please choose another one."),

Shouldn't this be

                           _("The directory for storing downloaded NVDA updates does not exist or is not writable. Please choose another one."),

It may also be more logical to explain this to the user in such words as you don't have the privlages to save updates to this location.

```
                                           if file.startswith("nvda_update_") and file.endswith(".exe"): 
```
My concern for this code is that someone might be able to maliciously put a file like nvda_virus.exe in the folder, with a virus in it. This code would then be run.
Since nvda now hashes and verifies updates we should check to make sure that nvaccess actually released this update.

           _("Update is ready to install.\nYou can install it now, or install it later from the exit dialog.\nInstall now?") if exitDialogEnabled else

           # Translators: A message presented when the update has been successfully downloaded asking the user to install or postpone.

```
           _("Update is ready to install.\nYou can install it now, or install it later from the NVDA menu.\nInstall now?"), 
```
Could this be reworked to be a bit more readable? I am not sure quite what is going on here because the ternary statement is a bit long winded.
```
                   with wx.DirDialog(gui.mainFrame, _("Store updates directory not writable. Select a writable one"), defaultPath=storeUpdatesDir) as d:
```
Could you extract the part of the with statement to a variable and use it in the with statement to make the code a bit more readable? It really doesn't matter either way, I don't know what the project recommends.

                           _("The downloaded file could not be moved to a safe location and could disappear on the next system restart.\n"

                                   "NVDA still can use it during this session, but if you want to preserv it, you can copy it yourselv from:\n"

```
                                   "%s" % self.destPath), 
```
Can you replace this with new python formatting syntax like {0} or {name}? I think this eases the translation process a bit from what I have heard. also yourselv should be yourself.
Also how will gettext treet \n? Why is there a " and then another " within the string?
+ if file.startswith("nvda_update_") and file.endswith(".exe") and (f != state["pendingUpdateFile"]) and (os.stat(f).st_mtime < time.time() - keepUpdatesFor * 86400):
Maybe put a comment here so people know what this number means.
+There is also a button called "Use default" for reverting this setting to its defaults, which is a folder called updates under the folder with your user configuration.
+If there are update files which have been stored in this folder and you change it - they will be moved to the new location.
Should the fact that this is disabled in secure mode be explained in the manual?

Great work.

zahyur · 2016-08-19T19:48:08Z

I'm so sorry - I haven't seen that there's code review. I don't mind if someone takes this over - I rarely use Windows those days. Same apply for my other contributions, which are still waiting code review. I'll be glad to see my ideas complemented by others and merged in NVDA, since I implemented them with the hope they'll be of help to users. Now while I'm thinking of it, there were also a few other update-related patches, and in one point I was thinking of separate Update preferences dialog. Of course, this is in case NVAccess approves those propositions, and probably should be a subject of another ticket.

derekriemer · 2016-08-19T20:58:47Z

Do you mind putting this into a pr? That way it can be officially review. Sent from a mobile device.
Please disregard errors as this is a smaller device.

On Aug 19, 2016, at 13:48, Zahari Yurukov notifications@github.com wrote:

I'm so sorry - I haven't seen that there's code review. I don't mind if someone takes this over - I rarely use Windows those days. Same apply for my other contributions, which are still waiting code review. I'll be glad to see my ideas complemented by others and merged in NVDA, since I implemented them with the hope they'll be of help to users. Now while I'm thinking of it, there were also a few other update-related patches, and in one point I was thinking of separate Update preferences dialog. Of course, this is in case NVAccess approves those propositions, and probably should be a subject of another ticket.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

zahyur · 2016-08-19T22:34:36Z

I'm not interested at this time.

LeonarddeR · 2016-08-20T12:04:13Z

We can't do any review on the current code as the attachment seems to be lost in the migration to github. @@zahyur, do you still have the original code somewhere?

zahyur · 2016-08-20T13:57:54Z

https://bitbucket.org/zahyur/nvda/
Under the t4263 branch.
Note that it's not up-to-date with the current master from NVAccess.

LeonarddeR · 2016-08-20T14:28:53Z

Thanks for this. I've pulled in the branch and tried to do a rebase to current master. Some little merge conflicts i've tried to fix as good as possible.
https://github.com/BabbageCom/nvda/tree/t4263

@derekriemer: Do I understand it correctly that you were willing to do a code review? I'd be happy to collaborate on getting this code into good shape, as I belief that it might be a very handy feature.

derekriemer · 2016-08-20T18:58:13Z

Note that I'm not actually authorized to do official code review. I was
just I guess thinking I did at the time, but now that we have pull
requests, that's how any changes get pulled in most efficiently.

On 8/20/2016 8:28 AM, Leonard de Ruijter wrote:

Thanks for this. I've pulled in the branch and tried to do a rebase to
current master. Some little merge conflicts i've tried to fix as good
as possible.
https://github.com/BabbageCom/nvda/tree/t4263

@derekriemer https://github.com/derekriemer: Do I understand it
correctly that you were willing to do a code review? I'd be happy to
collaborate on getting this code into good shape, as I belief that it
might be a very handy feature.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#4263 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFGivbpY-XBCVBGwSXiOKSJNhBs6ZET-ks5qhw8ngaJpZM4Joy5b.

Derek Riemer

Department of computer science, third year undergraduate student.
Proud user of the NVDA screen reader.
Open source enthusiast.
Member of Bridge Cu
Avid skiier.

Websites:
Honors portfolio http://derekriemer.com
Awesome little hand built weather app!
http://django.derekriemer.com/weather/

email me at derek.riemer@colorado.edu mailto:derek.riemer@colorado.edu
Phone: (303) 906-2194

derekriemer · 2016-08-20T18:58:54Z

But if you have questions about or disagree with what I said, just
ignore it as at the time I was a pretty novice coder.

On 8/20/2016 8:28 AM, Leonard de Ruijter wrote:

Thanks for this. I've pulled in the branch and tried to do a rebase to
current master. Some little merge conflicts i've tried to fix as good
as possible.
https://github.com/BabbageCom/nvda/tree/t4263

@derekriemer https://github.com/derekriemer: Do I understand it
correctly that you were willing to do a code review? I'd be happy to
collaborate on getting this code into good shape, as I belief that it
might be a very handy feature.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#4263 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFGivbpY-XBCVBGwSXiOKSJNhBs6ZET-ks5qhw8ngaJpZM4Joy5b.

Derek Riemer

Department of computer science, third year undergraduate student.
Proud user of the NVDA screen reader.
Open source enthusiast.
Member of Bridge Cu
Avid skiier.

Websites:
Honors portfolio http://derekriemer.com
Awesome little hand built weather app!
http://django.derekriemer.com/weather/

email me at derek.riemer@colorado.edu mailto:derek.riemer@colorado.edu
Phone: (303) 906-2194

LeonarddeR · 2016-08-24T17:38:34Z

I was just wondering whether you'd be interested in working together on this project to make it suitable to be included in official releases. That is, if the core developers are interested in including a sanitised pull request.

derekriemer · 2016-08-24T17:47:05Z

Sure.

LeonarddeR · 2016-09-03T11:56:36Z

Have looked into this again. I have the following thoughts:

Several gui messages will have to be redone, including those reported by @derekriemer. Example "Start previously downloaded updates, which are still not applyed"
To me, the possibility to choose a location for storing updates is a bit overkill. I think an updates directory in the userconfig is sufficient

jcsteh · 2016-09-03T23:22:09Z

Happy to support postponing of downloaded updates, but I don't think we should allow the user to choose the location at all. If a user wants to download a build for later use, this should be downloaded from the web, rather than relying on an auto update mechanism.

zahyur · 2016-09-04T10:39:25Z

Hi,
The idea of choosing the location was if there's a problem with the default location (it's not writable, there's not enough disk space, etc.).
The default temp directory on the system is not a good location, cause it's cleaned up on restart. Also - portable copies.

jcsteh · 2016-09-04T10:48:32Z

I should have been clearer: I agree the temporary directory isn't suitable once you can postpone updates. However, I still don't think this should be configurable, as it increases complexity and is the kind of configurability that might confuse users.

zahyur · 2016-09-04T12:25:14Z

Hi,
No, no - there's no temporary directory in this patch - it's a directory in the user's config directory - I was just clarifying why this is so.
My point was - what if the user's config directory isn't writable or there's not enough free space on the drive, where it recides.
One possibility is showing an error, and then the user is stuck.
I've just decided it's better user experiance to allow the user to change the download directory.
And if you can change the download directory, why you shouldn't be able to do it permanently?
It's just more cool for more people when they have a choice. If someone is confused - they just don't touch the thing they don't understand and use the defaults. And if they don't have a reason to change the defaults - excellent.
At the other end are Apple, for example, which say "we're smarter than you, we know exactly what is best for you, and you should buy it and use it". Well, I would never use an Apple product, not that it's ridiculously expensive, which is also a good reason, too.

Best wishes,
Zahari

LeonarddeR · 2016-09-04T18:59:52Z

I belief things should be as follows: 1. Downloads should be saved to the temp folder upon download. 2. If one decides to posphone, copy de download to the updates folder under userconfig 3. If the latter fails, tell the user that the download can't be saved and that he either should install directly or download again later.

derekriemer · 2016-09-04T19:08:05Z

I also agree that this is the better way to go.

On 9/4/2016 12:59 PM, Leonard de Ruijter wrote:

I belief things should be as follows:

Downloads should be saved to the temp folder upon download.

If one decides to posphone, copy de download to the updates folder
under userconfig

If the latter fails, tell the user that the download can't be saved
and that he either should install directly or download again later.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#4263 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFGivVV7zTfomlkEGGQApRAMevC33kv4ks5qmxUqgaJpZM4Joy5b.

Derek Riemer

Department of computer science, third year undergraduate student.
Proud user of the NVDA screen reader.
Open source enthusiast.
Member of Bridge Cu
Avid skiier.

Websites:
Honors portfolio http://derekriemer.com
Awesome little hand built weather app!
http://django.derekriemer.com/weather/

email me at derek.riemer@colorado.edu mailto:derek.riemer@colorado.edu
Phone: (303) 906-2194

zahyur · 2016-09-04T22:09:13Z

Hi,
As far as I can remember, it was something like that. You just want to remove the manual choosing of the directory.
I don't mind - I'm just sharing my opinion and my motivations. It was never ment to be something final - it's just how I see the world.
So feel free to express your opinion or change it how you think it fits best. It's what the FLOSS world is all about - as well as I understand it.
And BTW, that's one of the reasons I prefer code over an explanation - we all could understand each other. The other is that I prefer seeing the work done rather than just discussing.
And if I could go offtopic (as I do often recently) - I don't feel sorry for not a one peace of code I've ever wrote, cause I always learn something, and it always worths more than the actual thing.
So keep the good work, fight the good fight, and I hope people will love this feature. Just write code - the words doesn't matter in the software world.

Best wishes,
Zahari

jcsteh · 2016-09-04T22:09:51Z

I'm not sure I see a need to use the temp dir at all, as copying for postpone seems rather pointless. Eventually, we'll probably want an option to do automatic background downloads which would make the temp dir even less viable.

LeonarddeR · 2016-09-05T04:31:36Z

For portable copies on flash drives, no temp dir would mean that the download is saved to the flashdrive and than opened from it. In the case of a direct download-update, this would mean that there is a decrease in performance when compared to an update from an installer on the local drive.

jcsteh · 2016-09-05T04:40:44Z

True, but right now, we don't do update downloads for portable copies anyway.

LeonarddeR · 2016-09-05T04:59:10Z

The current code for this ticket allows for this. It downloads and than starts with the --launcher parameter instead of --install-silent

LeonarddeR · 2016-09-05T05:45:11Z

How about the possibility to config how long updates are to be kept?
Honestly, I also tend to remove this and simply choose a default.
Probably just keep a postponed update until either a new one is
downloaded or the particular update has been installed.

jcsteh · 2016-09-05T06:04:44Z

Agreed; just keep until installed or next one is downloaded.

bhavyashah · 2017-08-06T10:32:15Z

@jcsteh @LeonarddeR Could you please update us about the status of the code review for this feature?

* initial work on postponeing updates * Removed undesired code from the source and userdocs and added some copyright notices * Review actions * Some major code updates to make this actually work, hopefully * Fixed translator comment * Some more fixes, still some testing to be done * Support updating of portable copies * Review actions * More review actions * Actions for user guide * Fixed issue where a right paren was placed at the wrong position * Properly evaluate whether to show or hide the pending updates menu item on every time the menu is opened. Binding this evaluation to the item itself made the item never to show up. * Don't use the file variable name when looping through possible old updates. This broke the loading of the state file. Added state file loading debug logging while at it. * Properly destroy the UpdateResultDialog when installing a postponed update * When updating a portable copy, run NVDA with the -m command line argument * Fix KeyError on updating * make sure that the update state file will be updated to contain pending update info * When updating a portable copy, place paths that are provided as command line params between quotes to deal with paths containing spaces * Fix crash after restart with a postponed update * No longer add removeFile to the update state, as it is used nowhere * Call prePopup before raising the UpdateAskInstallDialog and make it modal * Use runScriptModalDialog for the UpdateAskInstallDialog * Clear the don't remind version when postponing an update. If we do not do this, a user will never be reminded about a postponed update automatically at all

nvaccessAuto added enhancement feature/update-check blocked/needs-code-review labels Nov 10, 2015

nvaccessAuto assigned jcsteh Nov 10, 2015

zahyur mentioned this issue Aug 19, 2016

Allowing automatic updates of portable copies #6281

Closed

LeonarddeR mentioned this issue Sep 3, 2016

Added the possibility to create a portable copy with the use of command line parameters #6329

Merged

LeonarddeR mentioned this issue Sep 6, 2016

Introduse the ability to postpone a downloaded update #4263 #6355

Merged

LeonarddeR mentioned this issue Nov 12, 2016

Split out downloader used by updateCheck so it can be used elsewhere #3504

Open

jcsteh mentioned this issue Aug 14, 2017

Allow the automatic check frequency of updates to be set #2353

Closed

jcsteh removed their assignment Sep 5, 2017

LeonarddeR mentioned this issue Feb 9, 2018

Check for updates: an instance of this dialog is shown each time a user selects Check for Updates from NVDA help menu #4579

Open

michaelDCurran closed this as completed in #6355 Mar 28, 2018

nvaccessAuto added this to the 2018.2 milestone Mar 28, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

introduse the ability to postpone a downloaded update #4263

introduse the ability to postpone a downloaded update #4263

nvaccessAuto commented Jul 11, 2014

nvaccessAuto commented Jul 11, 2014

nvaccessAuto commented Jul 11, 2014

nvaccessAuto commented Jul 17, 2014

nvaccessAuto commented Jul 18, 2014

nvaccessAuto commented Oct 3, 2014

nvaccessAuto commented May 29, 2015

zahyur commented Aug 19, 2016

derekriemer commented Aug 19, 2016

zahyur commented Aug 19, 2016

LeonarddeR commented Aug 20, 2016

zahyur commented Aug 20, 2016

LeonarddeR commented Aug 20, 2016

derekriemer commented Aug 20, 2016

derekriemer commented Aug 20, 2016

LeonarddeR commented Aug 24, 2016 via email

derekriemer commented Aug 24, 2016

LeonarddeR commented Sep 3, 2016

jcsteh commented Sep 3, 2016 via email

zahyur commented Sep 4, 2016

jcsteh commented Sep 4, 2016 via email

zahyur commented Sep 4, 2016

LeonarddeR commented Sep 4, 2016 via email

derekriemer commented Sep 4, 2016

zahyur commented Sep 4, 2016

jcsteh commented Sep 4, 2016 via email

LeonarddeR commented Sep 5, 2016 via email

jcsteh commented Sep 5, 2016 via email

LeonarddeR commented Sep 5, 2016 via email

LeonarddeR commented Sep 5, 2016

jcsteh commented Sep 5, 2016 via email

bhavyashah commented Aug 6, 2017

introduse the ability to postpone a downloaded update #4263

introduse the ability to postpone a downloaded update #4263

Comments

nvaccessAuto commented Jul 11, 2014

nvaccessAuto commented Jul 11, 2014

nvaccessAuto commented Jul 11, 2014

nvaccessAuto commented Jul 17, 2014

nvaccessAuto commented Jul 18, 2014

nvaccessAuto commented Oct 3, 2014

nvaccessAuto commented May 29, 2015

zahyur commented Aug 19, 2016

derekriemer commented Aug 19, 2016

zahyur commented Aug 19, 2016

LeonarddeR commented Aug 20, 2016

zahyur commented Aug 20, 2016

LeonarddeR commented Aug 20, 2016

derekriemer commented Aug 20, 2016

derekriemer commented Aug 20, 2016

LeonarddeR commented Aug 24, 2016 via email

derekriemer commented Aug 24, 2016

LeonarddeR commented Sep 3, 2016

jcsteh commented Sep 3, 2016 via email

zahyur commented Sep 4, 2016

jcsteh commented Sep 4, 2016 via email

zahyur commented Sep 4, 2016

LeonarddeR commented Sep 4, 2016 via email

derekriemer commented Sep 4, 2016

zahyur commented Sep 4, 2016

jcsteh commented Sep 4, 2016 via email

LeonarddeR commented Sep 5, 2016 via email

jcsteh commented Sep 5, 2016 via email

LeonarddeR commented Sep 5, 2016 via email

LeonarddeR commented Sep 5, 2016

jcsteh commented Sep 5, 2016 via email

bhavyashah commented Aug 6, 2017