Skip to content

Commit

Permalink
[A11y] Speculative fix for placeholder elements trying to add children
Browse files Browse the repository at this point in the history
The crash reports are all logging placeholder elements as trying to add
children but having a return value of false for CanHaveChildren(). A
possible explanation is that CanHaveChildren() returned true earlier
because the placeholder element.ShadowPseudoId() value was not set
for the initial call.

This hopefully fixes the issue by using a more robust API to check for
the placeholder element. Even if it does not fix the crash, it seems
cleaner and less brittle.

(cherry picked from commit 926950c)

Bug: 1407397
Change-Id: I59a10338525076e756e5a60cd475bdb26e75af9c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4590613
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Mason Freed <masonf@chromium.org>
Auto-Submit: Aaron Leventhal <aleventhal@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1154087}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4604457
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5790@{#600}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
aleventhal authored and Chromium LUCI CQ committed Jun 11, 2023
1 parent fa87a1a commit 347c055
Showing 1 changed file with 7 additions and 3 deletions.
10 changes: 7 additions & 3 deletions third_party/blink/renderer/modules/accessibility/ax_object.cc
Expand Up @@ -1004,9 +1004,13 @@ bool AXObject::CanHaveChildren(Element& element) {
// so there's no need to add its text children. Placeholder text is a separate
// node that gets removed when it disappears, so this will only be present if
// the placeholder is visible.
if (element.ShadowPseudoId() ==
shadow_element_names::kPseudoInputPlaceholder) {
return false;
if (Element* host = element.OwnerShadowHost()) {
if (auto* ancestor_input = DynamicTo<TextControlElement>(host)) {
if (ancestor_input->PlaceholderElement() == &element) {
// |element| is a placeholder.
return false;
}
}
}

if (IsA<HTMLBRElement>(element)) {
Expand Down

0 comments on commit 347c055

Please sign in to comment.