nvaccess / nvda

NVDA, the free and open source Screen Reader for Microsoft Windows
Other
2.08k stars 625 forks source link

Addon store restart dialog is overlaping installtasks dialog #15613

Closed beqabeqa473 closed 9 months ago

beqabeqa473 commented 11 months ago

Steps to reproduce:

Make a simple addon and in installtasks.py, in method onInstall try to show some informational message. install it from addon store as you install an addon from available addons tab, not with install external button

Actual behavior:

When pressing close button, dialog is shown but shortly is overlapped with NVDA's restart dialog

Expected behavior:

Dialog should stay alive untill it is closed.

NVDA logs, crash dumps and other attachments:

System configuration

NVDA installed/portable/running from source:

Installed

NVDA version:

Version: alpha-29601,f1c73d1d (2024.1.0.29601)

Windows version:

Windows 10 22H2 (AMD64) build 19045.3448

Name and version of other software in use when reproducing the issue:

Other information about your system:

Other questions

Does the issue still occur after restarting your computer?

Yes

Have you tried any other versions of NVDA? If so, please report their behaviors.

All versions which include addon store

If NVDA add-ons are disabled, is your problem still occurring?

Does the issue still occur after you run the COM Registration Fixing Tool in NVDA's tools menu?

CyrilleB79 commented 11 months ago

@beqabeqa473, it's not possible to use a simple test add-on from the add-on store (since it should actually be submitted to the store). Except of course if you have performed a test hack to mimic the add-on store. If you have performed such hack, could you please indicate the steps?

If instead you have tested with a real add-on from the store, could you indicate which one?

beqabeqa473 commented 11 months ago

Forexample instantTranslate or mouseWheelScrolling.

The problem is that this dialog can be very important, for example connected with migration process from one data storage method to another, and in this case user can lose their settings.

On 10/12/23, Cyrille Bougot @.***> wrote:

@beqabeqa473, it's not possible to use a simple test add-on from the add-on store (since it should actually be submitted to the store). Except of course if you have performed a test hack to mimic the add-on store. If you have performed such hack, could you please indicate the steps?

If instead you have tested with a real add-on from the store, could you indicate which one?

*

-- Reply to this email directly or view it on GitHub: https://github.com/nvaccess/nvda/issues/15613#issuecomment-1759490457 You are receiving this because you were mentioned.

Message ID: @.***>

-- with best regards Beqa Gozalishvili Tell: +995593454005 Email: @.*** Web: https://gozaltech.org Skype: beqabeqa473 Telegram: https://t.me/gozaltech facebook: https://facebook.com/gozaltech twitter: https://twitter.com/beqabeqa473 Instagram: https://instagram.com/beqa.gozalishvili

seanbudd commented 11 months ago

This should be fixed with #14430 - where in 2024.1 we can run the install tasks after NVDA restarts, instead of before.

beqabeqa473 commented 11 months ago

It is very bad decision.

installTasks are ment to execute on addon install/uninstall, and if i restart NVDA and installTasks will call onInstall apart with initializing addon, this will break a logic, where for example i need to make very serious changes, for example i am writing a synthdriver, where dictionary datastore was changed, it would be easier to make migration on installation rather than in the addon code itself. I don't need code which will be executed only once to be in the main addon code. InstallTasks wasn't designed that way. Please, reconsider this decision, as it will have its drawbacks.

On 10/13/23, Sean Budd @.***> wrote:

This should be fixed with #14430 - where in 2024.1 we can run the install tasks after NVDA restarts, instead of before.

-- Reply to this email directly or view it on GitHub: https://github.com/nvaccess/nvda/issues/15613#issuecomment-1760660720 You are receiving this because you were mentioned.

Message ID: @.***>

-- with best regards Beqa Gozalishvili Tell: +995593454005 Email: @.*** Web: https://gozaltech.org Skype: beqabeqa473 Telegram: https://t.me/gozaltech facebook: https://facebook.com/gozaltech twitter: https://twitter.com/beqabeqa473 Instagram: https://instagram.com/beqa.gozalishvili

seanbudd commented 9 months ago

@lukaszgo1 - based on your previous comment, do you still plan to fix this? would you like to be assigned to this issue?

Do you mind outlining how you plan to fix this? I can't imagine this being possible without add-on authors notifying NVDA when their blocking install task ends.

seanbudd commented 9 months ago

I can't imagine this being possible without add-on authors notifying NVDA when their blocking install task ends.

Mick suggested passing in a resource handler so add-on authors can use with blockingInstallTask: syntax

lukaszgo1 commented 9 months ago

Sorry for the delay. In general doing this without forcing add-on authors to notify when install tasks are done would be ideal, and from my testing seems pretty doable. Using the same approach as when installing the add-on from a external bundle that is executing the installation in the call to gui.ExecAndPump, which ensures that the restart dialog is shown only after installation of the add-on is done. I have some intermittent failures with this approach, but that is certainly due to issues in my prototype, rather than the design idea being flawed, so I'll submit a PR for this either today or tomorrow.