-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Support UIA ::SetFocus and basic property change notifications #11674
Changes from 10 commits
4501e34
71f585d
89cb2c6
184aeb7
62b08ec
a393db9
b5133e6
8084d43
1ba32fe
78f0709
f477f29
1c290ff
1915b7b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"type": "prerelease", | ||
"comment": "Add UIA focus management and AdvisedEvents for prop changes", | ||
"packageName": "react-native-windows", | ||
"email": "[email protected]", | ||
"dependentChangeType": "patch" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,13 @@ | ||
#include "pch.h" | ||
#include "CompositionRootAutomationProvider.h" | ||
#include <algorithm> | ||
#include "UiaHelpers.h" | ||
#pragma warning(push) | ||
#pragma warning(disable : 4229) | ||
#define IN | ||
#define OUT | ||
#include <atlsafe.h> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please do not use ATLSafe.h There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this PR adds SAFEARRAY support to WIL which we should be using instead: could you perhaps work with @chrisglein 's team to see that PR through and use it here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +@carlos-zamora who filed the original wil issue. Can you help? :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure! Here's the way we did it in the Terminal repo: And here's a quick example of us using it: Hope this helps! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice, I wasn't aware of the WIL API. @asklar I've been pulled off this work for now. Would you be OK with me just using the SAFEARRAY directly and opening an issue to move this to the WIL stuff? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jonthysell we can interface manually (which I think is what @FalseLobster is proposing) but it's going to be a lot more code than necessary and hard to maintain/reason over. WIL is header only and designed to simplify calling win32 apis so your code becomes a lot more concise. Ideally we can move to that sooner than later. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @FalseLobster I'm okay with taking things as you've written them and opening an issue to replace ATL with WIL and assign it to me. There's probably other places the (Fabric) codebase could benefit from access to WIL helpers to deal with all this old Win32 stuff. @asklar Yes, WIL is header only, but it's also "yet another re-wrapping of standard C++" to add to this project. Like do we really need every implementation of smart pointers that have ever been made? We have stl, boost, Mso, cppwinrt, now wil. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the alternative being used here is to use ATL. We should not be using ATL. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @asklar , @jonthysell , could we avoid dependency on the new library? C++/WinRT is also header-only library, and we know the overall impact on the developer experience. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. C++/WinRT is for working with WinRT, but these are not WinRT apis, so C++/WinRT doesn't help in this case. Feel free to use SAFEARRAY directly. |
||
#pragma warning(pop) | ||
|
||
namespace winrt::Microsoft::ReactNative::implementation { | ||
|
||
|
@@ -32,7 +39,7 @@ HRESULT __stdcall CompositionRootAutomationProvider::GetEmbeddedFragmentRoots(SA | |
} | ||
|
||
HRESULT __stdcall CompositionRootAutomationProvider::SetFocus(void) { | ||
return S_OK; | ||
return UiaSetFocusHelper(m_view); | ||
} | ||
|
||
HRESULT __stdcall CompositionRootAutomationProvider::GetPatternProvider(PATTERNID patternId, IUnknown **pRetVal) { | ||
|
@@ -122,10 +129,7 @@ HRESULT __stdcall CompositionRootAutomationProvider::ElementProviderFromPoint( | |
return UIA_E_ELEMENTNOTAVAILABLE; | ||
} | ||
|
||
auto spRootView = strongView->rootComponentView(); | ||
if (spRootView == nullptr) { | ||
return UIA_E_ELEMENTNOTAVAILABLE; | ||
} | ||
auto spRootView = std::static_pointer_cast<::Microsoft::ReactNative::RootComponentView>(strongView); | ||
|
||
if (m_hwnd == nullptr || !IsWindow(m_hwnd)) { | ||
// TODO: Add support for non-HWND based hosting | ||
|
@@ -189,4 +193,126 @@ HRESULT __stdcall CompositionRootAutomationProvider::Navigate( | |
return S_OK; | ||
} | ||
|
||
// The old C-style interface for SAFEARRAY is tedious to use. ATL provides CComSafeArray which allows us to interact | ||
// with the data structure using much more modern methods. However, AdviseEventAdded/Removed don't expect us to | ||
// deallocate their In param, so this is a simple RAII wrapper to Attach/Detach in the scope of those functions and | ||
// perform a little validation that the incoming SAFEARRAY is well formed. | ||
class UIAPropertyArray { | ||
CComSafeArray<PROPERTYID> m_propArray{}; | ||
|
||
public: | ||
UIAPropertyArray(SAFEARRAY *psaProperties) noexcept { | ||
VARTYPE vt; | ||
if (psaProperties && SUCCEEDED(SafeArrayGetVartype(psaProperties, &vt)) && vt == m_propArray.GetType()) { | ||
m_propArray.Attach(psaProperties); | ||
} | ||
} | ||
~UIAPropertyArray() noexcept { | ||
if (m_propArray.GetSafeArrayPtr() != nullptr) | ||
m_propArray.Detach(); | ||
} | ||
bool IsValid() noexcept { | ||
return m_propArray.GetSafeArrayPtr() != nullptr && m_propArray.GetDimensions() == 1 && m_propArray.GetCount() > 0; | ||
} | ||
CComSafeArray<PROPERTYID> *operator->() noexcept { | ||
return &m_propArray; | ||
} | ||
}; | ||
|
||
void AdviseEventAddedImpl( | ||
std::vector<CompositionRootAutomationProvider::AdvisedEvent> &advisedEvents, | ||
EVENTID idEvent) noexcept { | ||
auto it = std::find_if( | ||
advisedEvents.begin(), | ||
advisedEvents.end(), | ||
[idEvent](const CompositionRootAutomationProvider::AdvisedEvent &ae) noexcept { return ae.Event == idEvent; }); | ||
|
||
if (it == advisedEvents.end()) { | ||
advisedEvents.emplace_back(CompositionRootAutomationProvider::AdvisedEvent{idEvent, 1 /*Count*/}); | ||
} else { | ||
it->Count++; | ||
} | ||
} | ||
|
||
HRESULT CompositionRootAutomationProvider::AdvisePropertiesAdded(SAFEARRAY *psaProperties) noexcept { | ||
UIAPropertyArray props(psaProperties); | ||
|
||
if (!props.IsValid()) { | ||
return E_INVALIDARG; | ||
} | ||
|
||
// Note SAFEARRAY's upperbound is inclusive | ||
for (auto i = props->GetLowerBound(); i <= props->GetUpperBound(); i++) { | ||
auto prop = props->GetAt(i); | ||
AdviseEventAddedImpl(m_advisedProperties, prop); | ||
} | ||
return S_OK; | ||
} | ||
|
||
HRESULT CompositionRootAutomationProvider::AdviseEventAdded(EVENTID idEvent, SAFEARRAY *psaProperties) { | ||
if (idEvent == UIA_AutomationPropertyChangedEventId) { | ||
return AdvisePropertiesAdded(psaProperties); | ||
} | ||
AdviseEventAddedImpl(m_advisedEvents, idEvent); | ||
return S_OK; | ||
} | ||
|
||
HRESULT AdviseEventRemovedImpl( | ||
std::vector<CompositionRootAutomationProvider::AdvisedEvent> &advisedEvents, | ||
EVENTID idEvent) noexcept { | ||
auto it = std::find_if( | ||
advisedEvents.begin(), | ||
advisedEvents.end(), | ||
[idEvent](const CompositionRootAutomationProvider::AdvisedEvent &ae) noexcept { return ae.Event == idEvent; }); | ||
|
||
if (it == advisedEvents.end()) { | ||
assert(false); | ||
return UIA_E_INVALIDOPERATION; | ||
} else if (it->Count == 1) { | ||
advisedEvents.erase(it); | ||
} else { | ||
it->Count--; | ||
} | ||
return S_OK; | ||
} | ||
|
||
HRESULT CompositionRootAutomationProvider::AdvisePropertiesRemoved(SAFEARRAY *psaProperties) noexcept { | ||
UIAPropertyArray props(psaProperties); | ||
|
||
if (!props.IsValid()) { | ||
return E_INVALIDARG; | ||
} | ||
|
||
auto returnHr = S_OK; | ||
for (auto i = props->GetLowerBound(); i <= props->GetUpperBound(); i++) { | ||
auto prop = props->GetAt(i); | ||
auto hr = AdviseEventRemovedImpl(m_advisedProperties, prop); | ||
if (FAILED(hr)) { | ||
returnHr = hr; | ||
} | ||
} | ||
return returnHr; | ||
} | ||
|
||
HRESULT | ||
CompositionRootAutomationProvider::AdviseEventRemoved(EVENTID idEvent, SAFEARRAY *psaProperties) { | ||
if (idEvent == UIA_AutomationPropertyChangedEventId) { | ||
return AdvisePropertiesRemoved(psaProperties); | ||
} | ||
|
||
return AdviseEventRemovedImpl(m_advisedEvents, idEvent); | ||
} | ||
|
||
bool CompositionRootAutomationProvider::WasEventAdvised(EVENTID event) noexcept { | ||
return std::any_of(m_advisedEvents.begin(), m_advisedEvents.end(), [event](const AdvisedEvent &ae) { | ||
return ae.Event == event && ae.Count > 0; | ||
}); | ||
} | ||
|
||
bool CompositionRootAutomationProvider::WasPropertyAdvised(PROPERTYID prop) noexcept { | ||
return std::any_of(m_advisedProperties.begin(), m_advisedProperties.end(), [prop](const AdvisedEvent &ae) { | ||
return ae.Property == prop && ae.Count > 0; | ||
}); | ||
} | ||
|
||
} // namespace winrt::Microsoft::ReactNative::implementation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this for?
SAFEARRAY
?Most of the code is using std::vector, so where's the SAFEARRAY dependency being forced?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea, a lot of the UIA APIs use SAFEARRAY for certain parameters.