Skip to content

Verify success of forward review moves by word and line#12960

Merged
seanbudd merged 5 commits into
nvaccess:masterfrom
codeofdusk:t12808v3
Oct 21, 2021
Merged

Verify success of forward review moves by word and line#12960
seanbudd merged 5 commits into
nvaccess:masterfrom
codeofdusk:t12808v3

Conversation

@codeofdusk

@codeofdusk codeofdusk commented Oct 19, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Link to issue number:

Closes #12808.

Summary of the issue:

In Microsoft Word, new builds of Windows Console, and Edgium with UIA enabled, "bottom" is not reported when moving past the end of the document in review. These providers return nonzero when moving collapsed textInfos forward past the end, but simply set them to the exclusive end of the document.

Description of how this pull request fixes the issue:

When moving forward by word or line, maintain the old position and verify that the new position is after the previous.

Testing strategy:

Tested in affected modern providers, notepad with MSAA, Chrome with IA2, and the MS Word object model.

Known issues with pull request:

None known.

Change log entries:

== Bug Fixes ==

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

michaelDCurran and others added 2 commits October 19, 2021 10:41
@michaelDCurran @codeofdusk
review_nextLine script: even if move by line succeeds, double check t…
5bbf2e9 
…hat the new line does in deed start after the original review cursor position. Otherwise, we have hit the bottom.
@codeofdusk
Cleanup and apply fix to next word script as well.
d9741c3
This was referenced Oct 19, 2021
Closed
Closed
@codeofdusk codeofdusk marked this pull request as ready for review October 19, 2021 16:12
@codeofdusk codeofdusk requested a review from a team as a code owner October 19, 2021 16:12
@codeofdusk codeofdusk requested a review from seanbudd October 19, 2021 16:12
@michaelDCurran

michaelDCurran commented Oct 20, 2021

Copy link
Copy Markdown
Member

"...but simply set them to POSITION_LAST (i.e. collapsed at the very end of the document)."
This statement is not correct. NVDA's POSITION_LAST represents the start of the last character, not the (exclusive) end of the content/document.
However, the intention of the description is accurate, so just remove any mention of POSITION_LAST.

@codeofdusk

codeofdusk commented Oct 20, 2021

Copy link
Copy Markdown
Contributor Author

Description updated.

seanbudd
seanbudd previously approved these changes Oct 21, 2021
View reviewed changes

@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 @codeofdusk - I've left some minor fixes for the comments.

Tested in affected modern providers.

Have you tested these commands in other scenarios (eg not UIA)?

Comment thread source/globalCommands.py Outdated
Comment thread source/globalCommands.py Outdated
@seanbudd seanbudd dismissed their stale review October 21, 2021 01:16

was meant to be a comment not an approval

@codeofdusk @seanbudd
Apply suggestions from code review
0a712b0 
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
@codeofdusk

codeofdusk commented Oct 21, 2021

Copy link
Copy Markdown
Contributor Author

Have you tested these commands in other scenarios (eg not UIA)?

Yes, tested in Notepad with MSAA.

@AppVeyorBot

AppVeyorBot commented Oct 21, 2021

Copy link
Copy Markdown

See test results for failed build of commit 3c564a6699

@seanbudd seanbudd merged commit 1f13d45 into nvaccess:master Oct 21, 2021
@nvaccessAuto nvaccessAuto added this to the 2022.1 milestone Oct 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seanbudd seanbudd seanbudd approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2022.1

Development

Successfully merging this pull request may close these issues.

Some UIA text ranges have no apparent bottom

5 participants

@codeofdusk @michaelDCurran @AppVeyorBot @seanbudd @nvaccessAuto