Commit 985f9cb6 authored by Benjamin Beaudry's avatar Benjamin Beaudry Committed by Commit Bot

Remove IsReadOnly condition in IsUIAControl for views

We initially added IsUIAControl with a different implementation for
web content nodes and views nodes. For web content, we have this great
and complete implementation that got refined over time, but for views,
we never really updated it - it's basic.

Turns out that the value returned by IsUIAControl is incorrect for views
that are marked as read-only. For example, click on the "Try it" button
in the following link and try to focus the read-only address bar of the
new window. It doesn't work.
https://www.w3schools.com/jsref/tryit.asp?filename=tryjsref_win_open3

My first solution had the intent of unifying the implementations for web
content and for views. However, that caused issues with menus and
sub-menus when using Narrator, so I instead decided to go with a safer
approach - simply remove the IsReadOnly condition. That condition doesn't
have its place there and everything works better without.

I tried this on Chromium with and without UIA and on Edge, with UIA.
Everything still works great!

editable one.

AX-Relnotes: A read-only omnibox is now accessible to ATs, just like an
Bug: N/A
Change-Id: Ib551701e0220a3422c989b271ed5edfad15f060e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2490846Reviewed-by: default avatarNektarios Paisios <nektar@chromium.org>
Reviewed-by: default avatarKurt Catti-Schmidt <kschmi@microsoft.com>
Reviewed-by: default avatarJacques Newman <janewman@microsoft.com>
Commit-Queue: Benjamin Beaudry <benjamin.beaudry@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#820269}
parent 99646a9a
......@@ -7191,8 +7191,7 @@ bool AXPlatformNodeWin::IsUIAControl() const {
} // end of web-content only case.
const AXNodeData& data = GetData();
return !((IsReadOnlySupported(data.role) && data.IsReadOnlyOrDisabled()) ||
data.HasState(ax::mojom::State::kInvisible) ||
return !(data.HasState(ax::mojom::State::kInvisible) ||
(data.IsIgnored() && !data.HasState(ax::mojom::State::kFocusable)));
}
......
......@@ -28,6 +28,16 @@ using Microsoft::WRL::ComPtr;
namespace views {
namespace test {
#define EXPECT_UIA_BOOL_EQ(node, property_id, expected) \
{ \
ScopedVariant expectedVariant(expected); \
ASSERT_EQ(VT_BOOL, expectedVariant.type()); \
ScopedVariant actual; \
ASSERT_HRESULT_SUCCEEDED( \
node->GetPropertyValue(property_id, actual.Receive())); \
EXPECT_EQ(expectedVariant.ptr()->boolVal, actual.ptr()->boolVal); \
}
namespace {
// Whether |left| represents the same COM object as |right|.
......@@ -62,6 +72,13 @@ class ViewAXPlatformNodeDelegateWinTest : public ViewsTestBase {
ASSERT_EQ(S_OK, view_accessible.As(&service_provider));
ASSERT_EQ(S_OK, service_provider->QueryService(IID_IAccessible2_2, result));
}
ComPtr<IRawElementProviderSimple> GetIRawElementProviderSimple(View* view) {
ComPtr<IRawElementProviderSimple> result;
EXPECT_HRESULT_SUCCEEDED(view->GetNativeViewAccessible()->QueryInterface(
__uuidof(IRawElementProviderSimple), &result));
return result;
}
};
TEST_F(ViewAXPlatformNodeDelegateWinTest, TextfieldAccessibility) {
......@@ -488,5 +505,30 @@ TEST_F(ViewAXPlatformNodeDelegateWinTest, GridRowColumnCount) {
EXPECT_EQ(E_UNEXPECTED, grid_provider->get_RowCount(&row_count));
EXPECT_EQ(E_UNEXPECTED, grid_provider->get_ColumnCount(&column_count));
}
TEST_F(ViewAXPlatformNodeDelegateWinTest, IsUIAControlIsTrueEvenWhenReadonly) {
// This test ensures that the value returned by
// AXPlatformNodeWin::IsUIAControl returns true even if the element is
// read-only. The previous implementation was incorrect and used to return
// false for read-only views, causing all sorts of issues with ATs.
//
// Since we can't test IsUIAControl directly, we go through the
// UIA_IsControlElementPropertyId, which is computed using IsUIAControl.
Widget widget;
Widget::InitParams init_params = CreateParams(Widget::InitParams::TYPE_POPUP);
init_params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
widget.Init(std::move(init_params));
View* content = widget.SetContentsView(std::make_unique<View>());
Textfield* text_field = new Textfield();
text_field->SetReadOnly(true);
content->AddChildView(text_field);
ComPtr<IRawElementProviderSimple> textfield_provider =
GetIRawElementProviderSimple(text_field);
EXPECT_UIA_BOOL_EQ(textfield_provider, UIA_IsControlElementPropertyId, true);
}
} // namespace test
} // namespace views
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment