UI Automation in Windows Console: rename is21H1Plus to isImprovedTextRangeAvailable

[codeofdusk]

Link to issue number:

microsoft/terminal#9239

Summary of the issue:

Windows 10 21H1 has been released to insiders with significantly reduced scope (i.e. excluding the UIA changes it was originally set to include), making our internal names misleading. Additionally, there are plans to "undock" conhost from Windows, removing the guarantee that a given Windows version will run a particular version of the console (it's technically already possible to run newer console on older Windows, I do it for testing).

Description of how this pull request fixes the issue:

Renames:

• NVDAObjects.UIA.winConsoleUIA.is21H1Plus -> NVDAObjects.UIA.winConsoleUIA.isImprovedTextRangeAvailable
• NVDAObjects.UIA.winConsoleUIA.consoleUIATextInfo -> NVDAObjects.UIA.winConsoleUIA.ConsoleUIATextInfo (Start class name with upper case)
• NVDAObjects.UIA.winConsoleUIA.consoleUIATextInfoPre21H1 -> NVDAObjects.UIA.winConsoleUIA.ConsoleUIATextInfoWorkaroundEndInclusive (because much of the implementation deals with the fact that before Refactor UiaTextRange For Improved Navigation and Reliability microsoft/terminal#4018 text ranges had both end points inclusive, necessitating workarounds for expand, collapse, compareEndPoints, setEndPoint, etc)

Also moves the text override to the workaround textInfo as #10036 is no longer reproducible in new console.

Testing performed:

Verified that console still works as intended. These names were used internally only.

Known issues with pull request:

None.

Change log entry:

Changes for developers:

- ``NVDAObjects.UIA.winConsoleUIA.is21H1Plus`` renamed ``NVDAObjects.UIA.winConsoleUIA.isImprovedTextRangeAvailable``. (#12094)
- ``NVDAObjects.UIA.winConsoleUIA.consoleUIATextInfo`` renamed to start class name with upper case. (#12094)
- ``NVDAObjects.UIA.winConsoleUIA.consoleUIATextInfoPre21H1`` renamed ``NVDAObjects.UIA.winConsoleUIA.ConsoleUIATextInfoWorkaroundEndInclusive``. (#12094)
  - The implementation works around both end points being inclusive (in text ranges) before microsoft/terminal#4018
  - Workarounds for ``expand``, ``collapse``, ``compareEndPoints``, ``setEndPoint``, etc

Code Review Checklist:

This checklist is a reminder of things commonly forgotten in a new PR.
Authors, please do a self-review and confirm you have considered the following items.
Mark items you have considered by checking them.
You can do this when editing the Pull request description with an x: [ ] becomes [x].
You can also check the checkboxes after the PR is created.

• Pull Request description is up to date.
• Unit tests.
• System (end to end) tests.
• Manual tests.
• User Documentation.
• Change log entry.
• Context sensitive help for GUI changes.

[codeofdusk]

Cc @feerrenrut, @michaelDCurran, @carlos-zamora, @DHowett.

[codeofdusk]

Essentially, GetVisibleRanges is a convenient way to check whether microsoft/terminal#4018 is available, as that work fixed many bugs in the UIA implementation.

[feerrenrut]

Is there any other way to check for that? If not, then please just change the names and docs here to be really explicit about what you are trying to determine and why.

Perhaps a name like IsImprovedUIATextRangeAvailable with a docstring including all the notes you have given me in comments here, and also pointing to that PR.

[codeofdusk]

Is there any other way to check for that?

Sadly I don't think so, CC @miniksa and @carlos-zamora.

[josephsl]

Hi, one possible idea might be the ability to check for conhost.exe file metadata, as Bill pointed out that it is possible to test newer versions of this binary. I tried this approach but couldn’t get it to work (I used looking up app version for conhost.exe using file metadata method defined in app module class to no avail). Thanks.

[miniksa]

Is there any other way to check for that?

Sadly I don't think so, CC @miniksa and @carlos-zamora.

We are strongly advised by the Windows compatibility experts that detecting functionality is always supreme to detecting version, so we would have to say that we prefer the existing method of testing for the functionality to know you're on an appropriate version.

However, if you must go beyond that, you might be able to use VerifyVersionInfo of which I have a sample here https://github.com/microsoft/terminal/blob/22fd06e19b3e564d448d60ebf34ffd63764f8080/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp#L1158-L1168. I'm obviously guilty of ignoring their strong recommendation and checking the version there... but VerifyVersionInfo is preferred by the compat team to getting the number and doing the comparison yourself with one of the GetVersion* APIs.

Do note that generally speaking, the Windows compatibility team has installed a number of measures to make it very difficult for applications to get the version of the operating system and force them to feature detect instead. So you may get a "version lie" out of that API or any of the others that is crafted to the maximum version that NVDA is pre-manifested to support.

[codeofdusk]

I've updated comments to express intent more clearly, but haven't yet changed any names yet.

Something like isImprovedTextRangeAvailable as a replacement for is21H1Plus sounds good, but what should the new name for ConsoleUIATextInfoPre21H1 be?

[feerrenrut]

@codeofdusk for brevity you can probably drop the "consoleUIA" part since it is in the "winConsoleUIA" module already. Then I'd either mention its a work around for broken "GetVisibleRanges" perhaps TextInfoWorkaroundBrokenGetVisibleRanges. Other options could be to refer to what the replacement it is based like TextInfoUsingRangeFromPoint

I think TextInfoWorkaroundBrokenGetVisibleRanges is more clear, despite being more ugly. Hopefully the ugly helps to highlight that not everything can be relied on when this is there.

[codeofdusk]

@feerrenrut I settled on TextInfoWorkaroundEndInclusive (see description).

[AppVeyorBot]

• PR introduces Flake8 errors
• Build (for testing PR)

See test results for failed build of commit 64348e2b3b

[feerrenrut]

Let me know if you will change the TextInfo. Either way I think this can be merged.

[codeofdusk]

Ready for a merge whenever you are.

[feerrenrut]

The problem with using TextInfo as a name is that code like this becomes quite ambiguous for a reader. I think it is unnecessarily ambiguous. Consistency is worthwhile if the convention scales, I'm not sure this convention really makes anything better, other than allowing the developer a way out of coming up with a descriptive name.

Code is read many more times than it is written. Better to optimize for the reader IMO.

[feerrenrut]

When I say ambiguous, I mean that you have to be aware that there is a class within this module called TextInfo, and check that the import doesn't do from textInfos import TextInfo.

[codeofdusk]

OK, the name is now ConsoleUIATextInfo (with capitalization consistent with the rest of NVDA).

In the long term (i.e. once microsoft/terminal#6986 has been resolved), I hope to transition to just using the standard UIATextInfo implementation for consoles where isImprovedTextRangeAvailable... but that's in a follow-up PR.

[feerrenrut]

I'll look to merge this on Monday.

The base branch was changed.

[feerrenrut]

Thanks @codeofdusk. This is being merged into Beta, it will be merged back into alpha/master shortly.

[feerrenrut]

A note @codeofdusk, this requires a change log. Code is only "internal use" if the class / function is prefixed with an underscore. Otherwise it is considered an API change and should be documented.

[codeofdusk]

Something like this in dev changes:

• WinConsoleUIA.is21H1Plus has been renamed to WinConsoleUIA.isImprovedTextRangeAvailable and ConsoleUIATextInfoPre21H1 has been renamed to ConsoleUIATextInfoWorkaroundEndInclusive due to Microsoft changes
• consoleUIATextInfo has been renamed to ConsoleUIATextInfo for consistency in capitalization

[feerrenrut]

I have updated the description for you.

[codeofdusk]

LGTM.