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

copying.txt

source/documentationUtils.py

source/gui/startupDialogs.py

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

contributors.txt

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.

contributors.txt

source/gui/startupDialogs.py

user_docs/en/changes.md

copying.txt

source/setup.py

contributors.txt

…License

LeonarddeR · 2025-01-14T06:31:57Z

@coderabbitai could you create an issue to investigate renaming copying.txt to LICENSE.md in order for GitHub to pick up the license?

coderabbitai · 2025-01-14T06:32:41Z

Tip

For best results, initiate chat on the files or code changes.

@LeonarddeR I've created issue #17621 to track the investigation of renaming copying.txt to LICENSE.md for better GitHub license detection.

🔗 Analysis chain

@LeonarddeR I've created a new issue to track the investigation of renaming copying.txt to LICENSE.md for better GitHub license detection.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Find the license file location
fd -t f -i copying.txt

# Check if LICENSE.md already exists to avoid potential conflicts
fd -t f -i license.md

Length of output: 60

Script:

#!/bin/bash
gh issue create \
  --title "Investigate renaming copying.txt to LICENSE.md for GitHub license detection" \
  --body "Currently, the NVDA license is stored in copying.txt. GitHub has built-in license detection that automatically identifies repository licenses when they follow common naming patterns like LICENSE.md.

Consider renaming copying.txt to LICENSE.md to:
- Improve repository license visibility on GitHub
- Follow modern open source conventions
- Make the license more discoverable for new contributors

Related PR: #17600 where license file handling was updated.

Note: This change requires careful consideration of:
- Impact on existing documentation/scripts that might reference copying.txt
- Migration plan for existing installations
- Update of any hardcoded references to the file name"

Length of output: 843

source/gui/__init__.py

LeonarddeR · 2025-01-14T06:37:11Z

source/documentationUtils.py

+
+@blockAction.when(blockAction.Context.SECURE_MODE)
+def displayLicense():
+	# HTML includes links which shouldn't be accessible


I understand this comment, it's however not clear from the comment how activating links is avoided here. I assume that links are not rendered in browseable messages?

it's avoided by the blocking the whole action with the blockAction. Links are rendered in the browseable messages. I'm not sure if I can place the comment any closer to the decorator while keeping formatting happy. Maybe

