Fix open of Contributors and License Help menu options on Windows 11 by XLTechie · Pull Request #14816 · nvaccess/nvda

XLTechie · 2023-04-10T04:59:31Z

Link to issue number:

Summary of the issue:

Under Windows 11, attempting to open the NVDA Help menu items "Contributors" or "License", results in Notepad trying to open nonsense filenames while using installed, normal user level, copies of NVDA

Through testing, this has been narrowed to a bug in os.startfile in Python 3.7 as invoked on Windows 11, though the actual bug may be in a lower level library. The bug does not seem to occur when using the commandline "start" command.
It also does not occur in portable or admin level copies of NVDA.

Description of user facing changes

Restores the ability to open the Contributors List or License file from the NVDA Help menu.

Description of development approach

Through testing, I and @britechguy were able to narrow the source of the errors, and determine that attempting to open the file for editing, rather than by trusting the Windows open file association for .txt files, would open these files in Notepad correctly.

Taken from the text of the commit:

Added the _displayTextFileWorkaround function to the systemUtils module.
- Called it wherever os.startfile was opening a .txt file in gui/__init__.py.
- The function uses os.startfile's "edit" operation to open the text file, which still uses Notepad, at least on most systems.

Additionally:

gui.MainFrame was importing systemUtils late, in init, and onRunCOMRegistrationFixesCommand.
- Moved to a single import in top section.

Testing strategy:

Manual testing with 2023.1's Python console, that calling os.startfile as described, opens these files correctly under Windows 11. While calling it the standard way from the Python console, results in nonsense filenames being delivered to Notepad, and the subsequent failure of the task.
Using a try build of this PR, tested opening these help menu items on Windows 10, with expected success.
Tested that installing a try build of this PR on Windows 11, could open the files from the menu, and could open a copy of copying.txt at the root of the system drive, using the Python console.
A portable copy generated from this PR, placed in the root directory of a USB stick, could open its included text files from the Help menu, and could open a copy of copying.txt placed at the root of the USB drive.

Additionally, to confirm that consolidating and moving the imports of systemUtils didn't break anything, I tested that NVDA launched, and the COM Registration Fixing Tool still works, under Windows 10, which is sufficient.

Tested on:

Windows 11: 22H2, build 22621.1555
Windows 10: 22H2, build 19045.2846

Known issues with pull request:

It is not necessary to use operation="edit" under Windows 10. Since this seemed (1) rather harmless, and (2) a low impact bug, I didn't think it was necessary to test for Windows version before deciding which call structure to use for the new function.
It is conceivable, that opening for editing will have side effects on some systems. However, since it seems reasonable to presume that a user who has made editing text files open in some other program than Notepad is quite experienced, I thought the negative potential here was rather low to non-existent.
When we migrate to a newer Python version, the necessity of this workaround should be reevaluated, as hopefully this will be fixed by a newer library.

Change log entries:

Bug fixes
In Windows 11, it is (once again) possible to open the Contributors and License items on the NVDA Help menu.

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.

AppVeyorBot · 2023-04-10T10:08:22Z

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/8fpakdc028kbn81d/artifacts/output/nvda_snapshot_pr14816-28059,c64df920.exe
CI timing (mins):
INIT 0.0,
INSTALL_START 0.8,
INSTALL_END 0.8,
BUILD_START 0.0,
BUILD_END 22.1,
TESTSETUP_START 0.0,
TESTSETUP_END 0.3,
TEST_START 0.0,
TEST_END 21.7,
FINISH_END 0.2

See test results for failed build of commit c64df92087

Brian1Gaff · 2023-04-10T16:37:53Z

Good idea, what are we saying then, that running it twice ruins the fix in older windows? Brian

…

