|
@@ -0,0 +1,257 @@
|
|
|
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
|
+From: Julie Jeongeun Kim <[email protected]>
|
|
|
+Date: Wed, 8 Apr 2020 11:18:34 +0000
|
|
|
+Subject: a11y: AXPlatformNodeBase::GetIndexInParent() returns base::Optional
|
|
|
+
|
|
|
+This fixes the regression issue from the CL[1] which moved
|
|
|
+the logic of GetIndexInParent to AXPlatformNodeBase.
|
|
|
+
|
|
|
+Before the CL[1], atk_object::GetIndexInParent returns -1 as an
|
|
|
+invalid value but AXPlatformNodeBase::GetIndexInParent returns
|
|
|
+positive numbers or 0 except the case when it's not implemented
|
|
|
+or doesn't have |delegate_|. AXPlatformNodeBase::GetIndexInParent
|
|
|
+is also used for the internal logic in AXPlatformNodeBase with
|
|
|
+the assumption that the return value is not less than 0.
|
|
|
+The CL[1] moved the logic to AXPlatformNodeBase::GetIndexInParent
|
|
|
+with keeping the rule that the return value is not less than 0.
|
|
|
+
|
|
|
+The problem is that screen reader such as Orca expects that it
|
|
|
+returns -1 when it's invalid.
|
|
|
+
|
|
|
+With this CL, AXPlatformNodeBase::GetIndexInParent() returns
|
|
|
+base::Optional and the caller has -1 if GetIndexInParent() returns
|
|
|
+base::nullopt. It also uses GetFirstChild and GetNextSibling for
|
|
|
+children iteration in GetHypertextOffsetFromChild() and
|
|
|
+GetHypertextOffsetFromEndpoint() instead of GetIndexInParent and
|
|
|
+GetChildAt.
|
|
|
+
|
|
|
+[1]https://crrev.com/c/2087234
|
|
|
+
|
|
|
+Bug: 1065534
|
|
|
+Change-Id: I00e2ed421ed263cf379ba22f55049fd52d32df9b
|
|
|
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2129667
|
|
|
+Commit-Queue: Julie Kim <[email protected]>
|
|
|
+Reviewed-by: Nektarios Paisios <[email protected]>
|
|
|
+Reviewed-by: Martin Robinson <[email protected]>
|
|
|
+Reviewed-by: Joanmarie Diggs <[email protected]>
|
|
|
+Cr-Commit-Position: refs/heads/master@{#757381}
|
|
|
+
|
|
|
+diff --git a/content/test/data/accessibility/event/css-flex-text-update-expected-auralinux.txt b/content/test/data/accessibility/event/css-flex-text-update-expected-auralinux.txt
|
|
|
+index c68cfe98572b9b2edf3658053be3dea2040524e4..483d5bb7eb7be40fa0dbd81cb336fe41fe665efe 100644
|
|
|
+--- a/content/test/data/accessibility/event/css-flex-text-update-expected-auralinux.txt
|
|
|
++++ b/content/test/data/accessibility/event/css-flex-text-update-expected-auralinux.txt
|
|
|
+@@ -1,3 +1,3 @@
|
|
|
+-CHILDREN-CHANGED index:0 CHILD:(role=ROLE_STATIC) role=ROLE_STATIC ENABLED,SENSITIVE,SHOWING,VISIBLE
|
|
|
++CHILDREN-CHANGED index:-1 CHILD:(role=ROLE_STATIC) role=ROLE_STATIC ENABLED,SENSITIVE,SHOWING,VISIBLE
|
|
|
+ NAME-CHANGED:new role=ROLE_STATIC name='new' ENABLED,SENSITIVE,SHOWING,VISIBLE
|
|
|
+ STATE-CHANGE:DEFUNCT:TRUE role=ROLE_INVALID name='(null)' DEFUNCT
|
|
|
+diff --git a/content/test/data/accessibility/event/text-changed-expected-auralinux.txt b/content/test/data/accessibility/event/text-changed-expected-auralinux.txt
|
|
|
+index b1573d5a3e84bc168403ad989632333473a5c0a8..0fe6700c90e82846aa862cc09a567d5ad36bccc6 100644
|
|
|
+--- a/content/test/data/accessibility/event/text-changed-expected-auralinux.txt
|
|
|
++++ b/content/test/data/accessibility/event/text-changed-expected-auralinux.txt
|
|
|
+@@ -1,7 +1,7 @@
|
|
|
++CHILDREN-CHANGED index:-1 CHILD:(role=ROLE_STATIC) role=ROLE_STATIC ENABLED,SENSITIVE,SHOWING,VISIBLE
|
|
|
++CHILDREN-CHANGED index:-1 CHILD:(role=ROLE_STATIC) role=ROLE_STATIC ENABLED,SENSITIVE,SHOWING,VISIBLE
|
|
|
+ CHILDREN-CHANGED index:0 CHILD:(role=ROLE_STATIC) role=ROLE_PARAGRAPH ENABLED,SENSITIVE,SHOWING,VISIBLE
|
|
|
+ CHILDREN-CHANGED index:0 CHILD:(role=ROLE_STATIC) role=ROLE_PARAGRAPH ENABLED,SENSITIVE,SHOWING,VISIBLE
|
|
|
+-CHILDREN-CHANGED index:0 CHILD:(role=ROLE_STATIC) role=ROLE_STATIC ENABLED,SENSITIVE,SHOWING,VISIBLE
|
|
|
+-CHILDREN-CHANGED index:0 CHILD:(role=ROLE_STATIC) role=ROLE_STATIC ENABLED,SENSITIVE,SHOWING,VISIBLE
|
|
|
+ NAME-CHANGED:Modified Div role=ROLE_STATIC name='Modified Div' ENABLED,SENSITIVE,SHOWING,VISIBLE
|
|
|
+ NAME-CHANGED:Modified Heading role=ROLE_STATIC name='Modified Heading' ENABLED,SENSITIVE,SHOWING,VISIBLE
|
|
|
+ STATE-CHANGE:DEFUNCT:TRUE role=ROLE_INVALID name='(null)' DEFUNCT
|
|
|
+diff --git a/ui/accessibility/platform/ax_platform_node_auralinux.cc b/ui/accessibility/platform/ax_platform_node_auralinux.cc
|
|
|
+index 2f150dbbae32f9faae0f8b55a65133fb333aa5e5..f3e9ab6b2bb8ab0c4a688fe84861fe9d7e766a5a 100644
|
|
|
+--- a/ui/accessibility/platform/ax_platform_node_auralinux.cc
|
|
|
++++ b/ui/accessibility/platform/ax_platform_node_auralinux.cc
|
|
|
+@@ -2077,7 +2077,7 @@ gint GetIndexInParent(AtkObject* atk_object) {
|
|
|
+ if (!obj)
|
|
|
+ return -1;
|
|
|
+
|
|
|
+- return obj->GetIndexInParent();
|
|
|
++ return obj->GetIndexInParent().value_or(-1);
|
|
|
+ }
|
|
|
+
|
|
|
+ gint AtkGetIndexInParent(AtkObject* atk_object) {
|
|
|
+@@ -3774,7 +3774,7 @@ void AXPlatformNodeAuraLinux::OnSubtreeCreated() {
|
|
|
+ return;
|
|
|
+
|
|
|
+ g_signal_emit_by_name(GetParent(), "children-changed::add",
|
|
|
+- GetIndexInParent(), atk_object);
|
|
|
++ GetIndexInParent().value_or(-1), atk_object);
|
|
|
+ }
|
|
|
+
|
|
|
+ void AXPlatformNodeAuraLinux::OnSubtreeWillBeDeleted() {
|
|
|
+@@ -3788,7 +3788,7 @@ void AXPlatformNodeAuraLinux::OnSubtreeWillBeDeleted() {
|
|
|
+ return;
|
|
|
+
|
|
|
+ g_signal_emit_by_name(GetParent(), "children-changed::remove",
|
|
|
+- GetIndexInParent(), atk_object);
|
|
|
++ GetIndexInParent().value_or(-1), atk_object);
|
|
|
+ }
|
|
|
+
|
|
|
+ void AXPlatformNodeAuraLinux::OnParentChanged() {
|
|
|
+diff --git a/ui/accessibility/platform/ax_platform_node_base.cc b/ui/accessibility/platform/ax_platform_node_base.cc
|
|
|
+index 12ec1674a79bdc1f28d5d2b1bb4792b81cd8608f..d946b07612f55efebf85615063411d262552944b 100644
|
|
|
+--- a/ui/accessibility/platform/ax_platform_node_base.cc
|
|
|
++++ b/ui/accessibility/platform/ax_platform_node_base.cc
|
|
|
+@@ -149,15 +149,16 @@ base::string16 AXPlatformNodeBase::GetNameAsString16() const {
|
|
|
+ return base::UTF8ToUTF16(name);
|
|
|
+ }
|
|
|
+
|
|
|
+-int AXPlatformNodeBase::GetIndexInParent() {
|
|
|
++base::Optional<int> AXPlatformNodeBase::GetIndexInParent() {
|
|
|
+ AXPlatformNodeBase* parent = FromNativeViewAccessible(GetParent());
|
|
|
+ if (!parent)
|
|
|
+- return 0;
|
|
|
++ return base::nullopt;
|
|
|
+
|
|
|
+ int child_count = parent->GetChildCount();
|
|
|
+ if (child_count == 0) {
|
|
|
+- // |child_count| could be 0 if the node is PlatformIsLeaf.
|
|
|
+- return 0;
|
|
|
++ // |child_count| could be 0 if the parent is IsLeaf.
|
|
|
++ DCHECK(parent->IsLeaf());
|
|
|
++ return base::nullopt;
|
|
|
+ }
|
|
|
+
|
|
|
+ // Ask the delegate for the index in parent, and return it if it's plausible.
|
|
|
+@@ -176,7 +177,9 @@ int AXPlatformNodeBase::GetIndexInParent() {
|
|
|
+ if (parent->ChildAtIndex(i) == current)
|
|
|
+ return i;
|
|
|
+ }
|
|
|
+- return -1;
|
|
|
++ NOTREACHED()
|
|
|
++ << "Unable to find the child in the list of its parent's children.";
|
|
|
++ return base::nullopt;
|
|
|
+ }
|
|
|
+
|
|
|
+ // AXPlatformNode overrides.
|
|
|
+@@ -1429,12 +1432,8 @@ int32_t AXPlatformNodeBase::GetHypertextOffsetFromChild(
|
|
|
+ // cross-tree traversal is necessary.
|
|
|
+ if (child->IsTextOnlyObject()) {
|
|
|
+ int32_t hypertext_offset = 0;
|
|
|
+- int32_t index_in_parent = child->GetIndexInParent();
|
|
|
+- DCHECK_GE(index_in_parent, 0);
|
|
|
+- DCHECK_LT(index_in_parent, static_cast<int32_t>(GetChildCount()));
|
|
|
+- for (uint32_t i = 0; i < static_cast<uint32_t>(index_in_parent); ++i) {
|
|
|
+- auto* sibling = static_cast<AXPlatformNodeBase*>(
|
|
|
+- FromNativeViewAccessible(ChildAtIndex(i)));
|
|
|
++ for (AXPlatformNodeBase* sibling = GetFirstChild(); sibling != child;
|
|
|
++ sibling = sibling->GetNextSibling()) {
|
|
|
+ DCHECK(sibling);
|
|
|
+ if (sibling->IsTextOnlyObject()) {
|
|
|
+ hypertext_offset += (int32_t)sibling->GetHypertext().size();
|
|
|
+@@ -1510,7 +1509,7 @@ int AXPlatformNodeBase::GetHypertextOffsetFromEndpoint(
|
|
|
+ }
|
|
|
+
|
|
|
+ AXPlatformNodeBase* common_parent = this;
|
|
|
+- int32_t index_in_common_parent = GetIndexInParent();
|
|
|
++ base::Optional<int> index_in_common_parent = GetIndexInParent();
|
|
|
+ while (common_parent && !endpoint_object->IsDescendantOf(common_parent)) {
|
|
|
+ index_in_common_parent = common_parent->GetIndexInParent();
|
|
|
+ common_parent = static_cast<AXPlatformNodeBase*>(
|
|
|
+@@ -1519,7 +1518,6 @@ int AXPlatformNodeBase::GetHypertextOffsetFromEndpoint(
|
|
|
+ if (!common_parent)
|
|
|
+ return -1;
|
|
|
+
|
|
|
+- DCHECK_GE(index_in_common_parent, 0);
|
|
|
+ DCHECK(!(common_parent->IsTextOnlyObject()));
|
|
|
+
|
|
|
+ // Case 2. Is the selection endpoint inside a descendant of this object?
|
|
|
+@@ -1547,17 +1545,14 @@ int AXPlatformNodeBase::GetHypertextOffsetFromEndpoint(
|
|
|
+ //
|
|
|
+ // We can safely assume that the endpoint is in another part of the tree or
|
|
|
+ // at common parent, and that this object is a descendant of common parent.
|
|
|
+- int32_t endpoint_index_in_common_parent = -1;
|
|
|
+- for (int i = 0; i < common_parent->GetDelegate()->GetChildCount(); ++i) {
|
|
|
+- auto* child = static_cast<AXPlatformNodeBase*>(FromNativeViewAccessible(
|
|
|
+- common_parent->GetDelegate()->ChildAtIndex(i)));
|
|
|
+- DCHECK(child);
|
|
|
++ base::Optional<int> endpoint_index_in_common_parent;
|
|
|
++ for (AXPlatformNodeBase* child = common_parent->GetFirstChild();
|
|
|
++ child != nullptr; child = child->GetNextSibling()) {
|
|
|
+ if (endpoint_object->IsDescendantOf(child)) {
|
|
|
+ endpoint_index_in_common_parent = child->GetIndexInParent();
|
|
|
+ break;
|
|
|
+ }
|
|
|
+ }
|
|
|
+- DCHECK_GE(endpoint_index_in_common_parent, 0);
|
|
|
+
|
|
|
+ if (endpoint_index_in_common_parent < index_in_common_parent)
|
|
|
+ return 0;
|
|
|
+diff --git a/ui/accessibility/platform/ax_platform_node_base.h b/ui/accessibility/platform/ax_platform_node_base.h
|
|
|
+index 0355f281a600979f59a25086fd7400201693bf89..9cb3364d43cb93f5e612a3a076161b4f2e11adaf 100644
|
|
|
+--- a/ui/accessibility/platform/ax_platform_node_base.h
|
|
|
++++ b/ui/accessibility/platform/ax_platform_node_base.h
|
|
|
+@@ -69,8 +69,9 @@ class AX_EXPORT AXPlatformNodeBase : public AXPlatformNode {
|
|
|
+ std::string GetName() const;
|
|
|
+ base::string16 GetNameAsString16() const;
|
|
|
+
|
|
|
+- // This returns 0 if there's no parent.
|
|
|
+- virtual int GetIndexInParent();
|
|
|
++ // This returns nullopt if there's no parent, it's unable to find the child in
|
|
|
++ // the list of its parent's children, or its parent doesn't have children.
|
|
|
++ virtual base::Optional<int> GetIndexInParent();
|
|
|
+
|
|
|
+ // AXPlatformNode.
|
|
|
+ void Destroy() override;
|
|
|
+diff --git a/ui/accessibility/platform/ax_platform_node_mac.h b/ui/accessibility/platform/ax_platform_node_mac.h
|
|
|
+index a5624cb789a7b686c1b139d1845127114befd7e2..920f0a0536364d26b99dbe54cf372abaf7d34439 100644
|
|
|
+--- a/ui/accessibility/platform/ax_platform_node_mac.h
|
|
|
++++ b/ui/accessibility/platform/ax_platform_node_mac.h
|
|
|
+@@ -27,7 +27,6 @@ class AXPlatformNodeMac : public AXPlatformNodeBase {
|
|
|
+
|
|
|
+ // AXPlatformNodeBase.
|
|
|
+ void Destroy() override;
|
|
|
+- int GetIndexInParent() override;
|
|
|
+
|
|
|
+ protected:
|
|
|
+ void AddAttributeToList(const char* name,
|
|
|
+diff --git a/ui/accessibility/platform/ax_platform_node_mac.mm b/ui/accessibility/platform/ax_platform_node_mac.mm
|
|
|
+index 0454164364c6a4a1b1c11011603af5978d2b8ef5..bd9e17f1e21bd17a68988ee3c9ab08f2a1d99b2e 100644
|
|
|
+--- a/ui/accessibility/platform/ax_platform_node_mac.mm
|
|
|
++++ b/ui/accessibility/platform/ax_platform_node_mac.mm
|
|
|
+@@ -1257,11 +1257,6 @@ void AXPlatformNodeMac::AnnounceText(const base::string16& text) {
|
|
|
+ [native_node_ AXWindow], false);
|
|
|
+ }
|
|
|
+
|
|
|
+-int AXPlatformNodeMac::GetIndexInParent() {
|
|
|
+- // TODO(dmazzoni): implement this. http://crbug.com/396137
|
|
|
+- return -1;
|
|
|
+-}
|
|
|
+-
|
|
|
+ bool IsNameExposedInAXValueForRole(ax::mojom::Role role) {
|
|
|
+ switch (role) {
|
|
|
+ case ax::mojom::Role::kListBoxOption:
|
|
|
+diff --git a/ui/accessibility/platform/ax_platform_node_win.cc b/ui/accessibility/platform/ax_platform_node_win.cc
|
|
|
+index a61d4121d9eeb336e72dbb6a3e2bf884a9b4e799..222c9358ca8e9958d6e89faf8779e706862e1f8c 100644
|
|
|
+--- a/ui/accessibility/platform/ax_platform_node_win.cc
|
|
|
++++ b/ui/accessibility/platform/ax_platform_node_win.cc
|
|
|
+@@ -1370,10 +1370,11 @@ IFACEMETHODIMP AXPlatformNodeWin::get_attributes(BSTR* attributes) {
|
|
|
+ IFACEMETHODIMP AXPlatformNodeWin::get_indexInParent(LONG* index_in_parent) {
|
|
|
+ WIN_ACCESSIBILITY_API_HISTOGRAM(UMA_API_GET_INDEX_IN_PARENT);
|
|
|
+ COM_OBJECT_VALIDATE_1_ARG(index_in_parent);
|
|
|
+- *index_in_parent = GetIndexInParent();
|
|
|
+- if (*index_in_parent < 0)
|
|
|
++ base::Optional<int> index = GetIndexInParent();
|
|
|
++ if (!index.has_value())
|
|
|
+ return E_FAIL;
|
|
|
+
|
|
|
++ *index_in_parent = index.value();
|
|
|
+ return S_OK;
|
|
|
+ }
|
|
|
+
|
|
|
+diff --git a/ui/accessibility/platform/ax_platform_node_win_unittest.cc b/ui/accessibility/platform/ax_platform_node_win_unittest.cc
|
|
|
+index 2092d5c263dafa452c4977d53678ad81aa6575e1..8e2cf3d13fd4f936dd7c1e0a21bd245b589857d0 100644
|
|
|
+--- a/ui/accessibility/platform/ax_platform_node_win_unittest.cc
|
|
|
++++ b/ui/accessibility/platform/ax_platform_node_win_unittest.cc
|
|
|
+@@ -1149,8 +1149,7 @@ TEST_F(AXPlatformNodeWinTest, TestIAccessible2IndexInParent) {
|
|
|
+ ComPtr<IAccessible2> right_iaccessible2 = ToIAccessible2(right_iaccessible);
|
|
|
+
|
|
|
+ LONG index;
|
|
|
+- EXPECT_EQ(S_OK, root_iaccessible2->get_indexInParent(&index));
|
|
|
+- EXPECT_EQ(0, index);
|
|
|
++ EXPECT_EQ(E_FAIL, root_iaccessible2->get_indexInParent(&index));
|
|
|
+
|
|
|
+ EXPECT_EQ(S_OK, left_iaccessible2->get_indexInParent(&index));
|
|
|
+ EXPECT_EQ(0, index);
|