New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow addons to specify minimum compatible version of NVDA #6275

Closed
ctoth opened this Issue Aug 18, 2016 · 12 comments

Comments

Projects
None yet
7 participants
@ctoth

ctoth commented Aug 18, 2016

With the @script decorator and more upcoming changes that I hope to introduce to the addon system, it will be necessary for addons to specify what versions of NVDA they support so as to be able to alert users at installation time that their NVDA is not compatible.
I recommend a minimum_version key in the manifest.
Obviously this will not help existing versions of NVDA and addons, but the sooner we add it, the sooner it will be useful in future :)

@jcsteh

This comment has been minimized.

Contributor

jcsteh commented Aug 18, 2016

@josephsl

This comment has been minimized.

Collaborator

josephsl commented Aug 18, 2016

CC @derekriemer, @nvdaes, @jcsteh and other add-on writers.

@josephsl

This comment has been minimized.

Collaborator

josephsl commented Aug 18, 2016

Hi,

Blocked by #6205.

Design:

  1. For consistency reason, I propose calling it addon-minNVDAVersion or something short.
  2. The manifest value will specify version string like year.major.minor. I think build should be optional, or if 0, it should specify that it is for stable NVDA versions.
  3. The earliest opportunity to check for minimum version requirement is right before NVDA checks for existence of an earlier version of the add-on. I propose adding an error message if the requirement isn't met and abort, with a RuntimeError being generated and logged.
  4. We need to talk to add-ons community in regards to readiness of some add-ons to test it (two or three add-ons should be selected as canaries).

Thanks.

@josephsl

This comment has been minimized.

Collaborator

josephsl commented Aug 18, 2016

Hi,

Few issues and questions to consider:

  • Rollback: user installs NVDA Y, then installs add-on A, then downgrades to NVDA X that makes the installed add-on incompatible, and suppose both X and Y use this feature. Should add-on handler recognize this and stop the incompatible add-on from running when NVDA starts?
  • Disabled flag: suppose the above question is solved by doing a startup check, and the add-on is told to run. Is "suspended" the appropriate status for this add-on?

Thanks.

@nvdaes

This comment has been minimized.

Contributor

nvdaes commented Aug 18, 2016

Hi, I'm thinking what happens with add-ons like a Word add-on, that
become "bad" since NVDA adds the same keystroke for features. Then,
should be the minVersion a needed field, instead of just a
possibility?
Advantages: This could prevent the ussage of obsolete add-ons.
Disadvantages: Some add-ons would need to be updated.
I think that NVDA should suspend add-ons whose min version is
incompatible. Then the user could search an updated version or see the
add-on help, uninstall or even enable the add-on for testing.
Thanks.

@leonardder

This comment has been minimized.

Collaborator

leonardder commented Feb 10, 2018

This has been abandoned for a little while now. Is someone intending to implement this?

@leonardder leonardder added the feature label Feb 10, 2018

@josephsl

This comment has been minimized.

Collaborator

josephsl commented Feb 10, 2018

@michaelDCurran

This comment has been minimized.

Contributor

michaelDCurran commented Feb 12, 2018

@leonardder

This comment has been minimized.

Collaborator

leonardder commented Feb 13, 2018

I propose minimumNVDAVersion and lastTestedNVDAVersion in order not to create ambiguity with add-on version numbers itself. Both minimum and required is not scrtictly necessary I belief. Anyhow, I'm happy to take this issue.

@leonardder

This comment has been minimized.

Collaborator

leonardder commented Feb 13, 2018

While looking at the addonHandler code, I noticed that the Addon object has quite a few magic properties while not inheriting from AutoPropertyObject. Is there a particular reason for this?

@michaelDCurran

This comment has been minimized.

Contributor

michaelDCurran commented Feb 13, 2018

@jcsteh

This comment has been minimized.

Contributor

jcsteh commented Feb 14, 2018

@nvaccessAuto nvaccessAuto modified the milestone: 2018.4 Dec 6, 2018

feerrenrut added a commit that referenced this issue Dec 6, 2018

Allow add-ons to supply version compatibility information in manifest…
…s, and use this information to disable incompatible add-ons (PR #8006)

Closes #6275

* Allow setting minimum required NVDA version and last tested NVDA version in an add-on manifest, and use them to block add-ons from loading if desired.
* Move the version info retrieval code from appModuleHandler to fileUtils
* moved addonHandler into a package
* Fix scaling issues in the addon gui
* GUI additions:
   * Show unknown compatibility addons dialog on startup if there are addons already installed that are untested.
   * When attempting to install an incompatible / untested addon, a prompt is shown to block / confirm the installation.
   * When attempting to install a new version of NVDA with untested addons present, the user is warned that these addons will be disabled after installation. The untested addons can be listed, and any that should not be disabled can be flagged.
* During install of NVDA the user must check a box to confirm that they understand that their nvda installation contains addons that may not have been tested and will be disabled after the installation of NVDA.
* Addon manager GUI changes:
  * Buttons that relate to the currently selected add-on are arranged vertically to the right of the add-on list
  * Buttons that relate to the addon manager more generally are arranged horizontally at the bottom of the dialog.
* Fixed a bug in AutoWidthColumnCheckListCtrl which was stopping NVDA from being able to exit
* Update userguide and developerGuide
* Update changes file for #8006
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment