Properly handle and test "end of buffer" in UiaTextRange #6986
Assignees
Labels
Area-Accessibility
Issues related to accessibility
Issue-Task
It's a feature request, but it doesn't really need a major design.
Product-Conhost
For issues in the Console codebase
Product-Terminal
The new Windows Terminal.
Resolution-Fix-Committed
Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Comments
carlos-zamora
added
Product-Conhost
ghost
added
the
Needs-Triage
|
This issue was added to a terminal milestone, not Windows. Does this mean it won't make it into 21H1? |
|
@codeofdusk nope, those milestones are just our rough buckets for "when things should land by". Because Terminal 1.3 is an earlier milestone than 21H1, even though they have different shipping vehicles, this fix will ship in Windows around when Terminal 1.3 ships (if not slightly before) |
DHowett
removed
the
Needs-Triage
Does this then mean that all the UIA work (#4018 etc) could ship before 21H1? nvaccess/nvda#10964 might need to be adjusted in that case. |
ghost
pushed a commit
that referenced
this issue
Jul 20, 2020
In UiaTextRange, `_getBufferSize` returns an optimized version of the size of the buffer to be the origin and the last character in the buffer. This is to improve performance on search or checking if you are currently on the last word/line. When setting the endpoint and drawing the bounding rectangles, we should be retrieving the true buffer size. This is because it is still possible to create UiaTextRanges that are outside of this optimized size. The main source of this is `ExpandToEnclosingUnit()` when the unit is `Document`. The end _should_ be the last visible character, but it isn't because that would break our tests. This is an incomplete solution. #6986 is a follow up to completely test and implement the solution. The crash in #6402 was caused by getting the document range (a range of the full text buffer), then moving the end by one character. When we get the document range, we get the optimized size of the buffer (the position of the last character). Moving by one character is valid because the buffer still has more to explore. We then crash from checking if the new position is valid on the **optimized size**, not the **real size**. REFERENCES #6986 - follow up to properly handle/test this "end of buffer" problem Closes #6402
DHowett
pushed a commit
that referenced
this issue
Jul 20, 2020
In UiaTextRange, `_getBufferSize` returns an optimized version of the size of the buffer to be the origin and the last character in the buffer. This is to improve performance on search or checking if you are currently on the last word/line. When setting the endpoint and drawing the bounding rectangles, we should be retrieving the true buffer size. This is because it is still possible to create UiaTextRanges that are outside of this optimized size. The main source of this is `ExpandToEnclosingUnit()` when the unit is `Document`. The end _should_ be the last visible character, but it isn't because that would break our tests. This is an incomplete solution. #6986 is a follow up to completely test and implement the solution. The crash in #6402 was caused by getting the document range (a range of the full text buffer), then moving the end by one character. When we get the document range, we get the optimized size of the buffer (the position of the last character). Moving by one character is valid because the buffer still has more to explore. We then crash from checking if the new position is valid on the **optimized size**, not the **real size**. REFERENCES #6986 - follow up to properly handle/test this "end of buffer" problem Closes #6402 (cherry picked from commit c390b61)
DHowett
pushed a commit
that referenced
this issue
Jul 20, 2020
In UiaTextRange, `_getBufferSize` returns an optimized version of the size of the buffer to be the origin and the last character in the buffer. This is to improve performance on search or checking if you are currently on the last word/line. When setting the endpoint and drawing the bounding rectangles, we should be retrieving the true buffer size. This is because it is still possible to create UiaTextRanges that are outside of this optimized size. The main source of this is `ExpandToEnclosingUnit()` when the unit is `Document`. The end _should_ be the last visible character, but it isn't because that would break our tests. This is an incomplete solution. #6986 is a follow up to completely test and implement the solution. The crash in #6402 was caused by getting the document range (a range of the full text buffer), then moving the end by one character. When we get the document range, we get the optimized size of the buffer (the position of the last character). Moving by one character is valid because the buffer still has more to explore. We then crash from checking if the new position is valid on the **optimized size**, not the **real size**. REFERENCES #6986 - follow up to properly handle/test this "end of buffer" problem Closes #6402 (cherry picked from commit c390b61)
To the insider program. I can't comment on the official Windows ship cycle from here, though. 😄 |
|
In general, though, my "21H1" or "21H2" milestones very roughly track the Windows release. Since I can't comment on or commit to these dates on behalf of my organization, they're as good as advisory for my team and our partners. It's some amalgamation of "the last day we could possibly get features in" and "the day the bugfix bar drops", which is long before we sign off on a final build and ship the release to the general public. |
This was referenced Jul 20, 2020
This was referenced Sep 28, 2020
Closed
This was referenced May 13, 2021
4 tasks
2 tasks
ghost
added
Resolution-Fix-Committed
ghost
pushed a commit
that referenced
this issue
Sep 16, 2021
## Summary of the Pull Request Updates our `UiaTextRange` to no longer treat the end of the buffer as the "document end". Instead, we consider the "document end" to be the line beneath the cursor or last legible character (whichever is further down). In the event where the last legible character is on the last line of the buffer, we use the "end exclusive" position (left-most point on a line one past the end of the buffer). When movement of any kind occurs, we clamp each endpoint to the document end. Since the document end is an actual spot in the buffer (most of the time), this should improve stability because we shouldn't be pointing out-of-bounds anymore. The biggest benefit is that this significantly improves the performance of word navigation because screen readers no longer have to take into account the whitespace following the end of the prompt. Word navigation tests were added to the `TestTableWriter` (see #10886). 24 of the 85 tests were failing, however, they don't seem to interact with the document end, so I've marked them as skip and will fix them in a follow-up. This PR is large enough as-is, so I'm hoping I can take time in the follow-up to clean some things on the side (aka `preventBoundary` and `allowBottomExclusive` being used interchangeably). ## References #7000 - Epic Closes #6986 Closes #10925 ## Validation Steps Performed - [X] Tests pass - [X] @codeofdusk has been personally testing this build (and others)
|
🎉This issue was addressed in #11122, which has now been successfully released as Handy links: |
Closed
This issue was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When moving by unit, UiaTextRange keeps track of the last character to be the "end of the buffer". This provides significant performance improvements in our accessibility model for word navigation in particular.
However, it is still possible to create a UiaTextRange outside of that optimized buffer size. This has lead to a few bugs like #6402 and ##5243 across Terminal and ConHost.
This issue tracks taking the time to properly test this and polish this architectural change. Especially considering that this breaks a significant number of tests that already exist.
The text was updated successfully, but these errors were encountered: