Adding TextPattern providers to Text controls (port from 5.0) #3853
Conversation
- Add ITextRange and ITextProvider interfaces as a base of TextPattern
Refer to:
https://docs.microsoft.com/dotnet/api/system.windows.automation.provider.itextprovider
https://docs.microsoft.com/dotnet/api/system.windows.automation.provider.itextrangeprovider
- Implement UiaTextRange and UiaTextProvider to support text pattern in textbox controls
- Add tests
- Move IAutomationLiveRegion to Primitives
(cherry picked from commit 7545756)
- Implement base integration to AccessibleObject - Add TextPattern support for TextBox (singleline and multiline) and MaskedTextBox - Disable managed UiaTextProvider for RichTextBox to use native provider as the native Win32 RichEdit control already supports TextPattern. Return ControlAccessibleObject as AccessibilityInstance instead TextBoxBaseAccessibleObject (the latter supports managed UiaTextProvider). - Add tests Fixes dotnet#1588 (cherry picked from commit f2baffc)
vladimir-krestov
added
the
waiting-review
RussKie
changed the title
RussKie
added
Priority:1
|
|
||
| // Convert screen to client coordinates. | ||
| // (Essentially ScreenToClient but MapWindowPoints accounts for window mirroring using WS_EX_LAYOUTRTL.) | ||
| if (MapWindowPoints(new HandleRef(null, IntPtr.Zero), new HandleRef(this, _owningTextBoxBase.Handle), ref clientLocation, 1) == 0) |
There was a problem hiding this comment.
This is technically incorrect. HandleRef should have the object that will potentially close the handle if disposed. this does have a handle to _owningTextBoxBase so that should root it. We should fix this in master and we should implement the IHandle overload on the P/Invoke as we do in other places to avoid this sort of mistake. Additionally we already have an overload that takes IntPtr for the first argument, we shouldn't be wrapping nothing in a HandleRef or at the very least we can pass default.
There was a problem hiding this comment.
I implemented IHandle overload for MapWindowPoints like this.
I orientated to SendMessageW methods.
And now the method call looks like this:
Is it correct?
I added this implementation to .Net 5.0-release in #3865
|
|
||
| public override int LinesCount | ||
| => _owningTextBoxBase.IsHandleCreated | ||
| ? (int)(long)SendMessageW(new HandleRef(this, _owningTextBoxBase.Handle), (WM)EM.GETLINECOUNT) |
There was a problem hiding this comment.
Same comment as above. In this case we already have an IHandle overload so you can just pass _owningTextBoxBase.
|
|
||
| public abstract void SetSelection(int start, int end); | ||
|
|
||
| public ES GetEditStyle(IntPtr hWnd) => hWnd != IntPtr.Zero ? (ES)GetWindowLong(new HandleRef(null, hWnd), GWL.STYLE) : ES.LEFT; |
There was a problem hiding this comment.
Why are we not rooting the objects that own the handles in these three methods (e.g. passing in IHandle or HandleRef)? Also, the check for a null HWND here indicates that we may be hiding some other bug- where are we passing in a null handle?
|
|
||
| public override WS_EX WindowExStyle | ||
| => _owningTextBoxBase.IsHandleCreated | ||
| ? GetWindowExStyle(_owningTextBoxBase.Handle) |
There was a problem hiding this comment.
These calls are problematic as we're not keeping the _owningTextBoxBase rooted by pulling out the Handle and P\Invoking in the base class.
vladimir-krestov
removed
the
waiting-review
ghost
locked as resolved and limited conversation to collaborators
Fixes #1588
Related PR: #2701 (see comments for more information)
Proposed changes
Customer Impact
Regression?
Risk
Before
After
Test methodology
Accessibility testing
Test environment(s)
Microsoft Reviewers: Open in CodeFlow