nvda icon indicating copy to clipboard operation
nvda copied to clipboard

Addon store

Open feerrenrut opened this issue 1 year ago • 12 comments

This is intended to act as a base for "stacked pull requests" to build up this feature until it is considered ready to merge into NVDA.

Link to issue number:

https://github.com/nvaccess/nvda/issues/3208

Summary of the issue:

Historically, NVDA users have had to browse websites or use add-ons, not directly run/developed by NV Access, in order to learn about, download, and install add-ons. For a long time, easier manual updates for add-ons, and automatic updates for add-ons has been desired by NVDA users. Additionally, with the more common and structured approach to add-on API compatibility breaks, lowering the friction for delivery of updated (and compatible) add-ons is more important than ever.

User stories enabled by this work:

  • As an NVDA user, I want to easily browse add-ons which are compatible with my version of NVDA, to learn if they may fulfill an unmeet user need.
  • As an NVDA user, I want to be able to install compatible NVDA add-ons that I have determined will fulfill an unmet user need.

Additionally, this work unblocks the following user stories which may yet be incorporated into this PR:

  • As an NVDA user, I want to be able to browse the add-on catalog while offline, I understand I will not be able to install add-ons.
  • As an NVDA user, I want to be able to manually check if there are any updates available for add-ons I have installed, so that I know if there are newer versions available.
  • As an NVDA user, I want to be notified when there are updates to add-ons I have installed, so that I can manually update the add-ons when I am ready to do so.
  • As an NVDA user, I want NVDA to automatically keep add-ons up to date with the latest version available.
  • When NVDA notifies of an update (to NVDA), I want to be able to check the compatibility status of the add-ons I have installed, so that I can determine whether the NVDA update will break my core requirements.

Description of how this pull request fixes the issue:

A new dialog is introduced (NVDA menu, tools, addon store) which shows the latest compatible versions of add-ons (submitted to the add-on store).

  • Filter by text field, allowing filtering Add-ons by phrases that appear in any field.
  • List of "available add-ons" with columns:
    • Name, Version, Publisher, Status
    • Status indicates: available, downloading, downloaded etc.
    • The application key (context menu) will present a menu with the actions available for the currently selected add-on. Currently only "install".
  • Read-only edit "description", containing the description of the add-on provided by the author.
  • Action buttons for the addon, currently only "Install".
  • Read-only edit "details", containing extended details for the add-on
    • Publisher, Version, Channel, Homepage (link), License (link), Source Code (link)

Installation:

  • When installing an add-on, the add-on is downloaded in the background.
  • The user is able to continue browsing add-ons, and even queue further add-ons for download / installation.
  • When the user attempts to exit the add-on store, the add-ons are installed and the user is prompted to restart NVDA.
  • If the user attempts to exit the add-on store before the add-ons have completed downloading, the user is told how many add-ons are still downloading and asked if they wish to cancel the download of those add-ons. If they choose to cancel the still in progress downloads any add-ons that have already been downloaded are installed.

An image of the add-on store dialog: image

Testing strategy:

This system is designed to make replicating unusual occurrences easy for developers, strategies that are helpful:

  • Update the timestamp in a cached "available addons" json file to within 6 hours of current time. It will be used instead of fetching fresh data from the Add-on store API. This allows for testing variations of data provided by the API, including add-on versions that it shouldn't supply.
  • Use a simple python http server to supply add-on files hosted locally, this server can be accessed through a local proxy tool (E.G. Clumsy) to replicate varying network conditions. Ensure to update the download URL in the cached "available addons" json file to do so.
  • Modify the manifests of the *.nvda-addon files being supplied to replicate various outcomes.

Many edge cases have been tested this way, however we will be relying on real world usage from alpha users to find edge cases we haven't yet considered.

Known issues with pull request:

  • Versions of NVDA that have not been added to nvdaAPIVersions.json will not be able to query the API.
    • Until this is resolved, work around the issue for testing purposes by overriding currentVersion in _getCurrentApiVersionForURL (source/addonStore/dataManager.py) to (2022, 2, 0)
    • Further explanation on how to override the value.
  • Currently the add-on store dialog can only be used for browsing and installing add-ons.
    • Manual and automatic update is planned.
    • We intend to integrate all functionality from the add-on manager, eventually removing the add-on manager dialog.
    • It will continue to be possible to "side-load" add-ons.
  • Installed add-ons are not yet shown in the add-on store. However, once installed they are no longer listed unless the latest version available (of the add-on) has changed.
  • There is no support for "dev" add-ons (distributing pre-stable add-ons, targeting a future version of NVDA)
    • This could be resolved with https://github.com/nvaccess/addon-datastore/issues/40
  • Currently shows all add-ons. There is no way to select only stable / beta / all.
    • There are plans to address this with a new configuration panel for the add-on store.
  • There are 'ok' / 'cancel' buttons, despite no difference in behavior. A single button "done" is more appropriate.
  • Pressing enter on a add-on list item should open the context menu, where actions for that add-on are found.
  • Pressing enter on a link (in the details) should open the URL.
  • The user experience could be improved if add-ons were installed (and the add-on install task ran) when NVDA next restarts. However there is a risk this may break assumptions by add-on authors, so will considered a compatibility breaking change. This will be scheduled for the next compat breaking release.
  • User documentation is still missing for this change, due to evolving state of the UI it will be written when the approaching the final stages of the feature.

Change log entries:

New features

An add-on store has been introduced into NVDA.
  - This allows browsing / searching for community add-ons.
  - Installing add-ons

Code Review Checklist:

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

feerrenrut avatar Aug 04 '22 08:08 feerrenrut

It's a very good news to see this topic arrive in NVDA's repo!

The PR is not in draft. Is it already testable?

I get the following error:

