Prevent check failures in AXSelection resulting from DOM mutations
When AXSelection::Select is called to set the caret in an unfocused
input, FrameSelection::DidSetSelectionDeprecated focuses the node by
default. If this focus event happens to trigger a DOM mutation, the DOM
tree version associated with the selection will not match the version
associated with the document. As a result, when AXSelection::Select
goes to cache the selection range, the DCHECK for this condition in
SelectionTemplate<Strategy>::AssertValid fails.
We can stop this condition from happening by using the same logic in
FrameSelection::SetSelection, namely call SetSelectionDeprecated and
if true, then call DidSetSelectionDeprecated -- but only after we have
cached the selection range.
If instead the DOM mutation is a consequence of the selection itself,
there will be a similar check failures, both for the DOM tree version
and the fact that layout is needed. Therefore, if a layout is pending
immediately after we dispatch the selection start, update the layout,
the AXPositions, and our stored DOM tree and Style versions.
AX-Relnotes: N/A
Change-Id: Ifbd63744d77db10b8895905a130b0b901cad3e0c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2346329
Commit-Queue: Joanmarie Diggs <jdiggs@igalia.com>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803824}
diff --git a/content/browser/accessibility/accessibility_auralinux_browsertest.cc b/content/browser/accessibility/accessibility_auralinux_browsertest.cc
index 657d4b4..3c9e3f4 100644
--- a/content/browser/accessibility/accessibility_auralinux_browsertest.cc
+++ b/content/browser/accessibility/accessibility_auralinux_browsertest.cc
@@ -43,6 +43,13 @@
return result;
}
+static bool IsAtkObjectEditable(AtkObject* object) {
+ AtkStateSet* state_set = atk_object_ref_state_set(object);
+ bool result = atk_state_set_contains_state(state_set, ATK_STATE_EDITABLE);
+ g_object_unref(state_set);
+ return result;
+}
+
} // namespace
class AccessibilityAuraLinuxBrowserTest : public AccessibilityBrowserTest {
@@ -1565,6 +1572,170 @@
}
IN_PROC_BROWSER_TEST_F(AccessibilityAuraLinuxBrowserTest,
+ SelectionTriggersReparentingOnSelectionStart) {
+ LoadInitialAccessibilityTreeFromHtml(
+ R"HTML(<!DOCTYPE html>
+ <html>
+ <head>
+ <script>
+ document.onselectstart = () => {
+ var tomove = document.getElementById("tomove");
+ document.getElementById("div").appendChild(tomove);
+ }
+ </script>
+ </head>
+ <body>
+ <div id="tomove">Move me</div>
+ <p id="paragraph">hello world</p>
+ <div id="div"></div>
+ </body>
+ </html>)HTML");
+
+ AtkObject* document = GetRendererAccessible();
+ AtkObject* paragraph = atk_object_ref_accessible_child(document, 1);
+ ASSERT_EQ(atk_object_get_role(paragraph), ATK_ROLE_PARAGRAPH);
+
+ AccessibilityNotificationWaiter waiter(
+ shell()->web_contents(), ui::kAXModeComplete,
+ ax::mojom::Event::kTextSelectionChanged);
+
+ EXPECT_TRUE(atk_text_set_selection(ATK_TEXT(paragraph), 0, 0, 5));
+ waiter.WaitForNotification();
+
+ gchar* selected =
+ atk_text_get_selection(ATK_TEXT(paragraph), 0, nullptr, nullptr);
+ EXPECT_STREQ(selected, "hello");
+ g_free(selected);
+
+ g_object_unref(paragraph);
+}
+
+IN_PROC_BROWSER_TEST_F(AccessibilityAuraLinuxBrowserTest,
+ SelectionTriggersAnchorDeletionOnSelectionStart) {
+ LoadInitialAccessibilityTreeFromHtml(
+ R"HTML(<!DOCTYPE html>
+ <html>
+ <head>
+ <script>
+ document.onselectstart = () => {
+ document.getElementById("p").removeChild(p.childNodes[0]);
+ }
+ </script>
+ </head>
+ <body>
+ <p id="p"><span>hello</span> <span>world</span></p>
+ <button id="button">ok</button>
+ </body>
+ </html>)HTML");
+
+ AtkObject* document = GetRendererAccessible();
+ AtkObject* paragraph = atk_object_ref_accessible_child(document, 0);
+ ASSERT_EQ(atk_object_get_role(paragraph), ATK_ROLE_PARAGRAPH);
+
+ AtkObject* button = atk_object_ref_accessible_child(document, 1);
+ ASSERT_EQ(atk_object_get_role(button), ATK_ROLE_PUSH_BUTTON);
+
+ AccessibilityNotificationWaiter waiter(
+ shell()->web_contents(), ui::kAXModeComplete, ax::mojom::Event::kFocus);
+
+ EXPECT_TRUE(atk_text_set_selection(ATK_TEXT(paragraph), 0, 0, 11));
+ atk_component_grab_focus(ATK_COMPONENT(button));
+ waiter.WaitForNotification();
+
+ gchar* selected =
+ atk_text_get_selection(ATK_TEXT(paragraph), 0, nullptr, nullptr);
+ EXPECT_STREQ(selected, nullptr);
+ g_free(selected);
+
+ g_object_unref(paragraph);
+ g_object_unref(button);
+}
+
+IN_PROC_BROWSER_TEST_F(AccessibilityAuraLinuxBrowserTest,
+ SelectionTriggersFocusDeletionOnSelectionStart) {
+ LoadInitialAccessibilityTreeFromHtml(
+ R"HTML(<!DOCTYPE html>
+ <head>
+ <script>
+ document.onselectstart = () => {
+ document.getElementById("p").removeChild(p.childNodes[2]);
+ }
+ </script>
+ </head>
+ <body>
+ <p id="p"><span>hello</span> <span>world</span></p>
+ <button id="button">ok</button>
+ </body>
+ </html>)HTML");
+
+ AtkObject* document = GetRendererAccessible();
+ AtkObject* paragraph = atk_object_ref_accessible_child(document, 0);
+ ASSERT_EQ(atk_object_get_role(paragraph), ATK_ROLE_PARAGRAPH);
+
+ AtkObject* button = atk_object_ref_accessible_child(document, 1);
+ ASSERT_EQ(atk_object_get_role(button), ATK_ROLE_PUSH_BUTTON);
+
+ AccessibilityNotificationWaiter waiter(
+ shell()->web_contents(), ui::kAXModeComplete, ax::mojom::Event::kFocus);
+
+ EXPECT_TRUE(atk_text_set_selection(ATK_TEXT(paragraph), 0, 0, 11));
+ atk_component_grab_focus(ATK_COMPONENT(button));
+ waiter.WaitForNotification();
+
+ gchar* selected =
+ atk_text_get_selection(ATK_TEXT(paragraph), 0, nullptr, nullptr);
+ EXPECT_STREQ(selected, nullptr);
+ g_free(selected);
+
+ g_object_unref(paragraph);
+ g_object_unref(button);
+}
+
+IN_PROC_BROWSER_TEST_F(AccessibilityAuraLinuxBrowserTest,
+ SelectionTriggersReparentingOnFocus) {
+ LoadInitialAccessibilityTreeFromHtml(
+ R"HTML(<!DOCTYPE html>
+ <html>
+ <head>
+ <script>
+ function go() {
+ var edit = document.getElementById("edit");
+ document.getElementById("search").appendChild(edit);
+ }
+ </script>
+ </head>
+ <body>
+ <span id="edit" tabindex="0" contenteditable onfocusin="go()">foo</span>
+ <div id="search" role="search"></div>
+ </body>
+ </html>)HTML");
+
+ AtkObject* document = GetRendererAccessible();
+ AtkObject* section = atk_object_ref_accessible_child(document, 0);
+ AtkObject* edit = atk_object_ref_accessible_child(section, 0);
+ ASSERT_TRUE(IsAtkObjectEditable(edit));
+ ASSERT_FALSE(IsAtkObjectFocused(edit));
+
+ AccessibilityNotificationWaiter waiter(
+ shell()->web_contents(), ui::kAXModeComplete,
+ ax::mojom::Event::kTextSelectionChanged);
+
+ EXPECT_TRUE(atk_text_set_selection(ATK_TEXT(edit), 0, 1, 2));
+ waiter.WaitForNotification();
+
+ // When the unfocused contenteditable span has its selection set, focus will
+ // be set. That will trigger the script in the source to move that span to
+ // a different parent, causing focus to be removed and the selection cleared.
+ ASSERT_FALSE(IsAtkObjectFocused(edit));
+ gchar* selected = atk_text_get_selection(ATK_TEXT(edit), 0, nullptr, nullptr);
+ EXPECT_STREQ(selected, nullptr);
+ g_free(selected);
+
+ g_object_unref(edit);
+ g_object_unref(section);
+}
+
+IN_PROC_BROWSER_TEST_F(AccessibilityAuraLinuxBrowserTest,
TestFocusInputTextFields) {
auto verify_selection = [](AtkObject* object, const char* selection) {
gchar* selected_text =
diff --git a/third_party/blink/renderer/core/editing/frame_selection.cc b/third_party/blink/renderer/core/editing/frame_selection.cc
index 2333dc6a..3da05862 100644
--- a/third_party/blink/renderer/core/editing/frame_selection.cc
+++ b/third_party/blink/renderer/core/editing/frame_selection.cc
@@ -349,6 +349,17 @@
TaskType::kMiscPlatformAPI);
}
+void FrameSelection::SetSelectionForAccessibility(
+ const SelectionInDOMTree& selection,
+ const SetSelectionOptions& options) {
+ ClearDocumentCachedRange();
+
+ const bool did_set = SetSelectionDeprecated(selection, options);
+ CacheRangeOfDocument(CreateRange(selection.ComputeRange()));
+ if (did_set)
+ DidSetSelectionDeprecated(selection, options);
+}
+
void FrameSelection::NodeChildrenWillBeRemoved(ContainerNode& container) {
if (!container.InActiveDocument())
return;
diff --git a/third_party/blink/renderer/core/editing/frame_selection.h b/third_party/blink/renderer/core/editing/frame_selection.h
index 69cebad..43d64d19 100644
--- a/third_party/blink/renderer/core/editing/frame_selection.h
+++ b/third_party/blink/renderer/core/editing/frame_selection.h
@@ -166,6 +166,8 @@
const SetSelectionOptions&);
void DidSetSelectionDeprecated(const SelectionInDOMTree&,
const SetSelectionOptions&);
+ void SetSelectionForAccessibility(const SelectionInDOMTree&,
+ const SetSelectionOptions&);
// Call this after doing user-triggered selections to make it easy to delete
// the frame you entirely selected.
diff --git a/third_party/blink/renderer/modules/accessibility/ax_position.h b/third_party/blink/renderer/modules/accessibility/ax_position.h
index 58c23bf..e6d1726 100644
--- a/third_party/blink/renderer/modules/accessibility/ax_position.h
+++ b/third_party/blink/renderer/modules/accessibility/ax_position.h
@@ -191,6 +191,8 @@
uint64_t dom_tree_version_;
uint64_t style_version_;
#endif
+
+ friend class AXSelection;
};
MODULES_EXPORT bool operator==(const AXPosition&, const AXPosition&);
diff --git a/third_party/blink/renderer/modules/accessibility/ax_selection.cc b/third_party/blink/renderer/modules/accessibility/ax_selection.cc
index ad1745a..3a1f49d9 100644
--- a/third_party/blink/renderer/modules/accessibility/ax_selection.cc
+++ b/third_party/blink/renderer/modules/accessibility/ax_selection.cc
@@ -7,6 +7,7 @@
#include "third_party/blink/renderer/core/dom/events/event.h"
#include "third_party/blink/renderer/core/dom/node.h"
#include "third_party/blink/renderer/core/dom/range.h"
+#include "third_party/blink/renderer/core/editing/ephemeral_range.h"
#include "third_party/blink/renderer/core/editing/frame_selection.h"
#include "third_party/blink/renderer/core/editing/position.h"
#include "third_party/blink/renderer/core/editing/position_with_affinity.h"
@@ -329,6 +330,23 @@
return selection_builder.Build();
}
+void AXSelection::UpdateSelectionIfNecessary() {
+ Document* document = base_.ContainerObject()->GetDocument();
+ DCHECK(document);
+
+ LocalFrameView* view = document->View();
+ if (!view || !view->LayoutPending())
+ return;
+
+ document->UpdateStyleAndLayout(DocumentUpdateReason::kSelection);
+#if DCHECK_IS_ON()
+ base_.dom_tree_version_ = extent_.dom_tree_version_ = dom_tree_version_ =
+ document->DomTreeVersion();
+ base_.style_version_ = extent_.style_version_ = style_version_ =
+ document->StyleVersion();
+#endif // DCHECK_IS_ON()
+}
+
bool AXSelection::Select(const AXSelectionBehavior selection_behavior) {
if (!IsValid()) {
NOTREACHED() << "Trying to select an invalid accessibility selection.";
@@ -354,9 +372,9 @@
return true;
}
- const SelectionInDOMTree selection = AsSelection(selection_behavior);
- DCHECK(selection.AssertValid());
- Document* document = selection.Base().GetDocument();
+ const SelectionInDOMTree old_selection = AsSelection(selection_behavior);
+ DCHECK(old_selection.AssertValid());
+ Document* document = old_selection.Base().GetDocument();
if (!document) {
NOTREACHED() << "Valid DOM selections should have an attached document.";
return false;
@@ -374,46 +392,36 @@
// See the following section in the Selection API Specification:
// https://w3c.github.io/selection-api/#selectstart-event
- if (DispatchSelectStart(selection.Extent().ComputeContainerNode()) !=
+ if (DispatchSelectStart(old_selection.Base().ComputeContainerNode()) !=
DispatchEventResult::kNotCanceled) {
return false;
}
+ // If the anchor or focus is removed during the "selectstart" event, do
+ // not proceed.
+ if (base_.ContainerObject()->IsDetached() ||
+ extent_.ContainerObject()->IsDetached())
+ return false;
+
+ UpdateSelectionIfNecessary();
+
+ // Dispatching the "selectstart" event could potentially change the document
+ // associated with the current frame.
+ if (!frame_selection.IsAvailable())
+ return false;
+
+ // Re-retrieve the SelectionInDOMTree in case a DOM mutation took place.
+ // That way it will also have the updated DOM tree and Style versions,
+ // and the SelectionTemplate checks for each won't fail.
+ const SelectionInDOMTree selection = AsSelection(selection_behavior);
+
SetSelectionOptions::Builder options_builder;
options_builder.SetIsDirectional(true)
.SetShouldCloseTyping(true)
.SetShouldClearTypingStyle(true)
.SetSetSelectionBy(SetSelectionBy::kUser);
- frame_selection.ClearDocumentCachedRange();
- frame_selection.SetSelection(selection, options_builder.Build());
-
- // Cache the newly created document range. This doesn't affect the already
- // applied selection. Note that DOM's |Range| object has a start and an end
- // container that need to be in DOM order. See the DOM specification for more
- // information: https://dom.spec.whatwg.org/#interface-range
- Range* range = Range::Create(*document);
- if (selection.Extent().IsNull()) {
- DCHECK(selection.Base().IsNotNull())
- << "AX selections converted to DOM selections should have at least one "
- "endpoint non-null.\n"
- << *this << '\n'
- << selection;
- range->setStart(selection.Base().ComputeContainerNode(),
- selection.Base().ComputeOffsetInContainerNode());
- range->setEnd(selection.Base().ComputeContainerNode(),
- selection.Base().ComputeOffsetInContainerNode());
- } else if (selection.Base() < selection.Extent()) {
- range->setStart(selection.Base().ComputeContainerNode(),
- selection.Base().ComputeOffsetInContainerNode());
- range->setEnd(selection.Extent().ComputeContainerNode(),
- selection.Extent().ComputeOffsetInContainerNode());
- } else {
- range->setStart(selection.Extent().ComputeContainerNode(),
- selection.Extent().ComputeOffsetInContainerNode());
- range->setEnd(selection.Base().ComputeContainerNode(),
- selection.Base().ComputeOffsetInContainerNode());
- }
- frame_selection.CacheRangeOfDocument(range);
+ frame_selection.SetSelectionForAccessibility(selection,
+ options_builder.Build());
return true;
}
diff --git a/third_party/blink/renderer/modules/accessibility/ax_selection.h b/third_party/blink/renderer/modules/accessibility/ax_selection.h
index 8f3a5e4..713e6b8 100644
--- a/third_party/blink/renderer/modules/accessibility/ax_selection.h
+++ b/third_party/blink/renderer/modules/accessibility/ax_selection.h
@@ -94,6 +94,10 @@
AXSelection();
+ // If a layout is pending, update the style and layout along with the DOM
+ // tree and style versions of the AXSelection and associated AXPositions.
+ void UpdateSelectionIfNecessary();
+
// Determines whether this selection is targeted to the contents of a text
// field, and returns the start and end text offsets, as well as its
// direction. |start| should always be less than equal to |end|.