-- ***@***.*** Sent via blueyonder.(Virgin media) Please address personal E-mail to:- ***@***.***, putting 'Brian Gaff' in the display name field. ----- Original Message ----- From: "Lukasz Golonka" ***@***.***> To: "nvaccess/nvda" ***@***.***> Cc: "Subscribed" ***@***.***> Sent: Monday, April 10, 2023 12:54 PM Subject: Re: [nvaccess/nvda] Fix open of Contributors and License Help menu options on Windows 11 (PR #14816)

@lukaszgo1 commented on this pull request. > @@ -484,10 +484,22 @@ def __init__(self, frame: MainFrame): self.Bind(wx.EVT_MENU, lambda evt: os.startfile("http://www.nvda-project.org/"), item) # Translators: The label for the menu item to view NVDA License document. item = menu_help.Append(wx.ID_ANY, _("L&icense")) - self.Bind(wx.EVT_MENU, lambda evt: os.startfile(getDocFilePath("copying.txt", False)), item) + # Using operation="edit" to os.startfile below, to work around a Windows 11 related + # bug in Python 3.7's os.startfile. (#14725) Test reverting to original behavior after Python upgrade. Can this comment and the logic responsible for opening text files be moved to a separate function in `systemUtils`? This ensures that both code and the accompanying comment is not duplicated, and provides add-on developers with a tested way for opening text files under all versions of Windows. -- Reply to this email directly or view it on GitHub: #14816 (review) You are receiving this because you are subscribed to this thread. Message ID: ***@***.***>

XLTechie · 2023-04-11T06:55:47Z

@seanbudd PR description updated; new function created; a couple little unrelated odds and ends fixed. Ready for re-review.

XLTechie · 2023-04-11T08:02:18Z

@lukaszgo1 Regarding your request to put this in a public function so that add-ons can use it:

It turns out, according to this mailing list post, that it would not have worked anyway. Evidently, the operation="edit" fix, only works for files under "Program Files" directories.

Brian1Gaff · 2023-04-11T09:29:05Z

This seems strange to me. If a routine is being called one might expect it to be used by all such file open attempts using the same function, unless there is another, similar one we do not know about, or course. Brian

…

-- ***@***.*** Sent via blueyonder.(Virgin media) Please address personal E-mail to:- ***@***.***, putting 'Brian Gaff' in the display name field. ----- Original Message ----- From: "Luke Davis" ***@***.***> To: "nvaccess/nvda" ***@***.***> Cc: "Brian Gaff" ***@***.***>; "Comment" ***@***.***> Sent: Tuesday, April 11, 2023 9:06 AM Subject: Re: [nvaccess/nvda] Fix open of Contributors and License Help menu options on Windows 11 (PR #14816)

@lukaszgo1 Regarding your request to put this in a public function so that add-ons can use it: It turns out, according to [this mailing list post](https://nvda.groups.io/g/nvda/message/105816), that it would not have worked anyway. Evidently, the `operation="edit"` fix, only works for files under "`Program Files`" directories. -- Reply to this email directly or view it on GitHub: #14816 (comment) You are receiving this because you commented. Message ID: ***@***.***>

seanbudd

I've noticed more discussion in the mailing list thread

Does this fix still work correctly? What if NVDA is running as a portable copy outside program files?

Copy file "copying.txt" to "c:"
and in the console, execute the command "os.startfile(r"C:\copying.txt", operation="edit").
The error occurs.
Now copy the same file to "c:\Program files (x86)" and run the command:
os.startfile(r"C:\Program Files (x86)\copying.txt", operation="edit").

…systems. (#14816) Per #14725, Windows 11 can't open text files in Notepad with os.startfile's default operation. - Added the _displayTextFileWorkaround function to the systemUtils module. - Called it wherever os.startfile was opening a .txt file in gui/__init__.py. - The function uses os.startfile's "edit" operation to open the text file, which still uses Notepad, at least on most systems. Additionally: gui.MainFrame was importing systemUtils late, in __init__, and onRunCOMRegistrationFixesCommand. - Moved to a single import in top section.

XLTechie · 2023-04-13T00:56:13Z

@seanbudd The tests you requested have been done by @britechguy, and I have updated the description with the results. The original problem didn't exist for portable copies as far as anyone reported, so we hadn't tested that exhaustively before.

Other than that, I have moved the comments inside the def (because PEP-8 said so); the commit text should be good as-is; and the questionable side-lints have been withdrawn.

XLTechie · 2023-04-14T04:45:01Z

@seanbudd GTG

seanbudd · 2023-04-18T06:16:38Z

Thanks @XLTechie

XLTechie marked this pull request as ready for review April 10, 2023 06:20

XLTechie requested a review from a team as a code owner April 10, 2023 06:20

XLTechie requested a review from seanbudd April 10, 2023 06:20

lukaszgo1 reviewed Apr 10, 2023

View reviewed changes

Comment thread source/gui/__init__.py Outdated

XLTechie mentioned this pull request Apr 11, 2023

Contributors file do not open #14725

Closed

seanbudd reviewed Apr 11, 2023

View reviewed changes

Comment thread source/systemUtils.py Outdated

Comment thread source/systemUtils.py Outdated

seanbudd approved these changes Apr 18, 2023

View reviewed changes

Comment thread source/systemUtils.py Outdated

seanbudd and others added 3 commits April 18, 2023 16:10

fix missing space

0fffa6f

Merge remote-tracking branch 'origin/master' into fix14725

8e1a85b

update changes

c49f82f

seanbudd merged commit dc60282 into nvaccess:master Apr 18, 2023

nvaccessAuto added this to the 2023.2 milestone Apr 18, 2023

XLTechie deleted the fix14725 branch April 18, 2023 09:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix open of Contributors and License Help menu options on Windows 11 #14816

Fix open of Contributors and License Help menu options on Windows 11 #14816
seanbudd merged 4 commits into
nvaccess:masterfrom
XLTechie:fix14725

XLTechie commented Apr 10, 2023 •

edited

Loading

Uh oh!

AppVeyorBot commented Apr 10, 2023

Uh oh!

Uh oh!

Brian1Gaff commented Apr 10, 2023 via email

Uh oh!

XLTechie commented Apr 11, 2023

Uh oh!

XLTechie commented Apr 11, 2023

Uh oh!

Brian1Gaff commented Apr 11, 2023 via email

Uh oh!

seanbudd left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

XLTechie commented Apr 13, 2023 •

edited

Loading

Uh oh!

XLTechie commented Apr 14, 2023

Uh oh!

Uh oh!

seanbudd commented Apr 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

XLTechie commented Apr 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Link to issue number:

Summary of the issue:

Description of user facing changes

Description of development approach

Testing strategy:

Known issues with pull request:

Change log entries:

Code Review Checklist:

Uh oh!

AppVeyorBot commented Apr 10, 2023

Uh oh!

Uh oh!

Brian1Gaff commented Apr 10, 2023 via email

Uh oh!

XLTechie commented Apr 11, 2023

Uh oh!

XLTechie commented Apr 11, 2023

Uh oh!

Brian1Gaff commented Apr 11, 2023 via email

Uh oh!

seanbudd left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

XLTechie commented Apr 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

XLTechie commented Apr 14, 2023

Uh oh!

Uh oh!

seanbudd commented Apr 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

XLTechie commented Apr 10, 2023 •

edited

Loading

seanbudd left a comment •

edited

Loading

XLTechie commented Apr 13, 2023 •

edited

Loading