Skip to content

Handle incompatible add-ons when upgrading/downgrading NVDA API versions#15439

Merged
seanbudd merged 10 commits into
betafrom
fixCompatUpgrade
Sep 20, 2023
Merged

Handle incompatible add-ons when upgrading/downgrading NVDA API versions#15439
seanbudd merged 10 commits into
betafrom
fixCompatUpgrade

Conversation

@seanbudd

@seanbudd seanbudd commented Sep 13, 2023

Copy link
Copy Markdown
Member

Link to issue number:

Fixes #15437
Fixes #15412
Fixes #15414

Summary of the issue:

There are several scenarios which need to be supported when updating/downgrading NVDA with incompatible add-ons

Test Name Upgrade from Upgrade to Test notes
Upgrade to different NVDA version in the same API breaking release cycle X.1 X.1 Add-ons which remain incompatible are listed as incompatible on upgrading. Preserves state of enabled incompatible add-ons
Upgrade to a different but compatible API version X.1 X.2 Add-ons which remain incompatible are listed as incompatible on upgrading. Preserves state of enabled incompatible add-ons
Downgrade to a different but compatible API version X.2 X.1 Add-ons which remain incompatible are listed as incompatible on downgrading. Preserves state of enabled incompatible add-ons
Upgrade to an API breaking version X.1 (X+1).1 All incompatible add-ons are listed as incompatible on upgrading, overridden compatibility is reset.
Downgrade to an API breaking version (X+1).1 X.1 Add-ons which remain incompatible listed as incompatible on downgrading. Preserves state of enabled incompatible add-ons. Blocked add-ons which are now compatible are re-enabled.

Description of user facing changes

  • NVDA will reset compatibility overrides when updating to a different API breaking release, this means incompatible add-ons will be blocked again.
  • If an add-on is blocked due to compatibility and becomes compatible, e.g. via downgrading, it will be re-enabled.

Description of development approach

Store the BACK_COMPAT_TO version in the addon state pickle.
When updating the BACK_COMPAT_TO version, reset the incompatibility override state.
When downgrading, re-check add-on compatibility

Testing strategy:

Refer to manual test plan.

Builds:
https://drive.google.com/file/d/1iSsnwp3yg4LRl3p0v0mVINyU8YDmVe1f/view?usp=sharing

Known issues with pull request:

There may be minor issues when downgrading to versions that don't include this patch from a version with this patch.
Nothing major has been discovered.

Change log entries:

Bug fixes

  • Fix issues with handling incompatible add-ons when upgrading NVDA

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • Security precautions taken.

@seanbudd seanbudd added this to the 2023.3 milestone Sep 13, 2023
@LeonarddeR

LeonarddeR commented Sep 13, 2023 via email

Copy link
Copy Markdown
Collaborator

@seanbudd

seanbudd commented Sep 13, 2023

Copy link
Copy Markdown
Member Author

This should not be affected - however I'd note that we generally don't support downgrades, and that the goal of this PR is to prevent add-ons from getting to a state where they are stuck as incompatible and blocked.

@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit e9f57c9983

@CyrilleB79

Copy link
Copy Markdown
Contributor

So maybe it would be nice to clarify (elsewhere than in comments of issues/PRs) what is supported or not while downgrading NVDA. E.g.:

  • NVDA does not fully support downgrade meaning that there is no guarantee that the configuration or the add-on states is kept while downgrading, e.g.:
    • a disabled add-on can be reenabled during downgrading process
    • an option could be restored to default or modified during downgrading process
  • NVDA tries to offer a limited support of downgrade so that it remains usable after downgrade, e.g.:
    • options that have been restored during a downgrade process can be modified again manually
    • add-ons that were disabled during a downgrade process can be reenabled or reinstalled or uninstalled

@wmhn1872265132

Copy link
Copy Markdown
Contributor

There are two cases of downgrade:

  1. Install the old version directly;
  2. Uninstall the new version through the uninstaller and then reinstall the old version.

In my opinion, the fact that NVDA does not support downgrading refers to the first situation. There should be better support for the second situation, at least there should be no serious errors or missing functions.
Since the configuration of NVDA can be called by multiple different versions of software through the command line, etc., this situation becomes more complicated.

@seanbudd seanbudd marked this pull request as ready for review September 20, 2023 00:01
@seanbudd seanbudd requested a review from a team as a code owner September 20, 2023 00:01
@seanbudd seanbudd requested review from michaelDCurran and removed request for a team September 20, 2023 00:01
michaelDCurran
michaelDCurran previously approved these changes Sep 20, 2023
@seanbudd seanbudd merged commit 8fd56cb into beta Sep 20, 2023
@seanbudd seanbudd deleted the fixCompatUpgrade branch September 20, 2023 02:06
seanbudd pushed a commit that referenced this pull request Oct 2, 2023
…or the pickling and unpickling process. (#15556)

Fixes #15554

Summary of the issue:
When pickling add-ons state it is necessary to use only builtin types, so that it can be loaded in older versions of NVDA. IN PR #15439 the backCompatToVersion was mistakenly pickled as a custom named tuple MajorMinorPatch.
This means it is impossible to start older versions of NVDA with the config from the latest beta NVDA crashes on startup.

Description of user facing changes
It should be once again possible to start older versions of NVDA with the config used with the latest beta.

Description of development approach
The manualOverridesAPIVersion is pickled as a standard tuple. To make sure similar regressions are not introduced I have added unit test covering various scenarios of converting state to, and loading it from, pickled data. Note that I have intentionally not used pickle files, since when the given custom data type is added to NVDA it will always be unpickled successfully.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants