Skip to content

Conversation

nvdaes
Copy link
Collaborator

@nvdaes nvdaes commented Aug 26, 2025

Link to issue number:

Fixes #14041

Summary of the issue:

Users may want to see changes included in the current version of add-ons, especially to decide if they want to update them, and to get an idea of new features, bug fixes and so on.

Description of user facing changes:

  • In the add-on store, a new action allows to see changes for the current version of add-ons which specify this information.
  • In the Updatable add-ons dialog, the virtual list used in other tabs of the store is presented, instead of the old small list, so that the changelog, if available, can be opened from the context menu to decide if add-ons should be updated to the available version.
    List of changes will be shown in browse mode. If changes are writen in markdown, they will be rendered in formatted HTML. The title of that message will show the add-on version name, and buttons will be available to copy the info and close the browseable message.

Description of developer facing changes:

Add-on models have a new changelog property.

Description of development approach:

  • Created a new action to show the changelog, when available, similar to other actions.
  • Added changelog to add-on models.
  • In the Updatable add-ons dialog, the same list used in the store dialog is presented, so that changelog can be opened from the context menu.

Testing strategy:

  • Add list of changes to manifest.ini and json file corresponding to the urlShortener add-on. Check that changes can be shown from the context menu of the store, and that this action is not available for other add-ons.
  • Downgrade version of urlShortener to make an updatable add-on. In source/addonStore/models/addon.py, in the _createStoreCollectionFromJson function, add the following code just for testing:

	for addon in data:
		if addon["addonId"] == "urlShortener":
			addon["changelog"] = "changes"

Check that changelog can be shown just for this add-on.

Also, changelog in markdown has been created in the manifest and in the store collection, to check that markdown is properly rendered in HTML in the store and in the Updatable add-ons dialog.

	changelog = """
* A.
* b
"""
	addonCollection = _createAddonGUICollection()

	for addon in data:
		addon["changelog"] = changelog

Tests have been done with the resourceMonitor add-on to check that the context menu can be used from the Updatable add-ons dialog.

Known issues with pull request:

Sometimes, NVDA says thw word "panel" when the changelog is opened.

Code Review Checklist:

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

@nvdaes
Copy link
Collaborator Author

nvdaes commented Aug 26, 2025

I'm also working locally modifying files of the addon-datastore-validation repo. Also this would need to be added to the documentation (metadata) of the addon-datastore repo. I'll work on this if the direction of this PR is right.

@seanbudd
Copy link
Member

I think the most useful place to read the change log would be from the add-on update dialog. I think from the add-on store lists, it should be a button like Help or Homepage that opens a dialog, with the changelog rendered from markdown to HTML

@nvdaes
Copy link
Collaborator Author

nvdaes commented Aug 27, 2025

@seanbudd, I'm trying to convert markdown to HTML with the markdown.markdown function, but the manifest model doesn't support the strip method, and the conversion is not performed. I have displayed the browseable message with a button from the context menu, but seems that markdown conversion is difficult to achieve with the manifest.
Please let me know if you have any suggestions for this. I have though about creating a named temporary file to write the manifest changelog. Anyway, the changelog should be translatable, and markdown may need to have a document to be managed by translators, instead of using gettext, which is used to translate add-on summary and descriptions included in manifests.

@seanbudd
Copy link
Member

@nvdaes wouldn't calling str() on the change log entry convert it to a string that can be used in markdown converter?

@nvdaes
Copy link
Collaborator Author

nvdaes commented Aug 27, 2025

Sean wrote:

wouldn't calling str() on the change log entry convert it to a string that can be used in markdown converter?

Thanks! It works now.

@nvdaes
Copy link
Collaborator Author

nvdaes commented Aug 27, 2025

Now we can see the changelog from the installed add-ons list using Actions or the context menu, and when updatable add-ons are found, from the Update add-ons dialog.
But I'm struggling with two issues:

  • I've writen a changelog in urlShortener.json, but instead of the changelog, the add-on description is displayed.
  • I would like to press enter to display the changelog without tabbing to the button, but using an event associated to keycode return, it doesn't work.

@nvdaes
Copy link
Collaborator Author

nvdaes commented Aug 29, 2025

Descriptions were presented instead of changelogs in the updatable add-ons dialog, since I used the description key for testing. @seanbudd , when I try to use the changelog key, errors are raised since changelog is not available from data. Perhaps would be better to add changelog to addon-datastore-validation before finishing this PR?

@nvdaes
Copy link
Collaborator Author

nvdaes commented Aug 29, 2025

I think that we can finish this PR before working in addon-datastore-validation. Now I don't get errors.

@nvdaes
Copy link
Collaborator Author

nvdaes commented Sep 12, 2025

Now we use the virtual list available in the store dialog. Unfortunately, for now it's difficult to test this since I get the following error when trying to install an add-on. I think that this is not related to this PR:

ERROR - unhandled exception (21:13:46.401) - MainThread (6012):
Traceback (most recent call last):
File "gui\addonStoreGui\controls\storeDialog.py", line 268, in onClose
self._storeVM.installPending()
~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
File "gui\addonStoreGui\viewModels\store.py", line 595, in installPending
cls._doInstall(listItemVM, fileDownloaded)
~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "gui\addonStoreGui\viewModels\store.py", line 606, in _doInstall
installAddon(fileDownloaded)
~~~~~~~~~~~~^^^^^^^^^^^^^^^^
File "addonStore\install.py", line 85, in installAddon
addonObj = systemUtils.ExecAndPump[addonHandler.Addon](addonHandler.installAddonBundle, bundle).funcRes
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\Usuariopc\AppData\Local\Programs\Python\Python313-32\Lib\typing.py", line 1317, in call
result = self.origin(*args, **kwargs)
File "systemUtils.py", line 237, in init
while user32.MsgWaitForMultipleObjects(1, ctypes.byref(threadHandle), False, -1, 255) == 1:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ctypes.ArgumentError: argument 2: TypeError: expected LP_c_void_p instance instead of pointer to c_long

@nvdaes
Copy link
Collaborator Author

nvdaes commented Sep 17, 2025

nvda.log
After mergin last changes, I'm getting a lot of errors.
Mick created a PR to fix ExecAndPump, and I merged changes thinking that, in this way, an error where add-ons cannot be installed would be fixed. But now testing this is not possible since I hear a lot of sounds reporting errors.
cc: @seanbudd

@seanbudd
Copy link
Member

@nvdaes - are you still encountering this issue?

@nvdaes
Copy link
Collaborator Author

nvdaes commented Sep 19, 2025

@seanbudd , in fact I find this issue and also in a different PR which I opened to show spelling errors in braille.

Please let me know if I should remove and clone NVDA from scratch or something.

@seanbudd
Copy link
Member

Have you merged in the latest nvaccess/master? Is the issue occurring with both alpha and the installer built from the PR by GitHub action? Can you share the log with us?

@nvdaes
Copy link
Collaborator Author

nvdaes commented Sep 19, 2025

I've merged the master branch and I get this error running from source. I'll test the snapshot and alpha and will share the log, and if I still get the error.

@nvdaes
Copy link
Collaborator Author

nvdaes commented Sep 21, 2025

@seanbud, I've tested nvda_snapshot_alpha-52681,c75b2de9.exe, getting a lot of errors. Here's the log.
nvda.log

@SaschaCowley
Copy link
Member

@nvdaes most of those issues should be resolved by #18952. Let us know if they aren't.

@nvdaes
Copy link
Collaborator Author

nvdaes commented Sep 22, 2025

Thanks @SaschaCowley

Most issues are fixed by PR #18952

There is still an issue, but I don't know if this should be fixed by this PR. I find it also in NVDA 2025.3rc1

This happens when the option to update add-ons automatically is checked.
Exception in thread AutomaticAddonUpdate:
Traceback (most recent call last):
File "C:\Users\Usuariopc\AppData\Local\Programs\Python\Python313-32\Lib\threading.py", line 1043, in _bootstrap_inner
self.run()
~~~~~~~~^^
File "C:\Users\Usuariopc\AppData\Local\Programs\Python\Python313-32\Lib\threading.py", line 994, in run
self._target(*self._args, **self._kwargs)
~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "gui\message.py", line 64, in funcWrapper
return func(*args, **kwargs)
File "gui\addonStoreGui\controls\messageDialogs.py", line 653, in _updateAddons
ui.message(pgettext("addonStore", "Updating add-ons..."), SpeechPriority.NEXT)
~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "ui.py", line 251, in message
braille.handler.message(brailleText if brailleText is not None else text)
~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "braille.py", line 2962, in message
self._resetMessageTimer()
~~~~~~~~~~~~~~~~~~~~~~~^^
File "braille.py", line 2976, in _resetMessageTimer
self._messageCallLater = wx.CallLater(timeout, self._dismissMessage)
~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\Usuariopc\Documents\repos\nvda.venv\Lib\site-packages\wx\core.py", line 3471, in init
self.Start()
~~~~~~~~~~^^
File "C:\Users\Usuariopc\Documents\repos\nvda.venv\Lib\site-packages\wx\core.py", line 3492, in Start
self.timer.Start(self.millis, wx.TIMER_ONE_SHOT)
~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
wx._core.wxAssertionError: C++ assertion "wxThread::IsMain()" failed at ....\src\common\timerimpl.cpp(57) in wxTimerImpl::Start(): timer can only be started from the main thread

@nvdaes
Copy link
Collaborator Author

nvdaes commented Sep 22, 2025

I think that this is ready for review, though some system tests failed.

@nvdaes nvdaes marked this pull request as ready for review September 22, 2025 18:07
Comment on lines +447 to +449
from .actions import _MonoActionsContextMenu
from .addonList import AddonVirtualList
from gui.addonStoreGui.viewModels.store import AddonStoreVM
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can any of these imports be moved to top of file?

closeButton = bHelper.addButton(self, wx.ID_CLOSE, label=pgettext("addonStore", "&Close"))
closeButton.Bind(wx.EVT_BUTTON, self.onCloseButton)

def _createAddonsPanel(self, sHelper: BoxSizerHelper):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are several issues with this new design:

  • the buttons to open the store, update all and close are not visible
  • the table is clipped and doesn't contain the full information. Instead the table should fit the dialog, and the dialog should be wider.

@seanbudd seanbudd marked this pull request as draft September 23, 2025 03:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Changelog on addons updates
5 participants