ERROR - external:addonStore.dataManager.DataManager._getLatestAvailableAddonsData (11:25:34.047) - getAddonData (13724):
Unable to get data from API (https://nvaccess.org/addonStore/en/all/2022.4.0.json), response (400): b'Addon API version not available'
ERROR - stderr (11:25:34.104) - getAddonData (13724):
Exception in thread getAddonData:
Traceback (most recent call last):
  File "C:\Users\Cyrille\AppData\Local\Programs\Python\Python37-32\lib\threading.py", line 926, in _bootstrap_inner
    self.run()
  File "C:\Users\Cyrille\AppData\Local\Programs\Python\Python37-32\lib\threading.py", line 870, in run
    self._target(*self._args, **self._kwargs)
  File "C:\Users\Cyrille\Documents\DevP\GIT\nvda\source\gui\addonStoreGui\viewModels.py", line 569, in _getAddonsInBG
    addons = self._dataManager.getLatestAvailableAddons()
  File "C:\Users\Cyrille\Documents\DevP\GIT\nvda\source\addonStore\dataManager.py", line 222, in getLatestAvailableAddons
    return self._availableAddonCache.availableAddons
AttributeError: 'NoneType' object has no attribute 'availableAddons'

CyrilleB79 avatar Aug 04 '22 09:08 CyrilleB79

@CyrilleB79 theoretically it should be testable.

However, I had previously been testing it before the NVDA version change, and with a prior datastore cache this error doesn't cause a major issue. I'll look into improving the robustness in this case.

There is a known issue to allow support for "in development" NVDA versions on the API side.

For testing purposes, you can safely override currentVersion in _getCurrentApiVersionForURL (source/addonStore/dataManager.py):

def _getCurrentApiVersionForURL() -> str:
	#currentVersion = addonAPIVersion.CURRENT
	currentVersion = (2022, 2, 0)

feerrenrut avatar Aug 04 '22 09:08 feerrenrut

  • FAIL: Translation comments check. Translation comments missing or unexpectedly included. See build log for more information.
  • PASS: Unit tests.
  • PASS: Lint check.
  • FAIL: System tests. See test results for more information.
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/dgmjah06qiow0bb9/artifacts/output/nvda_snapshot_pr13985-26096,8584ece3.exe

See test results for failed build of commit 8584ece3ae

AppVeyorBot avatar Aug 04 '22 09:08 AppVeyorBot

Thanks. Done and the first error is actually gone.

Now, I have no previous dataStore cache and this seems to cause another issue:

ERROR - stderr (12:13:30.476) - getAddonData (8340):
Exception in thread getAddonData:
Traceback (most recent call last):
  File "C:\Users\Cyrille\AppData\Local\Programs\Python\Python37-32\lib\threading.py", line 926, in _bootstrap_inner
    self.run()
  File "C:\Users\Cyrille\AppData\Local\Programs\Python\Python37-32\lib\threading.py", line 870, in run
    self._target(*self._args, **self._kwargs)
  File "C:\Users\Cyrille\Documents\DevP\GIT\nvda\source\gui\addonStoreGui\viewModels.py", line 569, in _getAddonsInBG
    addons = self._dataManager.getLatestAvailableAddons()
  File "C:\Users\Cyrille\Documents\DevP\GIT\nvda\source\addonStore\dataManager.py", line 217, in getLatestAvailableAddons
    self._cacheAddons(addonData=decodedApiData, fetchTime=fetchTime)
  File "C:\Users\Cyrille\Documents\DevP\GIT\nvda\source\addonStore\dataManager.py", line 191, in _cacheAddons
    with open(self._cacheFile, 'w') as cacheFile:
FileNotFoundError: [Errno 2] No such file or directory: 'C:\\Users\\Cyrille\\Documents\\DevP\\GIT\\nvda\\source\\userConfig\\addonStore\\_cachedLatestAvailableAddons.json'

CyrilleB79 avatar Aug 04 '22 10:08 CyrilleB79

@CyrilleB79 The issue creating the cache file should now be resolved. You'll still need to override the currentVersion until we resolve this on the server. We haven't decided on the best way to handle pre-release NVDA.

feerrenrut avatar Aug 04 '22 10:08 feerrenrut

Hi,

The PR has a lot of potential.

Updating add-ons (in the future): at least the PR recognizes when updates are available. Would it be possible to present current installed version somewhere so users can compare current versus update version, similar to Add-on Updater implementation?

Also, I don't know if it is me, but when downloading add-ons, some kind of a dialog is briefly shown.

Thanks.

josephsl avatar Aug 04 '22 15:08 josephsl

Here's feedback from me:

  1. Installation is fast and very comfortable, even when various add-ons have been selected.
  2. I'd like that Install button has a shortcut, and even that multiple add-ons can be selected, for example with a list of checkboxes.
  3. When the install button is pressed, it would be nice if the add-ons list was focused instead of further details.
  4. Some add-ons perform onInstall tasks, and the add-on name maybe presented in this cases, or this may be documented so that authors mention it explicitly. For example, clipContentsDesigner and reportPasswords.

Congrats for this pull request. It's really comfortable how ad-ons are downloaded and installed.

nvdaes avatar Aug 04 '22 19:08 nvdaes

Thanks for the feedback.

@josephsl

Updating add-ons (in the future): at least the PR recognizes when updates are available. Would it be possible to present current installed version somewhere so users can compare current versus update version, similar to Add-on Updater implementation?

Our next phase will be looking at the updating use-cases more carefully. Hopefully this will address these concerns.

Also, I don't know if it is me, but when downloading add-ons, some kind of a dialog is briefly shown.

Can you get me a log or some information about the dialog? I haven't noticed this, and don't know what it could be.

@nvdaes

I'd like that Install button has a shortcut, and even that multiple add-ons can be selected, for example with a list of checkboxes.

Note you can access "actions" for the selected add-on via the application/context menu. I intend to open this menu with the enter key is press while an add-on is selected. This is still a todo item. When we are confident about the full list of actions we can also add in accelerator keys.

When the install button is pressed, it would be nice if the add-ons list was focused instead of further details.

Is the concern about how long it takes to get back to the list, perhaps an accelerator key (for the list) would be sufficient?

Some add-ons perform onInstall tasks, and the add-on name maybe presented in this cases, or this may be documented so that authors mention it explicitly. For example, clipContentsDesigner and reportPasswords.

Yes, this is a concern. It isn't always obvious which add-on is presenting a dialog. We intent to move the install tasks running to after NVDA is restarted, which will make this worse. This might resolve itself through add-on updates. Let's give it some time, if the problem still exists when we get to that stage we can look into providing more information about which add-on install is completing.

feerrenrut avatar Aug 05 '22 03:08 feerrenrut

Hi, upon further testing, looks like there is no separate dialog shown when downloading add-ons. Thanks.

josephsl avatar Aug 05 '22 11:08 josephsl

@feerrenrut

@nvdaes I'd like that Install button has a shortcut, and even that multiple add-ons can be selected, for example with a list of checkboxes. Note you can access "actions" for the selected add-on via the application/context menu. I intend to open this menu with the enter key is press while an add-on is selected. This is still a todo item. When we are confident about the full list of actions we can also add in accelerator keys.

OK, maybe better just to go on with this idea without the possibility of selecting multiple add-ons at the same time, since for each one we can also see further details, so multiple selections seems tricky

Is the concern about how long it takes to get back to the list, perhaps an accelerator key (for the list) would be sufficient?

No, the list is not focused after pressing the Install button. I think it maybe related to this code:

def _makeButtonClickedEventHandler(_action: AddonActionVM) -> Callable[[wx.CommandEvent, ], None]:
			"""Get around python binding to the latest value in a for loop, create a new lambda
			for each value with an explicit binding to the addon details.
			"""
			# evt: wx.CommandEvent
			return lambda evt: _action.actionHandler(self._detailsVM.listItem)

nvdaes avatar Aug 05 '22 17:08 nvdaes

Regarding the further details view, hyperlinks are reported but seems there's nothing to do with them, for example copy URLs or open them pressing Enter. Is this intended or a todo?

nvdaes avatar Aug 05 '22 17:08 nvdaes

No, the list is not focused after pressing the Install button. I think it maybe related to this code:

I see, STR:

  • Tab to an add-on description
  • Tab one more time to the install button
  • Press enter

Actual:

  • install button becomes disabled
  • focus shifts to the next control: "Other details"

Expected: Shift focus somewhere more useful.

I think It's debatable where focus should be shifted. Given this is the standard Windows behavior, I think there should be a pretty strong rational for changing it. Note that when there are other buttons in this group, focus will shift to one of those buttons.

At this stage, this is a low priority usability concern but may be worth considering further later.

feerrenrut avatar Aug 08 '22 03:08 feerrenrut

@feerrenrut From the discussion on the PR it seems you're planning to make the install tasks execute after NVDA is restarted, rather than, as it is now, during the add-on install. This is going to make most usages of install tasks pretty difficult and make them much less useful. I don't want to go into specific examples here to avoid getting off topic, and also because you gave no reasoning for the change, so it is hard to discuss pros and cons. Could you create a separate issue / discussion outlining what, exactly, you intend to modify and why, so that add-on and core developers can be informed in advance and check if this is going to meet their needs?

lukaszgo1 avatar Aug 14 '22 16:08 lukaszgo1

@lukaszgo1 I've created https://github.com/nvaccess/nvda/issues/14430 it would be helpful if you could explain how changing the timing of the add-on installation and install-tasks would affect add-on authors.

feerrenrut avatar Dec 09 '22 07:12 feerrenrut

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • PASS: Lint check.
  • FAIL: System tests (tags: installer NVDA). See test results for more information.
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/hpqocrbvhg06jt9v/artifacts/output/nvda_snapshot_pr13985-27265,560e6c0b.exe
  • CI timing (mins): INIT 0.0, INSTALL_START 0.8, INSTALL_END 1.0, BUILD_START 0.0, BUILD_END 25.4, TESTSETUP_START 0.0, TESTSETUP_END 0.3, TEST_START 0.0, TEST_END 23.0, FINISH_END 0.1

See test results for failed build of commit 560e6c0b85

AppVeyorBot avatar Dec 12 '22 13:12 AppVeyorBot

@feerrenrut this addon stor is really grate, but if you remove manage addons dialogue from the tools menu, then how we can disable / enable or remove our addons? because currently, we are doing it from manage addons.

amirmahdifard avatar Dec 21 '22 20:12 amirmahdifard

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • FAIL: Lint check. See test results for more information.
  • FAIL: System tests (tags: installer NVDA). See test results for more information.
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/es4bqgfslcx6pqdk/artifacts/output/nvda_snapshot_pr13985-27998,d3f0f4d9.exe
  • CI timing (mins): INIT 0.0, INSTALL_START 0.7, INSTALL_END 0.8, BUILD_START 0.0, BUILD_END 21.8, TESTSETUP_START 0.0, TESTSETUP_END 0.3, TEST_START 0.0, TEST_END 21.3, FINISH_END 0.1

See test results for failed build of commit d3f0f4d926

AppVeyorBot avatar Apr 03 '23 05:04 AppVeyorBot

Hi,

If a user disables one or more add-ons, installs add-on store build, then moves back to a previous build, the following traceback may result:

CRITICAL - main (08:33:07.417) - MainThread (19836): core failure Traceback (most recent call last): File "nvda.pyw", line 406, in File "core.pyc", line 526, in main File "addonHandler_init_.pyc", line 167, in initialize File "addonHandler_init_.pyc", line 78, in load AttributeError: Can't get attribute 'AddonStateCategory' on <module 'addonHandler' from 'C:\Program Files (x86)\NVDA\library.zip\addonHandler\init.pyc'>

Thanks.

josephsl avatar Apr 21 '23 15:04 josephsl

  • FAIL: Translation comments check. Translation comments missing or unexpectedly included. See build log for more information.
  • PASS: Unit tests.
  • PASS: Lint check.
  • PASS: System tests (tags: installer NVDA).
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/vm6drhotjpqjaa2s/artifacts/output/nvda_snapshot_pr13985-28155,fe9b1ba1.exe
  • CI timing (mins): INIT 0.0, INSTALL_START 0.9, INSTALL_END 0.9, BUILD_START 0.0, BUILD_END 25.0, TESTSETUP_START 0.0, TESTSETUP_END 0.3, TEST_START 0.0, TEST_END 21.9, FINISH_END 0.2

See test results for failed build of commit fe9b1ba10c

AppVeyorBot avatar Apr 27 '23 02:04 AppVeyorBot

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • PASS: Lint check.
  • FAIL: System tests (tags: installer NVDA). See test results for more information.
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/bn52ugu0bvj7wpq0/artifacts/output/nvda_snapshot_pr13985-28156,bec1af04.exe
  • CI timing (mins): INIT 0.0, INSTALL_START 0.8, INSTALL_END 0.9, BUILD_START 0.0, BUILD_END 22.8, TESTSETUP_START 0.0, TESTSETUP_END 0.3, TEST_START 0.0, TEST_END 18.8, FINISH_END 0.2

See test results for failed build of commit bec1af04af

AppVeyorBot avatar Apr 27 '23 03:04 AppVeyorBot

@seanbudd When checksum of an add-on doesn't match, a critical error is presented,but NVDA doesn't read it. It can be tested trying to install Unigram 2.0.0 from the store,checking out this PR. Here's NVDA's log. nvdaLog.zip

nvdaes avatar May 01 '23 08:05 nvdaes

@nvdaes - this should be fixed as of 03567da

seanbudd avatar May 03 '23 01:05 seanbudd

Hi all - this PR is now ready for code review. The userGuide documentation is still under review: https://github.com/nvaccess/nvda/pull/14883 Here's a try build of the addonStore with userGuide documentation

For a guide on smoke testing, review the manual test plan.

seanbudd avatar May 03 '23 02:05 seanbudd

Assuming no blocking issues are raised, this should get merged to alpha by end of next week.

seanbudd avatar May 03 '23 02:05 seanbudd

Hi @seanbudd , now, when I try to download Unigram 2.0.0, I can read the message. Then, if I press tab, NVDA says the word Panel, and pressing Enter, it appears that the dialog to install from external source can be opened from this extra unlabelled control.

nvdaes avatar May 03 '23 03:05 nvdaes

Hello, I think that we should send this to the add-ons mailing list. I'll do it unless NV Access wants to do it before.

2023-05-03 4:56 GMT+02:00, Sean Budd @.***>:

Assuming no blocking issues are raised, this should get merged to alpha next week.

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

Message ID: @.***>

nvdaes avatar May 03 '23 03:05 nvdaes

Hi @seanbudd , now, when I try to download Unigram 2.0.0, I can read the message. Then, if I press tab, NVDA says the word Panel, and pressing Enter, it appears that the dialog to install from external source can be opened from this extra unlabelled control.

I cannot reproduce this issue. Could you write up more specific steps to reproduce?

Hello, I think that we should send this to the add-ons mailing list.

As usual, we will post an announcement when we believe the feature is ready to be announced. We plan to announce this in nvda-devel and nvda-addons when this has been merged to master and the feature is ready for alpha testing. I'm not sure what value there is in announcing this now to the add-ons mailing list, or providing early warning that this will be merged soon. The feature is not ready for testing for add-on users, and the process has not changed for add-on authors/publishers. In case blocking issues are found, it may be some time before this is merged to master. Could you explain what you thinks needs to be announced and why and we can consider making the announcement early?

seanbudd avatar May 03 '23 04:05 seanbudd

Hi Sean, sorry, I have announced this thinking that you wouldn't have concerns with this, but the next time I'll wait. My intention was to have more testers since the try build has been published, and this particular feature has been expected long time, and for add-on authors and developers it maybe of interest. I'll try to reproduce the mentioned bug with the panel and will come back.

nvdaes avatar May 03 '23 05:05 nvdaes

Hi @seanbudd , here's log reproducing the commented bug about an unlabelled control, running a portable copy of NVDA on Windows 11. This can be reproduced just sometimes. Cheers.

nvdaes avatar May 03 '23 05:05 nvdaes