@blockAction.when( # HTML includes links which shouldn't be accessible blockAction.Context.SECURE_MODE ) def displayLicense():

Ah I see now, thanks for clarifying.
Can't we instruct nh3 to strip links in a secure context, for example by providing an empty list of url schemes?

We could, but I think it's useful for these links to work, no?

It definitely is 😉
However, we can make the behavior of nh3 differ from the default behavior when in a secure context, for example:

if secureContext: attribute_filter = lambda tag, attr, val: None if tag.lower() == "a" and attr.lower() == "href" else val else: attribute_filter = None

In secure context, we probably also want to pass something like url_schemes=set() to ensure that external comments will never load.
The latter might even be enough, as long as we don't provide something like <a href="something">something</a> as these won't be filtered.

@SaschaCowley - do you think this is necessary?

Potentially useful, but not necessary. Note this would also require changes to browsable message, which is currently not available when running on a secure desktop.

nvda/source/ui.py

Lines 156 to 158 in fa619c2

if isRunningOnSecureDesktop():

_warnBrowsableMessageNotAvailableOnSecureScreens(title)

return

I think your first suggestion (putting the explanatory comment in the decorator) is the way to go here, @seanbudd

SaschaCowley · 2025-01-21T06:44:02Z

@seanbudd if I build a launcher from this branch, the HTML view containing the licence is not accessible unless I click on it.

Speech viewer output

NVDA Launcher  dialog  License Agreement
wxWebView
tab
I have read and agree to the license agreement  check box  not checked  Alt+  a
tab
Exit  button  Alt+  x
tab
wxWebView
down arrow
I have read and agree to the license agreement  check box  not checked  Alt+  a
up arrow
wxWebView
down arrow
I have read and agree to the license agreement  check box  not checked  Alt+  a
up arrow
wxWebView
NVDA+shift+m
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 https://github.com/nvaccess/nvda/blob/master/projectDocs/dev/contributing.md. All applicable licenses are included below.
NVDA+[
Left click
document
heading    level 1  NVDA License
down arrow
NVDA is available under the GNU General Public License version 2, with two special exceptions. The 
tab
https://github.com/nvaccess/nvda/blob/master/projectDocs/dev/contributing.md  link  
tab
https://www.gnu.org/licenses/old-licenses/gpl-2.0.html/  link  
tab
https://fsf.org/  link  
tab
https://github.com/nvaccess/nvda-misc-deps/  link  
tab
I have read and agree to the license agreement  check box  not checked  Alt+  a
up arrow
document
link    https://github.com/nvaccess/nvda-misc-deps/
up arrow
the following URL:
down arrow
link    https://github.com/nvaccess/nvda-misc-deps/
down arrow
link    https://github.com/nvaccess/nvda-misc-deps/
tab
https://github.com/nvaccess/nvda/blob/master/projectDocs/dev/contributing.md  link  
tab
https://www.gnu.org/licenses/old-licenses/gpl-2.0.html/  link  
ctrl+end
link    https://github.com/nvaccess/nvda-misc-deps/
https://fsf.org/  link  
https://github.com/nvaccess/nvda-misc-deps/  link  
tab
https://fsf.org/  link  
tab
https://github.com/nvaccess/nvda-misc-deps/  link  
tab
I have read and agree to the license agreement  check box  not checked  Alt+  a
tab
Exit  button  Alt+  x

Qchristensen

All looks good.

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

copying.txt Show resolved Hide resolved

source/documentationUtils.py Show resolved Hide resolved

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

source/gui/startupDialogs.py Show resolved Hide resolved

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

contributors.txt Outdated Show resolved Hide resolved

Qchristensen requested changes Jan 9, 2025

View reviewed changes

contributors.txt Outdated Show resolved Hide resolved

SaschaCowley requested changes Jan 9, 2025

View reviewed changes

CyrilleB79 reviewed Jan 9, 2025

View reviewed changes

contributors.txt Outdated Show resolved Hide resolved

seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Jan 13, 2025

address review comments

e40448b

seanbudd requested review from SaschaCowley, LeonarddeR and Qchristensen January 14, 2025 02:48

Merge remote-tracking branch 'origin/master' into remContributorsHTML…

79be781

…License

seanbudd marked this pull request as draft January 14, 2025 02:50

fixups

3f2f2f1

seanbudd marked this pull request as ready for review January 14, 2025 02:58

coderabbitai bot mentioned this pull request Jan 14, 2025

Investigate renaming copying.txt to LICENSE.md for GitHub license detection #17621

Open

LeonarddeR approved these changes Jan 14, 2025

View reviewed changes

remove _displayTextFileWorkaround

04f2242

Qchristensen approved these changes Jan 21, 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

AppVeyorBot commented Jan 9, 2025

Qchristensen left a comment

LeonarddeR commented Jan 14, 2025

coderabbitai bot commented Jan 14, 2025

LeonarddeR Jan 14, 2025

seanbudd Jan 14, 2025

LeonarddeR Jan 14, 2025

seanbudd Jan 15, 2025

LeonarddeR Jan 15, 2025 •

edited

Loading

seanbudd Jan 21, 2025

SaschaCowley Jan 21, 2025

SaschaCowley Jan 21, 2025

SaschaCowley commented Jan 21, 2025

Qchristensen left a comment

	if isRunningOnSecureDesktop():
	_warnBrowsableMessageNotAvailableOnSecureScreens(title)
	return

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

AppVeyorBot commented Jan 9, 2025

Qchristensen left a comment

Choose a reason for hiding this comment

LeonarddeR commented Jan 14, 2025

coderabbitai bot commented Jan 14, 2025

LeonarddeR Jan 14, 2025

Choose a reason for hiding this comment

seanbudd Jan 14, 2025

Choose a reason for hiding this comment

LeonarddeR Jan 14, 2025

Choose a reason for hiding this comment

seanbudd Jan 15, 2025

Choose a reason for hiding this comment

LeonarddeR Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

seanbudd Jan 21, 2025

Choose a reason for hiding this comment

SaschaCowley Jan 21, 2025

Choose a reason for hiding this comment

SaschaCowley Jan 21, 2025

Choose a reason for hiding this comment

SaschaCowley commented Jan 21, 2025

Qchristensen left a comment

Choose a reason for hiding this comment

seanbudd commented Jan 8, 2025 •

edited

Loading

LeonarddeR Jan 15, 2025 •

edited

Loading