Remove contributors file from NVDA, read license as HTML #17600

seanbudd · 2025-01-08T06:15:33Z

Link to issue number:

Summary of the issue:

There is a transient bug with Windows that prevents opening the "contributors" and "license" file in notepad.
The contributors file is no longer maintained or useful, so this should instead be archived.
The license file is written in markdown, but displayed as plain text in notepad, or as a plain text scrollable text window. This makes it hard for users to read.

Description of user facing changes

The license file is now displayed in HTML, generated from markdown.
From the installer, this is an embedded WebView, from the menu, this is a browseable message.
The installer now has a more thorough agreement message to confirm the user has read the license
The contributors file can no longer be accessed from NVDA
Note: the license file is still not openable in secure screens, but now can be read in secure mode

Description of development approach

Use markdown module to convert markdown to html
Use nh3 to sanitize generated HTML
Generate and cache HTML from the license only when required, on first load.

Testing strategy:

Tested creating an NVDA installer and reading the license and opening the license from the NVDA menu.

Known issues with pull request:

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.

@coderabbitai summary

source/documentationUtils.py

user_docs/en/userGuide.md

source/documentationUtils.py

CyrilleB79 · 2025-01-08T07:42:26Z

Alternatively:
Why do you generate the HTML at runtime? Couldn't you generate it at build time instead given the content does not change?

Co-authored-by: Cyrille Bougot <[email protected]>

seanbudd · 2025-01-08T07:46:51Z

Why do you generate the HTML at runtime? Couldn't you generate it at build time instead given the content does not change?

Simplicity really - we don't need to store or use the HTML file anywhere else unlike other documentation.

AppVeyorBot · 2025-01-08T09:17:13Z

PASS: Translation comments check.
PASS: License 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/8pdofaxflh9u23yp/artifacts/output/nvda_snapshot_pr17600-34944,65aaa9ba.exe
CI timing (mins):
INIT 0.0,
INSTALL_START 1.3,
INSTALL_END 1.0,
BUILD_START 0.0,
BUILD_END 25.9,
TESTSETUP_START 0.0,
TESTSETUP_END 0.2,
TEST_START 0.0,
TEST_END 1.1,
FINISH_END 0.1

See test results for failed build of commit 65aaa9bac2

LeonarddeR · 2025-01-08T14:37:05Z

copying.txt

Should this be renamed to LICENSE.md while add it? This ensures that GitHub will also be able to view the license. See this documentation

we'd have to update every copyright header to reference license.md rather than copying.txt. It doesn't seem worth breaking existing links to the file

I think its worth doing this in a separate issue/PR

LeonarddeR · 2025-01-08T14:38:58Z

source/documentationUtils.py

+
+
+@lru_cache(maxsize=1)
+def _getSanitizedHtmlLicense() -> str:


This worries me a bit. It means that from now on, markdown and mh3 are bundled with release builds of NVDA just in case one wants to check the license. Have you considered creating the html in the build process instead? I think I'd prefer that.

This worries me a bit. It means that from now on, markdown and mh3 are bundled with release builds of NVDA just in case one wants to check the license.

Could you be more specific why this is worrying? Matter of disk space? Possibilities for add-ons? other?

Have you considered creating the html in the build process instead? I think I'd prefer that.

@seanbudd has already answered to this question in #17600 (comment). Though, I have not found this answer really convincing.
@seanbudd, you invokes simplicity regarding runtime generation, but on the contrary, I feel it more complex: instead of having only one process at build time to generate all HTML files, you have now two processes: one at runtime for license and one at build time for other files.

By the way, the license html file should probably added to .gitignore, no matter if it is generated at runtime or at build time.

I feel it more complex: instead of having only one process at build time to generate all HTML files, you have now two processes: one at runtime for license and one at build time for other files.

The two processes are very different - one is connected to our translation and documentation hosting system, the other is for rendering a file for use within NVDA. To add it to the build system there's a lot more complexity and chances for something to go wrong (e.g. bad bundling, different sanitation rules). We need to reuse other generated HTML files, but this only needs to be read at most once for most users.

By the way, the license html file should probably added to .gitignore, no matter if it is generated at runtime or at build time.

No file is generated at runtime currently

nh3 is already planned on being bundled in #17581

seanbudd · 2025-01-08T23:56:23Z

source/gui/startupDialogs.py

 		# Translators: The label of the license text which will be shown when NVDA installation program starts.
 		groupLabel = _("License Agreement")
-		sizer = wx.StaticBoxSizer(wx.VERTICAL, self, label=groupLabel)


note this had to be changed because of wxWidgets/wxWidgets#25058

user_docs/en/changes.md

user_docs/en/userGuide.md

AppVeyorBot · 2025-01-09T00:20:05Z

PASS: Translation comments check.
PASS: License 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/9n2asvexodelb9j8/artifacts/output/nvda_snapshot_pr17600-34948,77d2f0a9.exe
CI timing (mins):
INIT 0.0,
INSTALL_START 1.2,
INSTALL_END 0.9,
BUILD_START 0.0,
BUILD_END 27.0,
TESTSETUP_START 0.0,
TESTSETUP_END 0.2,
TEST_START 0.0,
TEST_END 1.1,
FINISH_END 0.2

