I think the wording of this could be better, first I want to double check I understand correctly that this message will be shown after NVDA has confirmed that the pending installation is in fact the latest available update? If so, I think the # Translators: comment could be clearer also, perhaps A message indicating that the latest version of NVDA has been downloaded and is waiting to be installed

Since we specify the version when we specify that the availability of the update, I think we should do this here also, so perhaps something like NVDA version {version} is pending installation or NVDA version {version} is ready to be installed

This building up of messages has a risk that it won't make sense once translated. I also think these labels could be simplified. I think it is adequate to simply set the label as "Update is ready to install" and rely on the presence of the buttons to indicate what options are available.

I would word this "Unable to postpone update"

This is missing a translator comment. I think there is an automated test that would catch this, you can run this with scons checkPot

I think it's worth logging something here at debug level.

What's with the -2. Is this the position. Might be clearer to put this in a pendingUpdateMenuItemPosition variable. Is it important for it to be second from the bottom? I am concerned that because this positioning is separated from the rest of that menu filing that the order might change, and this code wont be considered. The index of the position for this item could be saved at the point where the rest of the menu is populated.

perhaps a comment to say why exe files should be skipped?

this looks like a bug, here exeCuteParams has a capital C, and where it is passed to ShellExecute it does not.

I have a feeling that this dialog wont have the visual spacing expected between labels and buttons. What is the easiest way to test this, so I can inspect it?

I added some guiHelper stuff to it now. Here is a code snippet to make it show up:

import versionInfo, gui
versionInfo.updateVersionType="snapshot:next"
import updateCheck
updateCheck.initialize()
updateCheck.UpdateAskInstallDialog(gui.mainFrame, "dummy_path", "2018.3")

Thanks for the snippet. It looks ok to me.

Looks like this can be removed?

Should this be uncommented / removed?

This has already been removed. Probably legacy code from the initial author, but storeUpdatesDir doesn't have to be changed inside this function.

Oh sorry I must have misread this line.

Elsewhere we say "Install update" rather than "run update", I think "Install pending update" would be more consistent.

Do you think this dialog should tell you the updates version information? To be consistent with the other download update dialog?

There are a few places here that still refer to "run pending updates"

also refers to "run the pending update"

There is also a typo, a space needs to move one character over in Hel pmenu

what's this about?

If you call this, I think you also need to call postPopup().

Yes, you'd think so. The use of prePopup and postPopup inside updateCheck really puzzles me.

1. prePopup is only used in the UpdateDownloader, not for the other dialogs. It was called for the old update installation message box, so that's why I belief it should be called in the UpdateAskInstallDialog as well.
2. postPopup is even more confusing given it is being called in a wx.CallLater with a not very descriptive comment. A git blame points at @jcsteh, so may be he knows more?
3. In several places in the gui module, you see the following code:

mainFrame.prePopup()
dialog.Show()
mainFrame.postPopup()

However, this doesn't seem to make much sense, as Show doesn't block the caller. For ShowModal, it does make sense as ShowModal blocks the caller until the dialog is dismissed.

Perhaps its better to use def runScriptModalDialog in source/gui/__init__

TL;DR: The general rule is that if you're popping up a window (as opposed to a window which appears as a result of some action performed while our GUI was already visible), you must call prePopup. If you call prePopup, there must be a corresponding call to postPopup. Ideally, this postPopup call would occur once the window is dismissed, but for windows that aren't modal, it's enough in most cases to just call it after Show.

prePopup and postPopup have two major functions:

1. To force Windows to bring us to the foreground (prePopup) and to return to the previous foreground (postPopup). The way foreground stuff works in Windows is really obscure. prePopup mostly makes sense; we ask Windows to bring us to the foreground. However, we were seeing cases where the user would just be stuck in limbo when our pop-ups were dismissed. I still don't really understand why this is the case, but the show/hide in postPopup seemed to fix it. (Obviously, that doesn't actually get applied at the right tie for dialogs that aren't modal, but it seems to behave regardless.) This code is very old, wx has been updated and Windows XP is no longer a factor, so you could consider re-assessing to see whether we can get rid of this ugliness. However, note that installed versus portable is still a factor, since uiAccess gives the app special privileges regarding the foreground window.
2. These methods also set things up so that NVDA is aware of what the focus was before the pop-up. This is important for input gestures, for example, so it knows which object to get gestures for. postPopup does the necessary teardown of this state. The fact that it gets called immediately after Show is fine in this case because the dialog should already have done what it needs to do with this state.