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

UI Automation in Windows Console and Windows Terminal: Make POSITION_FIRST and POSITION_LAST relative to visible text in FORMATTED consoles #13671

Closed

Conversation

codeofdusk
Copy link
Contributor

@codeofdusk codeofdusk commented May 8, 2022

Link to issue number:

Closes #13157. Alternative solution to #14021.

Summary of the issue:

The ability for users to explore all console text enabled by #12669 has been well appreciated, but it poses problems in paginated output (less, more, etc.) as the review cursor jump to top/bottom commands are relative to the entire buffer, which can grow quite large.

Description of how this pull request fixes the issue:

Re-introduces a (generalized) ConsoleUIATextInfo to formatted consoles and wt that limits POSITION_FIRST and POSITION_LAST to the first and last visible (not absolute) text range. This textInfo implementation overrides no other behaviour in FORMATTED consoles.

Testing strategy:

Tested that the review cursor works as expected in FORMATTED and IMPROVED consoles and Windows Terminal.

Known issues with pull request:

  • It is now impossible to jump quickly to the top/bottom of the entire buffer.
  • There might be a better way to do this. Initially opening this PR as draft to get feedback on the approach.
  • The jump to top/bottom commands in these terminals now behave differently to the rest of NVDA (you can "jump to top line", then scroll up beyond it). It seems this behaviour is wanted (from discussions in that issue and by email with users) but something of which to be aware.

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

@codeofdusk
Copy link
Contributor Author

Could @michaelDCurran please provide feedback on this approach?

@LeonarddeR
Copy link
Collaborator

How does movement by the page unit behave in consoles? Wouldn't it be better and more consistent to add review commands to move the review cursor by page?

@codeofdusk
Copy link
Contributor Author

How does movement by the page unit behave in consoles?

It isn't defined.

Also CC @carlos-zamora.

@carlos-zamora
Copy link

How does movement by the page unit behave in consoles?

It isn't defined.

Also CC @carlos-zamora.

Hmm. We (Console) probably should define it though. Scrolling by "page" is currently just scrolling by viewport height (at least in Windows Terminal).

@seanbudd
Copy link
Member

Tested that the review cursor works as expected in FORMATTED and IMPROVED consoles and Windows Terminal.

Can you elaborate on this with examples?
It would be great to add system tests even, if possible.

It is now impossible to jump quickly to the top/bottom of the entire buffer.

I think this is a blocker - would it be possible to cache the top and the bottom somehow?

…MATTED consoles.

The ability for users to explore all console text enabled by nvaccess#12669 has been well appreciated, but it poses problems in paginated output (less, more, etc.) as the review cursor jump to top/bottom commands are relative to the entire buffer, which can grow quite large. To ease review of paginated output, make these commands jump relative to visible (not full) text. Note that it is now impossible to jump quickly to the top/bottom of the entire buffer.
Closes nvaccess#13157.
@codeofdusk
Copy link
Contributor Author

Can you elaborate on this with examples?

  1. Start console.
  2. Run git log from a repo.
  3. Press the space bar.
  4. press Shift+numpad 7. NVDA jumps to the top of the currently displayed page.
  5. Press numpad 7 several times. The review cursor should scroll up, past the current page into earlier output in the buffer.

would it be possible to cache the top and the bottom somehow?

How might this help?

@codeofdusk
Copy link
Contributor Author

@seanbudd, @michaelDCurran, @jcsteh any more thoughts on the approach here? This is becoming more relevant with #10964 and Windows Terminal eventually becoming the default console host.

@codeofdusk
Copy link
Contributor Author

Just checking in again on this PR as I'll be out of time to work on it soon. CC @seanbudd.

@seanbudd
Copy link
Member

@codeofdusk - if you want a PR review with the approach as-is, please mark it as ready for review.

If you would like a detailed investigation and general advice on solving #13157, we will need to schedule the work for later - there is no priority for this to jump the queue compared to other PRs.

@codeofdusk
Copy link
Contributor Author

  • This PR is a possible solution for windows terminal: NVDA reads invisible lines when using pagers such as less and more #13157 but I'm concerned about unintended consequences of overriding POSITION_FIRST/POSITION_LAST in this way. Does anything else (besides review top/bottom line) depend on those positions, and will changing those to visible positions only in consoles break other assumptions in NVDA or plugins?
  • I do think that a more general solution should be considered (for instance, movement of review by page and implementation of UNIT_PAGE upstream, or additional scripts for moving to the top/bottom visible line). This would make review consistent across all NVDA controls (i.e. "move to top line" would continue to move to the actual top line of the console buffer, similar to other NVDA controls).
  • That said, this PR preserves muscle memory for longer time terminal users (i.e. "move to top" always moved to the top visible, not actual, line before UI Automation in Windows Console: use UIATextInfo in FORMATTED consoles #12669).

So I'm not quite sure what to do with this PR. Maybe @LeonarddeR or @michaelDCurran could give input?

@seanbudd
Copy link
Member

The risks you describe with this PR seems like something a detailed investigation requires, or extensive testing on alpha.

I think #14021 is a safer approach. If it ends up not being comprehensive in fixing this issue, e.g. the muscle memory problem is too user hostile, we can look at the approach in this PR.

@codeofdusk
Copy link
Contributor Author

I'm actually on the phone with @carlos-zamora right now! He agrees that #14021 makes more sense and is willing to implement the needed changes on the terminal side. Closing this PR in favour of that one.

@codeofdusk codeofdusk closed this Aug 16, 2022
seanbudd pushed a commit that referenced this pull request Aug 17, 2022
Closes #13157 (given microsoft/terminal#13756). Supercedes #13671.

Summary of the issue:
There is no way to move the review cursor by page. This is especially useful in terminals as a "page" can be defined as a screenful of text, solving #13157 without violating any other assumptions in NVDA and still permitting easy jump to the absolute top/bottom of the terminal buffer using the currently existing scripts.

Description of how this pull request fixes the issue:
Added new scripts (bound to NVDA+pageUp/NVDA+pageDown on desktop and NVDA+Shift+pageUp/NVDA+Shift+pageDown on laptop) to move to previous/next page respectively.
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.

windows terminal: NVDA reads invisible lines when using pagers such as less and more
4 participants