See test results for failed build of commit 77d2f0a914

seanbudd · 2025-01-09T04:30:55Z

contributors.txt

+This is a list of people and organizations that contributed to the NVDA project in various ways since the beginning of the project.
+For an overview of code contributors, see also https://github.com/nvaccess/nvda/graphs/contributors.
+For a list of active contributors, see the experts list https://github.com/nvaccess/nvda/blob/master/projectDocs/community/expertsList.md
+This list is now archived.


Suggested change

This list is now archived.

This list was archived in 2025 and is no longer being modified,

Suggested change

This list is now archived.

This list was archived in January 2025, and is no longer being maintained.

Qchristensen

All looks good, and the size of the license on screen looks good for me.

Made a suggestion about noting the date of archiving the contributors file, otherwise all good.

Qchristensen · 2025-01-09T04:31:15Z

contributors.txt

+This is a list of people and organizations that contributed to the NVDA project in various ways since the beginning of the project.
+For an overview of code contributors, see also https://github.com/nvaccess/nvda/graphs/contributors.
+For a list of active contributors, see the experts list https://github.com/nvaccess/nvda/blob/master/projectDocs/community/expertsList.md
+This list is now archived.


Should we indicate when this list was archived? I know it's not up to date now, but saying it was archived in 2024 / 2025 may be useful for anyone looking at the file in, say 10 years time.

SaschaCowley · 2025-01-09T05:31:25Z

contributors.txt

+This is a list of people and organizations that contributed to the NVDA project in various ways since the beginning of the project.
+For an overview of code contributors, see also https://github.com/nvaccess/nvda/graphs/contributors.
+For a list of active contributors, see the experts list https://github.com/nvaccess/nvda/blob/master/projectDocs/community/expertsList.md
+This list is now archived.


Suggested change

This list is now archived.

This list was archived in January 2025, and is no longer being maintained.

SaschaCowley · 2025-01-09T05:49:51Z

source/gui/startupDialogs.py

+		groupText = wx.StaticText(self, label=groupLabel)
+		sizer.Add(groupText, border=gui.guiHelper.SPACE_BETWEEN_ASSOCIATED_CONTROL_VERTICAL)
+
+		licenseView: wx.html2.WebView = wx.html2.WebView.New(self, size=self.scaleSize((550, 400)))


Where are these dimensions coming from?

SaschaCowley · 2025-01-09T05:52:14Z

user_docs/en/changes.md

