Skip to content

Commit

Permalink
Make select-to-speak ignore nodes with user-select:none.
Browse files Browse the repository at this point in the history
This CL only affects the behavior of Select-to-Speak when STS is
triggered by selection followed by Search-S (it does not affect STS
triggered from the tray or by mouse selection). Since keyboard selection
does not highlight any nodes marked user-select:none, STS follows the
visual behavior here by ignoring these nodes when spoken out as well.
While this causes the behavior to be inconsistent between mouse- and
keyboard-based invocation, it follows the user's mental model: when a
rectangular selection is made with the mouse, STS speaks out everything
within the rectangle that the user draws, whereas when a selection is
made with the cursor and STS invoked from the keyboard, STS speaks out
whatever was actually selected.

Also added a browser test to verify this behavior.

Bug: 830106
Change-Id: I8552e7bda40f150df2ab4bb0338de706a717f30e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2426973
Commit-Queue: Ajit Narayanan <ajitnarayanan@google.com>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810768}
  • Loading branch information
ajitq authored and Commit Bot committed Sep 25, 2020
1 parent 6f303a8 commit 1f5e8fe
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,20 @@ class NodeUtils {
return ParagraphUtils.isWhitespace(ParagraphUtils.getNodeName(node));
}

/**
* Returns true if the node should be ignored by Select-to-Speak because
* it was marked with user-select:none. For Inline Text elements, the
* parent is marked with this attribute, hence the check.
*
* @param {!AutomationNode} node The node to test
* @return {boolean} whether this node was marked user-select:none
*/
static isNotSelectable(node) {
return node &&
(node.notUserSelectableStyle ||
(node.parent && node.parent.notUserSelectableStyle));
}

/**
* Returns true if a node is invisible for any reason.
* @param {!AutomationNode} node The node to test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,8 @@ class SelectToSpeak {
let selectedNode = firstPosition.node;
if (selectedNode.name && firstPosition.offset < selectedNode.name.length &&
!NodeUtils.shouldIgnoreNode(
selectedNode, /* include offscreen */ true)) {
selectedNode, /* include offscreen */ true) &&
!NodeUtils.isNotSelectable(selectedNode)) {
// Initialize to the first node in the list if it's valid and inside
// of the offset bounds.
nodes.push(selectedNode);
Expand Down Expand Up @@ -304,7 +305,8 @@ class SelectToSpeak {
}
}
if (!NodeUtils.shouldIgnoreNode(
selectedNode, /* include offscreen */ true)) {
selectedNode, /* include offscreen */ true) &&
!NodeUtils.isNotSelectable(selectedNode)) {
nodes.push(selectedNode);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,25 @@ TEST_F(
'This is some bold text');
});

TEST_F(
'SelectToSpeakKeystrokeSelectionTest', 'IgnoresTextMarkedNotUserSelectable',
function() {
this.testReadTextAtKeystroke(
'<div><p>This is some <span style="user-select:none">unselectable</span> text</p></div>',
function(desktop) {
const firstNode =
this.findTextNode(desktop, 'This is some ').root.children[0];
const lastNode = this.findTextNode(desktop, ' text');
chrome.automation.setDocumentSelection({
anchorObject: firstNode,
anchorOffset: 0,
focusObject: lastNode,
focusOffset: 5
});
},
'This is some text');
});

TEST_F(
'SelectToSpeakKeystrokeSelectionTest',
'HandlesSingleImageCorrectlyWithAutomation', function() {
Expand Down
3 changes: 3 additions & 0 deletions extensions/common/api/automation.idl
Original file line number Diff line number Diff line change
Expand Up @@ -1058,6 +1058,9 @@ enum GeneratedEventType {
// The affinity of the tree selection end, if any.
DOMString? selectionEndAffinity;

// Indicates that the node is marked user-select:none
boolean? notUserSelectableStyle;

//
// Range attributes.
//
Expand Down
4 changes: 2 additions & 2 deletions extensions/renderer/resources/automation/automation_node.js
Original file line number Diff line number Diff line change
Expand Up @@ -1317,8 +1317,8 @@ var stringAttributes = [

var boolAttributes = [
'busy', 'clickable', 'containerLiveAtomic', 'containerLiveBusy',
'editableRoot', 'liveAtomic', 'modal', 'scrollable', 'selected',
'supportsTextLocation'
'editableRoot', 'liveAtomic', 'modal', 'notUserSelectableStyle', 'scrollable',
'selected', 'supportsTextLocation'
];

var intAttributes = [
Expand Down

0 comments on commit 1f5e8fe

Please sign in to comment.