0

[A11y] Don't serialize dom node id

The same information can now be retrieved from the AXNodeID.
If it's a positive number, then it's the DOM node id.

Bug: none
Change-Id: I143c58b6b63b265c2fcb05ce958f09467a9e2427
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5791703
Commit-Queue: Joe Mason <joenotcharles@google.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Benjamin Beaudry <benjamin.beaudry@microsoft.com>
Auto-Submit: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Max Curran <curranmax@chromium.org>
Reviewed-by: Joe Mason <joenotcharles@google.com>
Cr-Commit-Position: refs/heads/main@{#1344150}
This commit is contained in:
Aaron Leventhal
2024-08-20 15:06:57 +00:00
committed by Chromium LUCI CQ
parent 8def5e6e08
commit e4680e692d
14 changed files with 74 additions and 59 deletions

@@ -699,8 +699,9 @@ optimization_guide::proto::AXIntAttribute IntAttributeToProto(
case ax::mojom::IntAttribute::kDropeffectDeprecated: case ax::mojom::IntAttribute::kDropeffectDeprecated:
return optimization_guide::proto::AXIntAttribute:: return optimization_guide::proto::AXIntAttribute::
AX_IA_DROPEFFECTDEPRECATED; AX_IA_DROPEFFECTDEPRECATED;
case ax::mojom::IntAttribute::kDOMNodeId: case ax::mojom::IntAttribute::kDOMNodeIdDeprecated:
return optimization_guide::proto::AXIntAttribute::AX_IA_DOMNODEID; return optimization_guide::proto::AXIntAttribute::
AX_IA_DOMNODEIDDEPRECATED;
case ax::mojom::IntAttribute::kIsPopup: case ax::mojom::IntAttribute::kIsPopup:
return optimization_guide::proto::AXIntAttribute::AX_IA_ISPOPUP; return optimization_guide::proto::AXIntAttribute::AX_IA_ISPOPUP;
case ax::mojom::IntAttribute::kNextWindowFocusId: case ax::mojom::IntAttribute::kNextWindowFocusId:

@@ -886,7 +886,7 @@ enum AXIntAttribute {
AX_IA_DROPEFFECTDEPRECATED = 59; AX_IA_DROPEFFECTDEPRECATED = 59;
AX_IA_DOMNODEID = 60; AX_IA_DOMNODEIDDEPRECATED = 60;
AX_IA_ISPOPUP = 61; AX_IA_ISPOPUP = 61;

@@ -150,7 +150,7 @@ std::string IntAttrToString(const ui::AXNode& node,
case ax::mojom::IntAttribute::kAriaCellRowSpan: case ax::mojom::IntAttribute::kAriaCellRowSpan:
case ax::mojom::IntAttribute::kAriaRowCount: case ax::mojom::IntAttribute::kAriaRowCount:
case ax::mojom::IntAttribute::kColorValue: case ax::mojom::IntAttribute::kColorValue:
case ax::mojom::IntAttribute::kDOMNodeId: case ax::mojom::IntAttribute::kDOMNodeIdDeprecated:
case ax::mojom::IntAttribute::kDropeffectDeprecated: case ax::mojom::IntAttribute::kDropeffectDeprecated:
case ax::mojom::IntAttribute::kErrormessageIdDeprecated: case ax::mojom::IntAttribute::kErrormessageIdDeprecated:
case ax::mojom::IntAttribute::kHierarchicalLevel: case ax::mojom::IntAttribute::kHierarchicalLevel:

@@ -588,8 +588,9 @@ IN_PROC_BROWSER_TEST_F(SnapshotAXTreeBrowserTest, SnapshotPDFMode) {
EXPECT_NE(ax::mojom::Role::kUnknown, node_data.role); EXPECT_NE(ax::mojom::Role::kUnknown, node_data.role);
EXPECT_NE(0, node_data.id); EXPECT_NE(0, node_data.id);
if (node_data.GetIntAttribute(ax::mojom::IntAttribute::kDOMNodeId) != 0) if (node_data.GetDOMNodeId()) {
dom_node_id_count++; dom_node_id_count++;
}
// We don't need bounding boxes to make a tagged PDF. Ensure those are // We don't need bounding boxes to make a tagged PDF. Ensure those are
// uninitialized. // uninitialized.

@@ -116,7 +116,7 @@ bool RecursiveBuildStructureTree(const ui::AXNode* ax_node,
SkPDF::StructureElementNode* tag) { SkPDF::StructureElementNode* tag) {
bool valid = false; bool valid = false;
tag->fNodeId = ax_node->GetIntAttribute(ax::mojom::IntAttribute::kDOMNodeId); tag->fNodeId = ax_node->data().GetDOMNodeId();
switch (ax_node->GetRole()) { switch (ax_node->GetRole()) {
case ax::mojom::Role::kRootWebArea: case ax::mojom::Role::kRootWebArea:
tag->fTypeString = kPDFStructureTypeDocument; tag->fTypeString = kPDFStructureTypeDocument;
@@ -172,8 +172,7 @@ bool RecursiveBuildStructureTree(const ui::AXNode* ax_node,
std::vector<int> header_ids; std::vector<int> header_ids;
header_ids.reserve(header_nodes.size()); header_ids.reserve(header_nodes.size());
for (ui::AXNode* header_node : header_nodes) { for (ui::AXNode* header_node : header_nodes) {
header_ids.push_back(header_node->GetIntAttribute( header_ids.push_back(header_node->data().GetDOMNodeId());
ax::mojom::IntAttribute::kDOMNodeId));
} }
tag->fAttributes.appendNodeIdArray( tag->fAttributes.appendNodeIdArray(
kPDFTableAttributeOwner, kPDFTableCellHeadersAttribute, header_ids); kPDFTableAttributeOwner, kPDFTableCellHeadersAttribute, header_ids);

@@ -2262,21 +2262,6 @@ void AXObject::SerializeUnignoredAttributes(ui::AXNodeData* node_data,
ax::mojom::blink::BoolAttribute::kNotUserSelectableStyle, true); ax::mojom::blink::BoolAttribute::kNotUserSelectableStyle, true);
} }
if (accessibility_mode.has_mode(ui::AXMode::kScreenReader) ||
accessibility_mode.has_mode(ui::AXMode::kPDFPrinting)) {
// The DOMNodeID from Blink. Currently only populated when using
// the accessibility tree for PDF exporting. Warning, this is totally
// unrelated to the ID attribute for an HTML element - it's an ID used to
// uniquely identify nodes in Blink.
// TODO(accessibility) Remove this and use the AXObjectID(), which is the
// the same when there there is a DOM node (will always be positive).
int dom_node_id = GetDOMNodeId();
if (dom_node_id) {
node_data->AddIntAttribute(ax::mojom::blink::IntAttribute::kDOMNodeId,
dom_node_id);
}
}
// If text, return early as a performance tweak, as the rest of the properties // If text, return early as a performance tweak, as the rest of the properties
// in this method do not apply to text. // in this method do not apply to text.
if (RoleValue() == ax::mojom::blink::Role::kStaticText) { if (RoleValue() == ax::mojom::blink::Role::kStaticText) {
@@ -6838,13 +6823,6 @@ AXObject::AXObjectVector AXObject::TableCellChildren() const {
return result; return result;
} }
int AXObject::GetDOMNodeId() const {
Node* node = GetNode();
if (node)
return node->GetDomNodeId();
return 0;
}
void AXObject::GetRelativeBounds(AXObject** out_container, void AXObject::GetRelativeBounds(AXObject** out_container,
gfx::RectF& out_bounds_in_container, gfx::RectF& out_bounds_in_container,
gfx::Transform& out_container_transform, gfx::Transform& out_container_transform,

@@ -1471,9 +1471,6 @@ class MODULES_EXPORT AXObject : public GarbageCollected<AXObject> {
int* index_in_ancestor1, int* index_in_ancestor1,
int* index_in_ancestor2); int* index_in_ancestor2);
// Blink-internal DOM Node ID. Currently used for PDF exporting.
int GetDOMNodeId() const;
bool IsHiddenForTextAlternativeCalculation( bool IsHiddenForTextAlternativeCalculation(
const AXObject* aria_label_or_description_root) const; const AXObject* aria_label_or_description_root) const;

@@ -3019,7 +3019,7 @@ int AXObjectCacheImpl::GetLocationSerializationDelay() {
// currently focused object, so schedule serializations (almost )immediately // currently focused object, so schedule serializations (almost )immediately
// if that object changes. The root is an exception because it often has focus // if that object changes. The root is an exception because it often has focus
// while the page is loading. // while the page is loading.
DOMNodeId focused_node_id = FocusedObject()->GetDOMNodeId(); DOMNodeId focused_node_id = FocusedNode()->GetDomNodeId();
if (focused_node_id != document_->GetDomNodeId() && if (focused_node_id != document_->GetDomNodeId() &&
changed_bounds_ids_.Contains(focused_node_id)) { changed_bounds_ids_.Contains(focused_node_id)) {
return kDelayForLocationUpdatesFocused; return kDelayForLocationUpdatesFocused;

@@ -1561,7 +1561,7 @@ const char* ToString(ax::mojom::IntAttribute int_attribute) {
return "nextFocusId"; return "nextFocusId";
case ax::mojom::IntAttribute::kImageAnnotationStatus: case ax::mojom::IntAttribute::kImageAnnotationStatus:
return "imageAnnotationStatus"; return "imageAnnotationStatus";
case ax::mojom::IntAttribute::kDOMNodeId: case ax::mojom::IntAttribute::kDOMNodeIdDeprecated:
return "domNodeId"; return "domNodeId";
case ax::mojom::IntAttribute::kNextWindowFocusId: case ax::mojom::IntAttribute::kNextWindowFocusId:
return "nextWindowFocusId"; return "nextWindowFocusId";
@@ -1700,7 +1700,7 @@ ax::mojom::IntAttribute StringToIntAttribute(const std::string& int_attribute) {
} else if (int_attribute == "kImageAnnotationStatus") { } else if (int_attribute == "kImageAnnotationStatus") {
return ax::mojom::IntAttribute::kImageAnnotationStatus; return ax::mojom::IntAttribute::kImageAnnotationStatus;
} else if (int_attribute == "kDomNodeId") { } else if (int_attribute == "kDomNodeId") {
return ax::mojom::IntAttribute::kDOMNodeId; return ax::mojom::IntAttribute::kDOMNodeIdDeprecated;
} else if (int_attribute == "kNextWindowFocusId") { } else if (int_attribute == "kNextWindowFocusId") {
return ax::mojom::IntAttribute::kNextWindowFocusId; return ax::mojom::IntAttribute::kNextWindowFocusId;
} else if (int_attribute == "kPreviousWindowFocusId") { } else if (int_attribute == "kPreviousWindowFocusId") {

@@ -772,11 +772,8 @@ enum IntAttribute {
// Note: aria-dropeffect is deprecated in WAI-ARIA 1.1. // Note: aria-dropeffect is deprecated in WAI-ARIA 1.1.
kDropeffectDeprecated = 59, kDropeffectDeprecated = 59,
// The DOMNodeID from Blink. Currently only populated when using // Deprecated, use AXNodeData.id (see ui/accessibility/ax_node_id_forward.h).
// the accessibility tree for PDF exporting. Warning, this is totally kDOMNodeIdDeprecated = 60,
// unrelated to the accessibility node ID, or the ID attribute for an
// HTML element - it's an ID used to uniquely identify nodes in Blink.
kDOMNodeId = 60,
// Indicates whether the element is a popover ("popup") and if so, what type. // Indicates whether the element is a popover ("popup") and if so, what type.
[MinVersion=1] kIsPopup = 61, [MinVersion=1] kIsPopup = 61,

@@ -174,7 +174,7 @@ bool IsNodeIdIntAttribute(ax::mojom::IntAttribute attr) {
case ax::mojom::IntAttribute::kAriaCellRowSpan: case ax::mojom::IntAttribute::kAriaCellRowSpan:
case ax::mojom::IntAttribute::kImageAnnotationStatus: case ax::mojom::IntAttribute::kImageAnnotationStatus:
case ax::mojom::IntAttribute::kDropeffectDeprecated: case ax::mojom::IntAttribute::kDropeffectDeprecated:
case ax::mojom::IntAttribute::kDOMNodeId: case ax::mojom::IntAttribute::kDOMNodeIdDeprecated:
case ax::mojom::IntAttribute::kAriaNotificationInterruptDeprecated: case ax::mojom::IntAttribute::kAriaNotificationInterruptDeprecated:
case ax::mojom::IntAttribute::kAriaNotificationPriorityDeprecated: case ax::mojom::IntAttribute::kAriaNotificationPriorityDeprecated:
return false; return false;
@@ -642,6 +642,10 @@ AXTextAttributes AXNodeData::GetTextAttributes() const {
return text_attributes; return text_attributes;
} }
int AXNodeData::GetDOMNodeId() const {
return id > 0 ? id : 0;
}
void AXNodeData::SetName(const std::string& name) { void AXNodeData::SetName(const std::string& name) {
// Elements with role='presentation' have Role::kNone. They should not be // Elements with role='presentation' have Role::kNone. They should not be
// named. Objects with Role::kUnknown were never given a role. This check // named. Objects with Role::kUnknown were never given a role. This check
@@ -1670,8 +1674,7 @@ std::string AXNodeData::ToString(bool verbose) const {
case ax::mojom::IntAttribute::kDropeffectDeprecated: case ax::mojom::IntAttribute::kDropeffectDeprecated:
result += " dropeffect=" + value; result += " dropeffect=" + value;
break; break;
case ax::mojom::IntAttribute::kDOMNodeId: case ax::mojom::IntAttribute::kDOMNodeIdDeprecated:
result += " dom_node_id=" + value;
break; break;
case ax::mojom::IntAttribute::kAriaNotificationInterruptDeprecated: case ax::mojom::IntAttribute::kAriaNotificationInterruptDeprecated:
result += result +=

@@ -149,6 +149,10 @@ struct AX_BASE_EXPORT AXNodeData {
// Convenience functions. // Convenience functions.
// //
// Return the DOMNodeID, if this object was associated with a DOM Node in
// an HTML renderer, otherwise return 0.
int GetDOMNodeId() const;
// Adds the name attribute or replaces it if already present. Also sets the // Adds the name attribute or replaces it if already present. Also sets the
// NameFrom attribute if not already set. // NameFrom attribute if not already set.
// //

@@ -5067,25 +5067,27 @@ IFACEMETHODIMP AXPlatformNodeWin::Navigate(
void AXPlatformNodeWin::GetRuntimeIdArray( void AXPlatformNodeWin::GetRuntimeIdArray(
AXPlatformNodeWin::RuntimeIdArray& runtime_id) { AXPlatformNodeWin::RuntimeIdArray& runtime_id) {
int dom_id;
runtime_id[0] = UiaAppendRuntimeId; runtime_id[0] = UiaAppendRuntimeId;
// The combination of tree/frame id and Blink (DOM) id is unique and gives // The combination of tree/frame id and Blink (DOM) id is unique and gives
// nodes stable ids across layouts/tree movement. If there's a valid tree // nodes stable ids across layouts/tree movement. If there's a valid tree
// id, use that, otherwise fall back to the globally unique id. // id, use that, otherwise fall back to the globally unique id.
AXTreeID tree_id = GetDelegate()->GetTreeData().tree_id; int dom_id = GetData().GetDOMNodeId();
if (GetIntAttribute(ax::mojom::IntAttribute::kDOMNodeId, &dom_id) && if (dom_id) {
tree_id != AXTreeIDUnknown()) { AXTreeID tree_id = GetDelegate()->GetTreeData().tree_id;
AXActionHandlerRegistry::FrameID frame_id = if (tree_id != AXTreeIDUnknown()) {
AXActionHandlerRegistry::GetInstance()->GetFrameID(tree_id); AXActionHandlerRegistry::FrameID frame_id =
runtime_id[1] = frame_id.first; AXActionHandlerRegistry::GetInstance()->GetFrameID(tree_id);
runtime_id[2] = frame_id.second; runtime_id[1] = frame_id.first;
runtime_id[3] = dom_id; runtime_id[2] = frame_id.second;
} else { runtime_id[3] = dom_id;
runtime_id[1] = 0; return;
runtime_id[2] = 0; }
runtime_id[3] = GetUniqueId();
} }
runtime_id[1] = 0;
runtime_id[2] = 0;
runtime_id[3] = GetUniqueId();
} }
IFACEMETHODIMP AXPlatformNodeWin::GetRuntimeId(SAFEARRAY** runtime_id) { IFACEMETHODIMP AXPlatformNodeWin::GetRuntimeId(SAFEARRAY** runtime_id) {

@@ -5130,9 +5130,9 @@ TEST_F(AXPlatformNodeWinTest, UIAGetEmbeddedFragmentRoots) {
EXPECT_EQ(nullptr, embedded_fragment_roots.Get()); EXPECT_EQ(nullptr, embedded_fragment_roots.Get());
} }
TEST_F(AXPlatformNodeWinTest, UIAGetRuntimeId) { TEST_F(AXPlatformNodeWinTest, UIAGetRuntimeIdForGeneratedId) {
AXNodeData root_data; AXNodeData root_data;
root_data.id = 1; root_data.id = -99;
root_data.role = ax::mojom::Role::kRootWebArea; root_data.role = ax::mojom::Role::kRootWebArea;
Init(root_data); Init(root_data);
@@ -5163,6 +5163,39 @@ TEST_F(AXPlatformNodeWinTest, UIAGetRuntimeId) {
EXPECT_HRESULT_SUCCEEDED(::SafeArrayUnaccessData(runtime_id.Get())); EXPECT_HRESULT_SUCCEEDED(::SafeArrayUnaccessData(runtime_id.Get()));
} }
TEST_F(AXPlatformNodeWinTest, UIAGetRuntimeIdForSuppliedId) {
AXNodeData root_data;
root_data.id = 1;
root_data.role = ax::mojom::Role::kRootWebArea;
Init(root_data);
ComPtr<IRawElementProviderFragment> root_provider =
GetRootIRawElementProviderFragment();
base::win::ScopedSafearray runtime_id;
EXPECT_HRESULT_SUCCEEDED(root_provider->GetRuntimeId(runtime_id.Receive()));
LONG array_lower_bound;
EXPECT_HRESULT_SUCCEEDED(
::SafeArrayGetLBound(runtime_id.Get(), 1, &array_lower_bound));
EXPECT_EQ(0, array_lower_bound);
LONG array_upper_bound;
EXPECT_HRESULT_SUCCEEDED(
::SafeArrayGetUBound(runtime_id.Get(), 1, &array_upper_bound));
EXPECT_EQ(3, array_upper_bound);
int* array_data;
EXPECT_HRESULT_SUCCEEDED(::SafeArrayAccessData(
runtime_id.Get(), reinterpret_cast<void**>(&array_data)));
EXPECT_EQ(UiaAppendRuntimeId, array_data[0]);
EXPECT_EQ(-1, array_data[1]);
EXPECT_EQ(-1, array_data[2]);
EXPECT_EQ(1, array_data[3]);
EXPECT_HRESULT_SUCCEEDED(::SafeArrayUnaccessData(runtime_id.Get()));
}
TEST_F(AXPlatformNodeWinTest, UIAIWindowProviderGetIsModalUnset) { TEST_F(AXPlatformNodeWinTest, UIAIWindowProviderGetIsModalUnset) {
AXNodeData root; AXNodeData root;
root.id = 1; root.id = 1;