@@ -50,6 +50,8 @@ To use this feature, "allow NVDA to control the volume of other applications" mu
  * Updated CLDR to version 46.0. (#17484, @OzancanKaratas)
 * Short versions of the most commonly used command line options have been added: `-d` for `--disable-addons` and `-n` for `--lang`.
 Prefix matching on command line flags, e.g. using `--di` for `--disable-addons` is no longer supported. (#11644, @CyrilleB79)
+* The "Contributors" file was removed from the NVDA menu. (#16922)


Suggested change

* The "Contributors" file was removed from the NVDA menu. (#16922)

* The "Contributors" file has been removed from the NVDA menu. (#16922)

SaschaCowley · 2025-01-09T06:11:38Z

copying.txt

@@ -3,7 +3,7 @@
 NVDA is available under the GNU General Public License version 2, with two special exceptions.
 The exceptions are outlined in the sections "Non-GPL Components in Plugins and Drivers" and "Microsoft Distributable Code".
 NVDA also includes and uses components which are made available under different free and open source licenses.
-Information about how to obtain and build the code for yourself is available at http://community.nvda-project.org/wiki/AccessingAndRunningSourceCode
+Information about how to obtain and build the code for yourself is available at http://community.nvda-project.org/wiki/AccessingAndRunningSourceCode.


This link no longer works.

Suggested change

Information about how to obtain and build the code for yourself is available at http://community.nvda-project.org/wiki/AccessingAndRunningSourceCode.

Information about how to obtain and build the code for yourself is available at <https://github.com/nvaccess/nvda/blob/master/projectDocs/dev/contributing.md>.

SaschaCowley · 2025-01-09T06:12:58Z

copying.txt

@@ -3,7 +3,7 @@
 NVDA is available under the GNU General Public License version 2, with two special exceptions.
 The exceptions are outlined in the sections "Non-GPL Components in Plugins and Drivers" and "Microsoft Distributable Code".
 NVDA also includes and uses components which are made available under different free and open source licenses.
-Information about how to obtain and build the code for yourself is available at http://community.nvda-project.org/wiki/AccessingAndRunningSourceCode
+Information about how to obtain and build the code for yourself is available at http://community.nvda-project.org/wiki/AccessingAndRunningSourceCode.
 All applicable licenses are included below.

 ## GNU GENERAL PUBLIC LICENSE


Can you please place the link to the GPL in angle brackets so it's picked up by markdown?

SaschaCowley · 2025-01-09T06:14:25Z

copying.txt

+* mfc*.dll
+* Microsoft.VC*.manifest
+* msvc*.dll


Suggested change

* mfc*.dll

* Microsoft.VC*.manifest

* msvc*.dll

* `mfc*.dll`

* `Microsoft.VC*.manifest`

* `msvc*.dll`

SaschaCowley · 2025-01-09T06:16:54Z

source/setup.py

Please update copyright header

CyrilleB79 · 2025-01-09T07:02:33Z

contributors.txt

+This is a list of people and organizations that contributed to the NVDA project in various ways since the beginning of the project.
+For an overview of code contributors, see also https://github.com/nvaccess/nvda/graphs/contributors.
+For a list of active contributors, see the experts list https://github.com/nvaccess/nvda/blob/master/projectDocs/community/expertsList.md
+This list is now archived.


Please move this sentence just after the first sentence of the file for clarity.

Remove contributors file from NVDA, read license as HTML

6bff723

seanbudd requested review from a team as code owners January 8, 2025 06:15

seanbudd requested review from Qchristensen and SaschaCowley January 8, 2025 06:15

seanbudd added 2 commits January 8, 2025 17:16

update docs

ff3576f

fix changes

60c0a72

seanbudd requested a review from gerald-hartig January 8, 2025 07:11

CyrilleB79 reviewed Jan 8, 2025

View reviewed changes

source/documentationUtils.py Outdated Show resolved Hide resolved

source/documentationUtils.py Outdated Show resolved Hide resolved

user_docs/en/userGuide.md Outdated Show resolved Hide resolved

source/documentationUtils.py Show resolved Hide resolved

Apply suggestions from code review

6e035d9

Co-authored-by: Cyrille Bougot <[email protected]>

seanbudd added 2 commits January 8, 2025 19:01

fix markdown readibility

30ab607

fix changes

97ec694

LeonarddeR suggested changes Jan 8, 2025

View reviewed changes

seanbudd marked this pull request as draft January 8, 2025 23:17

use embedded view

0215d3f

seanbudd marked this pull request as ready for review January 8, 2025 23:50

seanbudd commented Jan 8, 2025

View reviewed changes

seanbudd commented Jan 9, 2025

View reviewed changes

user_docs/en/changes.md Outdated Show resolved Hide resolved

seanbudd commented Jan 9, 2025

View reviewed changes

user_docs/en/userGuide.md Outdated Show resolved Hide resolved

seanbudd added 2 commits January 9, 2025 12:20

fix installer

d12a774

Apply suggestions from code review

bdbcff0

seanbudd commented Jan 9, 2025

View reviewed changes

Qchristensen requested changes Jan 9, 2025

View reviewed changes

SaschaCowley requested changes Jan 9, 2025

View reviewed changes

CyrilleB79 reviewed Jan 9, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove contributors file from NVDA, read license as HTML #17600

Remove contributors file from NVDA, read license as HTML #17600

seanbudd commented Jan 8, 2025 •

edited

Loading

CyrilleB79 commented Jan 8, 2025

seanbudd commented Jan 8, 2025

AppVeyorBot commented Jan 8, 2025

LeonarddeR Jan 8, 2025

seanbudd Jan 8, 2025

seanbudd Jan 9, 2025

LeonarddeR Jan 8, 2025

CyrilleB79 Jan 8, 2025

seanbudd Jan 8, 2025

seanbudd Jan 8, 2025

seanbudd Jan 8, 2025

AppVeyorBot commented Jan 9, 2025

seanbudd Jan 9, 2025

SaschaCowley Jan 9, 2025

Qchristensen left a comment

Qchristensen Jan 9, 2025

SaschaCowley Jan 9, 2025

SaschaCowley Jan 9, 2025

SaschaCowley Jan 9, 2025

SaschaCowley Jan 9, 2025

SaschaCowley Jan 9, 2025

SaschaCowley Jan 9, 2025

SaschaCowley Jan 9, 2025

CyrilleB79 Jan 9, 2025

	This list is now archived.
	This list was archived in 2025 and is no longer being modified,

	This list is now archived.
	This list was archived in January 2025, and is no longer being maintained.

	* The "Contributors" file was removed from the NVDA menu. (#16922)
	* The "Contributors" file has been removed from the NVDA menu. (#16922)

	Information about how to obtain and build the code for yourself is available at http://community.nvda-project.org/wiki/AccessingAndRunningSourceCode.
	Information about how to obtain and build the code for yourself is available at <https://github.com/nvaccess/nvda/blob/master/projectDocs/dev/contributing.md>.



		@lru_cache(maxsize=1)
		def _getSanitizedHtmlLicense() -> str:

Remove contributors file from NVDA, read license as HTML #17600

Are you sure you want to change the base?

Remove contributors file from NVDA, read license as HTML #17600

Conversation

seanbudd commented Jan 8, 2025 • edited Loading

Link to issue number:

Summary of the issue:

Description of user facing changes

Description of development approach

Testing strategy:

Known issues with pull request:

Code Review Checklist:

CyrilleB79 commented Jan 8, 2025

seanbudd commented Jan 8, 2025

AppVeyorBot commented Jan 8, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AppVeyorBot commented Jan 9, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Qchristensen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seanbudd commented Jan 8, 2025 •

edited

Loading