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

Fix movement at end of document for modern UIA providers #12898

Closed
wants to merge 1 commit into from

Conversation

codeofdusk
Copy link
Contributor

@codeofdusk codeofdusk commented Oct 2, 2021

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 POSITION_LAST (i.e. collapsed at the very end of the document).

Description of how this pull request fixes the issue:

If a UIA provider reports that a collapsed range has successfully been moved forward (i.e. move returns nonzero), but the new range is collapsed at POSITION_LAST, return 0 and restore the range to its original position.

Testing strategy:

Tested that movement in review works as expected in Word, FORMATTED consoles, and Edgium.

Known issues with pull request:

This might have a slight performance impact, but I've tried to optimize this so that we only make extra UIA calls for this specific case.

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

codeofdusk added a commit to codeofdusk/nvda that referenced this pull request Oct 2, 2021
)

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 POSITION_LAST (i.e. collapsed at the very end of the document).

If a UIA provider reports that a collapsed range has successfully been moved forward (i.e. move returns nonzero), but the new range is collapsed at POSITION_LAST, return 0 and restore the range to its original position.
@codeofdusk
Copy link
Contributor Author

Marking this for review, though the build is failing and I can't figure out why.

@codeofdusk codeofdusk marked this pull request as ready for review October 2, 2021 22:31
@codeofdusk codeofdusk requested a review from a team as a code owner October 2, 2021 22:31
codeofdusk added a commit to codeofdusk/nvda that referenced this pull request Oct 2, 2021
)

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 POSITION_LAST (i.e. collapsed at the very end of the document).

If a UIA provider reports that a collapsed range has successfully been moved forward (i.e. move returns nonzero), but the new range is collapsed at POSITION_LAST, return 0 and restore the range to its original position.
codeofdusk added a commit to codeofdusk/nvda that referenced this pull request Oct 2, 2021
)

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 POSITION_LAST (i.e. collapsed at the very end of the document).

If a UIA provider reports that a collapsed range has successfully been moved forward (i.e. move returns nonzero), but the new range is collapsed at POSITION_LAST, return 0 and restore the range to its original position.
codeofdusk added a commit to codeofdusk/nvda that referenced this pull request Oct 2, 2021
)

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 POSITION_LAST (i.e. collapsed at the very end of the document).

If a UIA provider reports that a collapsed range has successfully been moved forward (i.e. move returns nonzero), but the new range is collapsed at POSITION_LAST, return 0 and restore the range to its original position.
@LeonarddeR
Copy link
Collaborator

This pr doesn't solve the following case:

>>> nav.makeTextInfo("last").move("character", 1)
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "NVDAObjects\UIA\web.pyc", line 186, in move
  File "NVDAObjects\UIA\web.pyc", line 177, in _collapsedMove
  File "NVDAObjects\UIA\web.pyc", line 144, in _moveToEdgeOfReplacedContent
  File "baseObject.pyc", line 26, in __get__
  File "NVDAObjects\UIA\web.pyc", line 115, in _get_UIAElementAtStartWithReplacedContent
  File "monkeyPatches\comtypesMonkeyPatches.pyc", line 30, in __call__
_ctypes.COMError: (-2147467259, 'Unspecified error', (None, None, None, 0, None))

How are those degenerate ranges supposed to behave? Is it expected behaviour that you can't move from position last at all, even not backwards?

@codeofdusk
Copy link
Contributor Author

How are those degenerate ranges supposed to behave? Is it expected behaviour that you can't move from position last at all, even not backwards?

Word and FORMATTED conhost return 0 for these.

@seanbudd
Copy link
Member

seanbudd commented Oct 4, 2021

Marking this for review, though the build is failing and I can't figure out why.

Apologies, ignore the build failure, we had to fix something to work with the latest Windows SDK - see #12903.
I've triggered another build for this PR now - hopefully it works.

@codeofdusk
Copy link
Contributor Author

Thanks @seanbudd. It looks like this PR built successfully so is now ready for review.

@LeonarddeR
Copy link
Collaborator

Note that this pr replaces #12825.

Before going further on this, I think we should define expected behavior for the following case:

  1. Open Notepad or Word, respectively
  2. Write a line of text.
  3. Place your caret at the first character of the line
  4. Open a python console
  5. Execute review.move("line", 1)

Actual behavior for Notepad: Method returns 0, textInfo still collapsed at start of line
Actual behavior for Word: Method returns 1, textInfo collapsed at end of line (end of document)

I think both make sense equally, with a slight preference for the UIA behavior. This pr changes that. I think we should agree on behavior first before continuing.

@codeofdusk
Copy link
Contributor Author

I think both make sense equally, with a slight preference for the UIA behavior

The UIA implementation causes the following problem:

  1. At the start of the last line, the "move review to next line" command is invoked.
  2. The review info is expanded to line.
  3. The review info is collapsed at start.
  4. The review info is moved down one line (i.e. back to the end of the same line).
  5. Some UIA text ranges have no apparent bottom #12808 (i.e. there is no bottom).

@LeonarddeR
Copy link
Collaborator

Yes, I see. My question intends to bring the discussion on a more fundamental level, i.e. what do we expect? We could patch the UIA implementation but could also consider changing NVDA"s behavior.

Additional details:

  1. The Word object model behaves similar to UIA. There is a workaround in NVDAObjects.window.winword.WordDocumentTextInfo
  2. The problem in Some UIA text ranges have no apparent bottom #12808 also applies to wordpad when using ITextDocumentTextInfo. This is described in Regression in rich edit controls since 2015.2 #5275
  3. In an MSHTML textarea, the review cursor reports bottom when moving to the bottom, but not before it mentions an empty blank line that in reality does not exist.

So to summarize, the behavior I describe in #12898 (comment) when moving by line from the start of the last line of the document to return 0 and stay on the start of the last line only applies to offset based textInfo.

@codeofdusk
Copy link
Contributor Author

How do you suggest changing NVDA's behaviour?

@LeonarddeR
Copy link
Collaborator

If we change the move method on OffsetsTextinfo in such a way that it moves to the end of the range when moving from the start of the last line, we can change the global command to fetch the last position of the textInfo

api.getReviewPosition().obj.makeTextInfo(textInfos.POSITION_LAST)

Then, we can compare the new position with that and report bottom appropriately. This is somehow similar to how script_review_nextCharacter works to ensure that the review cursor doesn't move past the end of the line.

@codeofdusk
Copy link
Contributor Author

I've attempted to implement this change in #12916. Your review would be appreciated!

@codeofdusk codeofdusk marked this pull request as draft October 7, 2021 20:50
codeofdusk added a commit to codeofdusk/nvda that referenced this pull request Oct 19, 2021
)

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 POSITION_LAST (i.e. collapsed at the very end of the document).

If a UIA provider reports that a collapsed range has successfully been moved forward (i.e. move returns nonzero), but the new range is collapsed at POSITION_LAST, return 0 and restore the range to its original position.
@codeofdusk codeofdusk changed the base branch from master to beta October 19, 2021 06:07
)

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 POSITION_LAST (i.e. collapsed at the very end of the document).

If a UIA provider reports that a collapsed range has successfully been moved forward (i.e. move returns nonzero), but the new range is collapsed at POSITION_LAST, return 0 and restore the range to its original position.
@codeofdusk codeofdusk changed the base branch from beta to master October 19, 2021 06:36
@codeofdusk
Copy link
Contributor Author

Closing in favour of #12960.

@codeofdusk codeofdusk closed this Oct 19, 2021
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.

Some UIA text ranges have no apparent bottom
4 participants