Skip to content

GitLab

T
tangled
Commit 21937dad authored by Nektarios Paisios's avatar Nektarios Paisios Committed by Commit Bot
Browse files
Options

Blink should be able to handle selection on AX objects with no nodes. 

Tries to deal with the most egregious bugs reported by VFO that have to do with selection.
R=dmazzoni@chromium.org
TESTED=Layout tests, manually with Jaws

Bug: 747452
Change-Id: I5806087e4c9871150fc33dd423641d61fadee1e3
Reviewed-on: https://chromium-review.googlesource.com/580541
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Reviewed-by: default avatarDominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488721}
parent 1be4d5c1
Hide whitespace changes
Inline Side-by-side
Showing with 75 additions and 35 deletions
content/browser/accessibility/browser_accessibility_com_win.cc
View file @ 21937dad
......@@ -2370,40 +2370,19 @@ void BrowserAccessibilityComWin::ComputeStylesIfNeeded() {
// |offset| could either be a text character or a child index in case of
// non-text objects.
// TODO(nektar): Remove this function once selection bugs are fixed in Blink.
// Currently, to be safe, we convert to text leaf equivalents and we don't use
// tree positions.
// TODO(nektar): Remove this function once selection fixes in Blink are
// thoroughly tested and convert to tree positions.
AXPlatformPosition::AXPositionInstance
BrowserAccessibilityComWin::CreatePositionForSelectionAt(int offset) const {
if (!owner()->IsNativeTextControl() && !owner()->IsTextOnlyObject()) {
auto* manager = Manager();
DCHECK(manager);
const BrowserAccessibilityComWin* child = this;
// TODO(nektar): Make parents of text-only objects not include the text of
// children in their hypertext.
for (size_t i = 0; i < owner()->InternalChildCount(); ++i) {
int new_offset = offset;
child = ToBrowserAccessibilityComWin(owner()->InternalGetChild(i));
DCHECK(child);
if (child->owner()->IsTextOnlyObject()) {
new_offset -= child->owner()->GetText().length();
} else {
new_offset -= 1;
}
if (new_offset <= 0)
break;
offset = new_offset;
}
AXPlatformPositionInstance position =
AXPlatformPosition::CreateTextPosition(manager->ax_tree_id(),
child->owner()->GetId(), offset,
ui::AX_TEXT_AFFINITY_DOWNSTREAM)
->AsLeafTextPosition();
if (position->GetAnchor() &&
position->GetAnchor()->GetRole() == ui::AX_ROLE_INLINE_TEXT_BOX) {
return position->CreateParentPosition();
AXPlatformPositionInstance position =
owner()->CreatePositionAt(offset)->AsLeafTextPosition();
if (position->GetAnchor() &&
position->GetAnchor()->GetRole() == ui::AX_ROLE_INLINE_TEXT_BOX) {
return position->CreateParentPosition();
}
return position;
}
return owner()->CreatePositionAt(offset);
}
//
......@@ -2899,8 +2878,8 @@ void BrowserAccessibilityComWin::SetIA2HypertextSelection(LONG start_offset,
CreatePositionForSelectionAt(static_cast<int>(start_offset));
AXPlatformPositionInstance end_position =
CreatePositionForSelectionAt(static_cast<int>(end_offset));
Manager()->SetSelection(AXPlatformRange(start_position->AsTextPosition(),
end_position->AsTextPosition()));
Manager()->SetSelection(
AXPlatformRange(std::move(start_position), std::move(end_position)));
}
bool BrowserAccessibilityComWin::IsHyperlink() const {
......
third_party/WebKit/LayoutTests/accessibility/list-with-selection.html 0 → 100644
View file @ 21937dad
<!DOCTYPE html>
<script src="../resources/testharness.js"></script>
<script src="../resources/testharnessreport.js"></script>
<ul id="ul">
<li>Item1</li>
<li>Item2</li>
</ul>
<ul id="ul_editable" contenteditable>
<li>Item1</li>
<li>Item2</li>
</ul>
<ol id="ol">
<li>Item1</li>
<li>Item2</li>
</ol>
<ol id="ol_editable" contenteditable>
<li>Item1</li>
<li>Item2</li>
</ol>
<script>
test(function() {
if (!window.accessibilityController)
return;
let ids = ['ul', 'ul_editable', 'ol', 'ol_editable'];
for (let id of ids) {
let axList = accessibilityController.accessibleElementById(id);
assert_not_equals(axList, undefined, id);
// Select both items in the list.
axList.setSelectedTextRange(0, 2);
// |selectionStart| and |selectionEnd| are character-based.
let selectionText = 'Item1\nItem2';
assert_equals(axList.selectionStart, 0, id);
assert_equals(axList.selectionEnd, selectionText.length, id);
let selection = window.getSelection();
assert_equals(selection.toString(), selectionText, id);
if (window.testRunner)
document.getElementById(id).style.display = "none";;
}
}, 'Setting the selection on various lists through the accessibility APIs should work.');
</script>
\ No newline at end of file
third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp
View file @ 21937dad
......@@ -1896,6 +1896,14 @@ AXLayoutObject* AXLayoutObject::GetUnignoredObjectFromNode(Node& node) const {
// Convert from an accessible object and offset to a VisiblePosition.
static VisiblePosition ToVisiblePosition(AXObject* obj, int offset) {
if (!obj || offset < 0)
return VisiblePosition();
// Some objects don't have an associated node, e.g. |LayoutListMarker|.
if (obj->GetLayoutObject() && !obj->GetNode() && obj->ParentObject()) {
return ToVisiblePosition(obj->ParentObject(), obj->IndexInParent());
}
if (!obj->GetNode())
return VisiblePosition();
......@@ -1903,7 +1911,7 @@ static VisiblePosition ToVisiblePosition(AXObject* obj, int offset) {
if (!node->IsTextNode()) {
int child_count = obj->Children().size();
// Place position immediately before the container node, if there was no
// Place position immediately before the container node, if there were no
// children.
if (child_count == 0) {
if (!obj->ParentObject())
......@@ -1921,10 +1929,13 @@ static VisiblePosition ToVisiblePosition(AXObject* obj, int offset) {
static_cast<unsigned>(offset) > (obj->Children().size() - 1)
? offset - 1
: offset;
AXObject* child_obj = obj->Children()[clamped_offset];
Node* child_node = child_obj->GetNode();
// If a particular child can't be selected, expand to select the whole
// object.
if (!child_node || !child_node->parentNode())
return VisiblePosition();
return ToVisiblePosition(obj->ParentObject(), obj->IndexInParent());
// The index in parent.
int adjusted_offset = child_node->NodeIndex();
......@@ -1983,7 +1994,7 @@ void AXLayoutObject::SetSelection(const AXRange& selection) {
}
LocalFrame* frame = GetLayoutObject()->GetFrame();
if (!frame)
if (!frame || !frame->Selection().IsAvailable())
return;
// TODO(editing-dev): Use of updateStyleAndLayoutIgnorePendingStylesheets
......
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