Skip to content

NVDA source code and user guide: remove Windows 8 constant mentions#15666

Merged
seanbudd merged 20 commits into
nvaccess:masterfrom
josephsl:removeWin8ConstantMentions
Oct 23, 2023
Merged

NVDA source code and user guide: remove Windows 8 constant mentions#15666
seanbudd merged 20 commits into
nvaccess:masterfrom
josephsl:removeWin8ConstantMentions

Conversation

@josephsl

@josephsl josephsl commented Oct 22, 2023

Copy link
Copy Markdown
Contributor

Link to issue number:

Closes #15663
Closes #15664

Summary of the issue:

Remove mention of Windows 8 (winVersion.WIN8) constant from source code, and for the user guide, remove Windows 8 mention from audio ducking setting.

Description of user facing changes

None

Description of development approach

  • winVersion: isFullScreenMagnificationAvailable will return Windows 8.1 or later.
  • WTS lock session: removed private Windows 7 enumeration.
  • User guide: edited audio ducking setting paragraph.

Testing strategy:

Manual testing with code coverage - making sure NVDA runs on Windows 8.1 and later with screen curtain, Ease of Access, and session lock functionality tested.

Known issues with pull request:

None

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.

Deprecation notice:

easeOfAccess.canConfigTerminateOnDesktopSwitch is deprecated with no replacement - this wil always return True as NVDA now requires Windows 8.1 or later.

Thanks.

…ch' and mark it as deprecated. Re nvaccess#15662.

Windows 8.1 is the mimimum OS, therefore config can termiante on desktop switch.
…te enumerations. Re nvaccess#15663.

Return Windows 8.1 or later lock session state enumeration as Windows 7 is no longer supported by NVDA.
…15664.

Mostly for screen curtain: as NVDA requires Windows 8.1 or later, there is no need to actualy provide a flag that says full screen magnification API is present. However the API is not supported for 32-bit apps (WWoW64), so keep the flag around in case Microsoft removes in a future Windows release.
@josephsl josephsl requested review from a team as code owners October 22, 2023 22:22
@josephsl josephsl changed the title NVDA source code and user uide: remove Windows 8 constant mentions NVDA source code and user guide: remove Windows 8 constant mentions Oct 22, 2023
@josephsl

Copy link
Copy Markdown
Contributor Author

Note: #15662 is closed thanks to #15644.

@seanbudd

Copy link
Copy Markdown
Member

Should you remove "Closes #15662" from the PR description?

Comment thread source/winVersion.py Outdated
Comment thread source/easeOfAccess.py
josephsl and others added 2 commits October 22, 2023 17:04
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
@josephsl

Copy link
Copy Markdown
Contributor Author

Initial comment edited, thanks for the reminder.

Comment thread source/winVersion.py Outdated
testing our specific usage of the API with each Windows version since Windows 8
"""
return getWinVer() >= WIN8
return True

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i think it makes sense to deprecate this function to match the strategy used in #15644.
That way usage in NVDA core is discouraged and removed in favour of simpler code.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i.e ScreenCurtainProvider.canStart can also be simplified to return True

Comment thread user_docs/en/changes.t2t Outdated

- Using ``watchdog.getFormattedStacksForAllThreads`` is deprecated - please use ``logHandler.getFormattedStacksForAllThreads`` instead. (#15616, @lukaszgo1)
- ``easeOfAccess.canConfigTerminateOnDesktopSwitch`` has been deprecated, as it became obsolete since Windows 7 is no longer supported. (#15644, @LeonarddeR)
- ``winVersion.isFullScreenMagnificationAvailable`` has been deprecated, as it became obsolete since Windows 7 is no longer supported. (#15664, @josephsl)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this deprecation could also suggest ScreenCurtainProvider.canStart as an alternative.

…ancementProviders.screenCurtain.ScreenCurtainProvider.canStart. Re nvaccess#15664
@josephsl

josephsl commented Oct 23, 2023 via email

Copy link
Copy Markdown
Contributor Author

Comment thread source/winVersion.py
testing our specific usage of the API with each Windows version since Windows 8
"""
return getWinVer() >= WIN8
if NVDAState._allowDeprecatedAPI():

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can this please use the standard __getattr__ methoud outlined in deprecations.md?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I did it like this initially as the deprecated symbol is a function, not a constant or a variable, using gui.quit as precedence. If an add-on or other part of NVDA calls winVersion.isFullScreenMagnificationAvailable using the proposed deprecation method, Python will raise attribute error, which amounts to removing the function altogether from the module.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Makes sense

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the gui.quit example did come before this was standardised though, so I wouldn't use it as precedent

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this should give us a motivation to refine the getattr method when dealing with deprecated functions and class methods. I'll follow the suggested process, and then will create a new GitHub issue.

Comment thread source/winVersion.py Outdated
testing our specific usage of the API with each Windows version since Windows 8
"""
log.debugWarning(
"Deprecated function called: winVersion.isFullScreenMagnificationAvailable", stack_info=True

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this should suggest the alternative as well

@seanbudd seanbudd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @josephsl

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Screen curtain: assume full screen magnification is available (for now) Windows API: remove Windows 7 specific lock session structure/enumeration

3 participants