Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Windows Terminal #10784

Merged
merged 22 commits into from
Apr 6, 2020
Merged

Conversation

codeofdusk
Copy link
Contributor

@codeofdusk codeofdusk commented Feb 13, 2020

Link to issue number:

Closes #10305. Supersedes #10716 for now.

Summary of the issue:

The new Windows Terminal does not support automatic readout, suppression of password characters, and other terminal niceties.

Since newer Windows Console builds correctly set the end of text ranges as exclusive, NVDA's current UIA console workarounds produce incorrect behaviour in these builds.

Description of how this pull request fixes the issue:

  • Like UI Automation in Windows Console: Use UIA by default on Windows 10 version 2004 and later #10716, splits the consoleUIATextInfo workarounds into a subclass. This will be useful for Windows 10 version 21H1, where making UIA default in consoles will be strongly worth considering.
  • Adds a new winConsoleUIA.WinTerminalUIA class and attaches it to Windows Terminal controls.
  • Re-introduces support for IUIAutomationTextPattern::GetVisibleRanges for consoles returning one contiguous range (see UIA: Fix GetVisibleRanges() and add Tracing microsoft/terminal#4495).
  • Factors out the password/doubled character suppression into a NVDAObjects.behaviors.EnhancedTermTypedCharSupport class, which KeyboardHandlerBasedTypedCharSupport now subclasses.
  • Adds new logic to properly choose the textInfo implementation to use based on the number of visible ranges presented by the console (newer builds present just one range).

Testing performed:

Tested that Windows Terminal controls correctly receive the WinTerminalUIA type and that consoles work as expected (including keyboard support enhancements).

Tested that in newer Windoes Console (built from the terminal master branch), bounding works as expected even after the window is maximized and a large amount of text is written.

Tested that automatic readout, password suppression, and typed character reporting are functional in the Windows Terminal build in microsoft/terminal#2447.

Tested that the correct textInfo implementation is chosen for the included console on Windows 10 version 1909 Vs. the latest open console from Microsoft.

Known issues with pull request:

None.

Change log entry:

== new features ==

@AppVeyorBot
Copy link

See test results for failed build of commit 89b418db2e

@codeofdusk
Copy link
Contributor Author

Cc @LeonarddeR @michaelDCurran

@codeofdusk
Copy link
Contributor Author

@LeonarddeR @michaelDCurran Could you please have another look? I think this is now ready for merge, and will become functional with microsoft/terminal#4826.

@codeofdusk
Copy link
Contributor Author

The PR description has also been updated with additional changes and testing notes.

@codeofdusk
Copy link
Contributor Author

Cc @feerrenrut and @michaelDCurran Could this PR please be reviewed?

@feerrenrut feerrenrut added enhancement app/windows-terminal New terminal app, potentially supersedes app/windows-console (repo: microsoft/terminal) labels Mar 18, 2020
@feerrenrut
Copy link
Contributor

I haven't yet looked through the changes carefully, my initial impression is concern for regressions. Anything you can do here to reduce the chance of regression (particularly in windows console) will help us to get confidence in this PR.

We are planning to try to clear the backlog of PR's soon, we will initially focus on bug fixes and small PR's.

@codeofdusk
Copy link
Contributor Author

codeofdusk commented Mar 18, 2020

I haven't yet looked through the changes carefully, my initial impression is concern for regressions. Anything you can do here to reduce the chance of regression (particularly in windows console) will help us to get confidence in this PR.

We are planning to try to clear the backlog of PR's soon, we will initially focus on bug fixes and small PR's.

Chance of regressions seems fairly low, as the new terminal and old console functionality are in separate subclasses (WinConsoleUIA and WinTerminalUIA). However, this PR does lay the groundwork for improved UIA console support in Windows 10 2104, which may (likely will) be good enough to use as default when new conhost (including microsoft/terminal#4018) ships. The 2104 console changes will be implemented in a separate PR (that will depend on this one).

@dpy013

This comment has been minimized.

@codeofdusk

This comment has been minimized.

Copy link
Collaborator

@LeonarddeR LeonarddeR left a comment

Choose a reason for hiding this comment

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

Have you tested this on insider builds of Windows 2004?

source/NVDAObjects/UIA/winConsoleUIA.py Outdated Show resolved Hide resolved
source/NVDAObjects/UIA/winConsoleUIA.py Outdated Show resolved Hide resolved
source/NVDAObjects/UIA/winConsoleUIA.py Outdated Show resolved Hide resolved
source/NVDAObjects/UIA/winConsoleUIA.py Outdated Show resolved Hide resolved
source/NVDAObjects/UIA/winConsoleUIA.py Outdated Show resolved Hide resolved
source/NVDAObjects/UIA/winConsoleUIA.py Show resolved Hide resolved
source/NVDAObjects/UIA/winConsoleUIA.py Outdated Show resolved Hide resolved
source/NVDAObjects/UIA/winConsoleUIA.py Outdated Show resolved Hide resolved
source/NVDAObjects/behaviors.py Outdated Show resolved Hide resolved
source/NVDAObjects/behaviors.py Show resolved Hide resolved
@josephsl
Copy link
Collaborator

josephsl commented Mar 31, 2020 via email

@codeofdusk
Copy link
Contributor Author

@LeonarddeR I think all requested changes have now been made.

@codeofdusk
Copy link
Contributor Author

I've rebased this PR on master as it's been open for a while.

@LeonarddeR Could you please have another look?

@dpy013
Copy link
Contributor

dpy013 commented Apr 2, 2020

hi @codeofdusk
Is pr basically complete now?
thanks

@codeofdusk
Copy link
Contributor Author

hi @codeofdusk
Is pr basically complete now?
thanks

Yes, awaiting review from @LeonarddeR and @michaelDCurran.

Copy link
Collaborator

@LeonarddeR LeonarddeR left a comment

Choose a reason for hiding this comment

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

Yay, this shortens the diff significantly, that's good!

source/NVDAObjects/UIA/winConsoleUIA.py Show resolved Hide resolved
source/NVDAObjects/UIA/winConsoleUIA.py Show resolved Hide resolved
source/NVDAObjects/UIA/winConsoleUIA.py Outdated Show resolved Hide resolved
source/NVDAObjects/UIA/winConsoleUIA.py Show resolved Hide resolved
source/NVDAObjects/behaviors.py Outdated Show resolved Hide resolved
@codeofdusk
Copy link
Contributor Author

@LeonarddeR @feerrenrut Could you please have another look?

Copy link
Collaborator

@LeonarddeR LeonarddeR left a comment

Choose a reason for hiding this comment

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

Thanks @codeofdusk

@michaelDCurran
Copy link
Member

Try build: https://ci.appveyor.com/api/buildjobs/milyub46w9dmxg7l/artifacts/output%2Fnvda_snapshot_try-t10305-19897%2C6ff9867f.exe

@michaelDCurran
Copy link
Member

I shall try to review this on Monday.

@codeofdusk
Copy link
Contributor Author

codeofdusk commented Apr 5, 2020

Perhaps in a separate PR, we may need to rethink how and when we clear the typed word buffer for new text lines in EnhancedTermTypedCharSupport._calculateNewText.

STR:

  1. Enable "speak typed words".
  2. Open Open Console (the 21H1 conhost).
  3. Run git log.
  4. Press q to close the pager.
  5. Type git and press space.

NVDA says "qgit", not "git".

For reference, the 21H1 console (OpenConsole.exe) and Windows Terminal (WindowsTerminal.exe) as of commit microsoft/terminal@4f8acb4 can be found here. On my system, I've replaced the default conhost with this version (see microsoft/terminal#1817). The pre-21H1 console still works as expected after this PR.

This saves us from scanning the old and new lines twice in _calculateNewText (once for diffing and once for finding nonblank indices), and fixes typed word reporting for 21H1 console.

This new approach does not resolve nvaccess#10942.
@michaelDCurran
Copy link
Member

@codeofdusk I'm happy with this. Are you ready for me to merge this?

@codeofdusk
Copy link
Contributor Author

codeofdusk commented Apr 6, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app/windows-terminal New terminal app, potentially supersedes app/windows-console (repo: microsoft/terminal) enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for Windows Terminal
9 participants