Skip to content

Commit

Permalink
ChromeVox: Allow movement by word through non-inline text boxes.
Browse files Browse the repository at this point in the history 
This change adds functionality in ChromeVox to move by word through
elements other than inline text boxes. Previously, when attempting
to move by word through elements such as buttons, ChromeVox would
simply jump to the next element instead of moving by word through
the element. This change was motivated by a bug where it was
impossible to move by word through a Google calendar appointment
because appointments are buttons.

Bug: 948700
Change-Id: I904695c266d025176ae1aa7ed2fe3bfd4d060d5f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1642683
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#671084}
  • Loading branch information
@akihiroota87
akihiroota87 authored and Commit Bot committed Jun 20, 2019
1 parent e590411 commit 6c7cfb6
Show file tree
Hide file tree
Showing 8 changed files with 118 additions and 55 deletions.
30 changes: 25 additions & 5 deletions chrome/browser/resources/chromeos/chromevox/cvox2/background/background_test.extjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,24 +250,44 @@ TEST_F('ChromeVoxBackgroundTest', 'CaretNavigation', function() {
});

/** Tests that individual buttons are stops for move-by-word functionality. */
TEST_F('ChromeVoxBackgroundTest', 'CaretNavigationTreatsButtonAsWord', function() {
TEST_F('ChromeVoxBackgroundTest', 'CaretNavigationMoveThroughButtonByWord', function() {
var mockFeedback = this.createMockFeedback();
this.runWithLoadedTree(this.buttonDoc, function() {
mockFeedback.expectSpeech('start');
mockFeedback.call(doCmd('nextWord'))
mockFeedback.call(doCmd('nextObject'))
.expectSpeech('hello button one', 'Button');
mockFeedback.call(doCmd('previousWord'))
.expectSpeech('start');
mockFeedback.call(doCmd('nextWord'))
.expectSpeech('hello');
mockFeedback.call(doCmd('nextWord'))
.expectSpeech('button');
mockFeedback.call(doCmd('nextWord'))
.expectSpeech('one');
mockFeedback.call(doCmd('nextWord'))
.expectSpeech('cats');
mockFeedback.call(doCmd('nextWord'))
.expectSpeech('hello button two', 'Button');
.expectSpeech('hello');
mockFeedback.call(doCmd('nextWord'))
.expectSpeech('button');
mockFeedback.call(doCmd('nextWord'))
.expectSpeech('two');
mockFeedback.call(doCmd('nextWord'))
.expectSpeech('end');
mockFeedback.call(doCmd('previousWord'))
.expectSpeech('hello button two', 'Button');
.expectSpeech('two');
mockFeedback.call(doCmd('previousWord'))
.expectSpeech('button');
mockFeedback.call(doCmd('previousWord'))
.expectSpeech('hello');
mockFeedback.call(doCmd('previousWord'))
.expectSpeech('cats');
mockFeedback.call(doCmd('previousWord'))
.expectSpeech('hello button one', 'Button');
.expectSpeech('one');
mockFeedback.call(doCmd('previousWord'))
.expectSpeech('button');
mockFeedback.call(doCmd('previousWord'))
.expectSpeech('hello');
mockFeedback.replay();
});
});
Expand Down
66 changes: 36 additions & 30 deletions chrome/browser/resources/chromeos/chromevox/cvox2/background/cursors.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,33 +333,41 @@ cursors.Cursor.prototype = {

switch (movement) {
case Movement.BOUND:
var wordStarts, wordEnds;
if (newNode.role == RoleType.INLINE_TEXT_BOX) {
var start, end;
for (var i = 0; i < newNode.wordStarts.length; i++) {
if (newIndex >= newNode.wordStarts[i] &&
newIndex <= newNode.wordEnds[i]) {
start = newNode.wordStarts[i];
end = newNode.wordEnds[i];
break;
}
}
if (goog.isDef(start) && goog.isDef(end))
newIndex = dir == Dir.FORWARD ? end : start;
wordStarts = newNode.wordStarts;
wordEnds = newNode.wordEnds;
} else {
newIndex = cursors.NODE_INDEX;
wordStarts = newNode.nonInlineTextWordStarts;
wordEnds = newNode.nonInlineTextWordEnds;
}
var start, end;
for (var i = 0; i < wordStarts.length; i++) {
if (newIndex >= wordStarts[i] && newIndex <= wordEnds[i]) {
start = wordStarts[i];
end = wordEnds[i];
break;
}
}
if (goog.isDef(start) && goog.isDef(end))
newIndex = dir == Dir.FORWARD ? end : start;
break;
case Movement.DIRECTIONAL:
var wordStarts, wordEnds;
var start;
if (newNode.role == RoleType.INLINE_TEXT_BOX) {
// Go to the next word stop in the same piece of text.
for (var i = 0; i < newNode.wordStarts.length; i++) {
if (newIndex >= newNode.wordStarts[i] &&
newIndex <= newNode.wordEnds[i]) {
var nextIndex = dir == Dir.FORWARD ? i + 1 : i - 1;
start = newNode.wordStarts[nextIndex];
break;
}
wordStarts = newNode.wordStarts;
wordEnds = newNode.wordEnds;
} else {
wordStarts = newNode.nonInlineTextWordStarts;
wordEnds = newNode.nonInlineTextWordEnds;
}
// Go to the next word stop in the same piece of text.
for (var i = 0; i < wordStarts.length; i++) {
if (newIndex >= wordStarts[i] && newIndex <= wordEnds[i]) {
var nextIndex = dir == Dir.FORWARD ? i + 1 : i - 1;
start = wordStarts[nextIndex];
break;
}
}
if (goog.isDef(start)) {
Expand All @@ -374,16 +382,14 @@ cursors.Cursor.prototype = {
newNode = AutomationUtil.findNextNode(
newNode, dir, AutomationPredicate.leafWithWordStop);
if (newNode) {
if (newNode.role == RoleType.INLINE_TEXT_BOX) {
var starts = newNode.wordStarts;
if (starts.length) {
newIndex = dir == Dir.BACKWARD ?
starts[starts.length - 1] :
starts[0];
}
} else {
// For non-text nodes, move by word = by object.
newIndex = cursors.NODE_INDEX;
var starts;
if (newNode.role == RoleType.INLINE_TEXT_BOX)
starts = newNode.wordStarts;
else
starts = newNode.nonInlineTextWordStarts;
if (starts.length) {
newIndex = dir == Dir.BACKWARD ? starts[starts.length - 1] :
starts[0];
}
}
}
Expand Down
37 changes: 37 additions & 0 deletions chrome/browser/resources/chromeos/chromevox/cvox2/background/cursors_test.extjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,11 @@ simpleDoc: function() {/*!
multiInlineDoc: function() {/*!
<p style='max-width: 5px'>start diff line</p>
<p>end
*/},

buttonAndInlineTextDoc: function() {/*!
<div>Inline text content</div>
<div role="button">Button example content</div>
*/}
};

Expand Down Expand Up @@ -596,3 +601,35 @@ TEST_F('ChromeVoxCursorsTest', 'SelectionAdjustmentsNonRichText', function() {

});
});

TEST_F('ChromeVoxCursorsTest', 'MovementByWordThroughNonInlineText',
function() {this.runCursorMovesOnDocument(this.buttonAndInlineTextDoc,
[
// Move forward by word.
// 'text' start and end indices.
[WORD, DIRECTIONAL, FORWARD, {index: 7, value: 'Inline text content'}],
[WORD, BOUND, FORWARD, {index: 11, value: 'Inline text content'}],
// 'content' start and end incies.
[WORD, DIRECTIONAL, FORWARD, {index: 12, value: 'Inline text content'}],
[WORD, BOUND, FORWARD, {index: 19, value: 'Inline text content'}],
// 'Button' start and end indices.
[WORD, DIRECTIONAL, FORWARD, {index: 0, value: 'Button example content'}],
[WORD, BOUND, FORWARD, {index: 6, value: 'Button example content'}],
// 'example' start and end indices.
[WORD, DIRECTIONAL, FORWARD, {index: 7, value: 'Button example content'}],
[WORD, BOUND, FORWARD, {index: 14, value: 'Button example content'}],
// 'content' start index. Reached last word of last object.
[WORD, DIRECTIONAL, FORWARD, {index: 15, value: 'Button example content'}],
[WORD, DIRECTIONAL, FORWARD, {index: 15, value: 'Button example content'}],

// Move backward by word.
// Only test start indices.
[WORD, DIRECTIONAL, BACKWARD, {index: 7, value: 'Button example content'}],
[WORD, DIRECTIONAL, BACKWARD, {index: 0, value: 'Button example content'}],
[WORD, DIRECTIONAL, BACKWARD, {index: 12, value: 'Inline text content'}],
[WORD, DIRECTIONAL, BACKWARD, {index: 7, value: 'Inline text content'}],
[WORD, DIRECTIONAL, BACKWARD, {index: 0, value: 'Inline text content'}],
// Reached first word of first object.
[WORD, DIRECTIONAL, BACKWARD, {index: 0, value: 'Inline text content'}]
]);
});
4 changes: 2 additions & 2 deletions chrome/test/data/extensions/api_test/automation/tests/tabs/word_boundaries.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ var allTests = [
{ attributes: { name: 'Example text for testing purposes' } });
var expectedWordStarts = [0, 8, 13, 17, 25];
var expectedWordEnds = [7, 12, 16, 24, 33];
var wordStarts = node.wordStartOffsets();
var wordEnds = node.wordEndOffsets();
var wordStarts = node.nonInlineTextWordStarts;
var wordEnds = node.nonInlineTextWordEnds;
assertEq(expectedWordStarts.length, wordStarts.length);
assertEq(expectedWordEnds.length, wordEnds.length);
assertEq(wordStarts.length, wordEnds.length);
Expand Down
4 changes: 2 additions & 2 deletions extensions/common/api/automation.idl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -592,12 +592,12 @@
// The start index of each word within the node's name. This is different
// from wordStarts because it is not restricted to inline text boxes and can
// be used for any type of element.
long[]? wordStartOffsets;
long[]? nonInlineTextWordStarts;

// The end index of each word within the node's name. This is different
// from wordEnds because it is not restricted to inline text boxes and can
// be used for any type of element.
long[]? wordEndOffsets;
long[]? nonInlineTextWordEnds;

// The nodes, if any, which this node is specified to control via
// <a href="http://www.w3.org/TR/wai-aria/states_and_properties#aria-controls">
Expand Down
1 change: 0 additions & 1 deletion extensions/renderer/api/automation/automation_internal_custom_bindings.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -796,7 +796,6 @@ void AutomationInternalCustomBindings::AddRoutes() {
node->GetString16Attribute(ax::mojom::StringAttribute::kName));
result.Set(gin::ConvertToV8(isolate, word_ends));
});

// Bindings that take a Tree ID and Node ID and string attribute name
// and return a property of the node.

Expand Down
20 changes: 10 additions & 10 deletions extensions/renderer/resources/automation/automation_node.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,14 @@ AutomationNodeImpl.prototype = {
return GetTableCellRowIndex(this.treeID, this.id);
},

get nonInlineTextWordStarts() {
return GetWordStartOffsets(this.treeID, this.id);
},

get nonInlineTextWordEnds() {
return GetWordEndOffsets(this.treeID, this.id);
},

doDefault: function() {
this.performAction_('doDefault');
},
Expand Down Expand Up @@ -872,14 +880,6 @@ AutomationNodeImpl.prototype = {
return impl.get(info.nodeId);
},

wordStartOffsets: function() {
return GetWordStartOffsets(this.treeID, this.id);
},

wordEndOffsets: function() {
return GetWordEndOffsets(this.treeID, this.id);
},

addEventListener: function(eventType, callback, capture) {
this.removeEventListener(eventType, callback);
if (!this.listeners[eventType])
Expand Down Expand Up @@ -1701,8 +1701,6 @@ utils.expose(AutomationNode, AutomationNodeImpl, {
'toString',
'boundsForRange',
'languageAnnotationForStringAttribute',
'wordStartOffsets',
'wordEndOffsets',
],
readonly: $Array.concat(
publicAttributes,
Expand Down Expand Up @@ -1739,6 +1737,8 @@ utils.expose(AutomationNode, AutomationNodeImpl, {
'tableCellRowHeaders',
'tableCellColumnIndex',
'tableCellRowIndex',
'nonInlineTextWordStarts',
'nonInlineTextWordEnds',
]),
});

Expand Down
11 changes: 6 additions & 5 deletions third_party/closure_compiler/externs/automation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -652,17 +652,18 @@ chrome.automation.AutomationNode.prototype.wordStarts;
chrome.automation.AutomationNode.prototype.wordEnds;

/**
* The start and end index of each word within the node's name. Different from wordStarts and wordEnds because they're not restricted to inline text boxes and can be used for any type of element.
* The start index of each word within the node's name. This is different from wordStarts because it is not restricted to inline text boxes and can be used for any type of element.
* @type {(!Array<number>|undefined)}
* @see https://developer.chrome.com/extensions/automation#type-wordStartOffsets
* @see https://developer.chrome.com/extensions/automation#type-nonInlineTextWordStarts
*/
chrome.automation.AutomationNode.prototype.wordStartOffsets;
chrome.automation.AutomationNode.prototype.nonInlineTextWordStarts;

/**
* The end index of each word within the node's name. This is different from wordEnds because it is not restricted to inline text boxes and can be used for any type of element.
* @type {(!Array<number>|undefined)}
* @see https://developer.chrome.com/extensions/automation#type-wordEndOffsets
* @see https://developer.chrome.com/extensions/automation#type-nonInlineTextWordEnds
*/
chrome.automation.AutomationNode.prototype.wordEndOffsets;
chrome.automation.AutomationNode.prototype.nonInlineTextWordEnds;

/**
* The nodes, if any, which this node is specified to control via <a href="http://www.w3.org/TR/wai-aria/states_and_properties#aria-controls"> <code>aria-controls</code></a>.
Expand Down

0 comments on commit 6c7cfb6

Please sign in to comment.