Commit 0c7892d1 authored by David Tseng's avatar David Tseng Committed by Commit Bot

Fix degraded typing feedback

- EditableTextBase's state transitions that detect insertions are unreliable:
1. when typing quickly, Chrome can and does combine events. type the keys 1, 2,
3 quickly and you might get value/text changes for "1", "12", and "123"; or you
might get value changes for "1", "123". The latter is really bad and triggers
the insertion (with length > 1) state erroniously.
2. the source of the insertion is unclear. It might have been from a paste, programmatic splicing.
3. It is impossible to reconstruct what part of the text actually got
inserted. Trivial example: 'aaa', 'aaaaaaaa'. Unknown whether the entire value was replaced, or just a substring.

In this change, we will turn off insertion announcements, which were being
triggered in many undesirable code paths.

Instead, we will:
- listen to clipboard events for cut, copy, paste (and
remove the previous copy command)
- fixes the regression introduced by splitting
text field into several subroles. Set selection wasn't working.

Test: type a lot with various typing echo settings and cut/copy/paste.

Bug: 791157
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ida442aa0656c1d66e489c6064f8959ccb7177b46
Reviewed-on: https://chromium-review.googlesource.com/804535
Commit-Queue: David Tseng <dtseng@chromium.org>
Reviewed-by: default avatarDominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521793}
parent 76178d77
...@@ -752,15 +752,6 @@ ...@@ -752,15 +752,6 @@
} }
} }
}, },
{
"command": "copy",
"sequence": {
"keys": {
"keyCode": [67],
"ctrlKey": [true]
}
}
},
{ {
"command": "previousRow", "command": "previousRow",
"sequence": { "sequence": {
......
...@@ -404,6 +404,7 @@ TEST_F('CvoxEventWatcherUnitTest', 'EditableTextListbox', function() { ...@@ -404,6 +404,7 @@ TEST_F('CvoxEventWatcherUnitTest', 'EditableTextListbox', function() {
*/ */
TEST_F('CvoxEventWatcherUnitTest', 'EditableTextListboxUpdatingInput', TEST_F('CvoxEventWatcherUnitTest', 'EditableTextListboxUpdatingInput',
function() { function() {
cvox.ChromeVoxEditableTextBase.shouldSpeakInsertions = true;
this.loadHtml('<div>' + this.loadHtml('<div>' +
'<button id="before">Before</button>' + '<button id="before">Before</button>' +
'<label for="input">Query</label>' + '<label for="input">Query</label>' +
......
...@@ -194,6 +194,13 @@ cvox.ChromeVoxEditableTextBase.useIBeamCursor = cvox.ChromeVox.isMac; ...@@ -194,6 +194,13 @@ cvox.ChromeVoxEditableTextBase.useIBeamCursor = cvox.ChromeVox.isMac;
cvox.ChromeVoxEditableTextBase.eventTypingEcho = false; cvox.ChromeVoxEditableTextBase.eventTypingEcho = false;
/**
* @type {boolean} Whether insertions (i.e. changes of greater than one
* character) should be spoken.
*/
cvox.ChromeVoxEditableTextBase.shouldSpeakInsertions = false;
/** /**
* The maximum number of characters that are short enough to speak in response * The maximum number of characters that are short enough to speak in response
* to an event. For example, if the user selects "Hello", we will speak * to an event. For example, if the user selects "Hello", we will speak
...@@ -619,6 +626,8 @@ cvox.ChromeVoxEditableTextBase.prototype.describeTextChangedHelper = function( ...@@ -619,6 +626,8 @@ cvox.ChromeVoxEditableTextBase.prototype.describeTextChangedHelper = function(
var triggeredByUser = evt.triggeredByUser; var triggeredByUser = evt.triggeredByUser;
if (insertedLen > 1) { if (insertedLen > 1) {
if (!cvox.ChromeVoxEditableTextBase.shouldSpeakInsertions)
return;
utterance = inserted; utterance = inserted;
} else if (insertedLen == 1) { } else if (insertedLen == 1) {
if ((cvox.ChromeVox.typingEcho == cvox.TypingEcho.WORD || if ((cvox.ChromeVox.typingEcho == cvox.TypingEcho.WORD ||
......
...@@ -111,6 +111,7 @@ CvoxEditableTextUnitTest.prototype = { ...@@ -111,6 +111,7 @@ CvoxEditableTextUnitTest.prototype = {
cvox.ChromeVoxEditableTextBase.useIBeamCursor = true; cvox.ChromeVoxEditableTextBase.useIBeamCursor = true;
cvox.ChromeVox.typingEcho = cvox.TypingEcho.CHARACTER_AND_WORD; cvox.ChromeVox.typingEcho = cvox.TypingEcho.CHARACTER_AND_WORD;
cvox.ChromeVoxEditableTextBase.eventTypingEcho = false; cvox.ChromeVoxEditableTextBase.eventTypingEcho = false;
cvox.ChromeVoxEditableTextBase.shouldSpeakInsertions = true;
cvox.ChromeVox.braille = new TestBraille(); cvox.ChromeVox.braille = new TestBraille();
/** Simple mock. */ /** Simple mock. */
......
...@@ -159,6 +159,13 @@ Background = function() { ...@@ -159,6 +159,13 @@ Background = function() {
chrome.accessibilityPrivate.onAccessibilityGesture.addListener( chrome.accessibilityPrivate.onAccessibilityGesture.addListener(
this.onAccessibilityGesture_); this.onAccessibilityGesture_);
document.addEventListener('copy', this.onClipboardEvent_);
document.addEventListener('cut', this.onClipboardEvent_);
document.addEventListener('paste', this.onClipboardEvent_);
/** @private {boolean} */
this.preventPasteOutput_ = false;
/** /**
* Maps a non-desktop root automation node to a range position suitable for * Maps a non-desktop root automation node to a range position suitable for
* restoration. * restoration.
...@@ -734,6 +741,42 @@ Background.prototype = { ...@@ -734,6 +741,42 @@ Background.prototype = {
return true; return true;
}, },
/**
* Detects various clipboard events and provides spoken output.
*
* Note that paste is explicitly skipped sometimes because during a copy or
* cut, the copied or cut text is retrieved by pasting into a fake text
* area. To prevent this from triggering paste output, this staste is tracked
* via a field.
* @param {!Event} evt
* @private
*/
onClipboardEvent_: function(evt) {
var text = '';
if (evt.type == 'paste') {
if (this.preventPasteOutput_) {
this.preventPasteOutput_ = false;
return;
}
text = evt.clipboardData.getData('text');
cvox.ChromeVox.tts.speak(
Msgs.getMsg(evt.type, [text]), cvox.QueueMode.QUEUE);
} else if (evt.type == 'copy' || evt.type == 'cut') {
window.setTimeout(function() {
this.preventPasteOutput_ = true;
var textarea = document.createElement('textarea');
document.body.appendChild(textarea);
textarea.focus();
document.execCommand('paste');
var clipboardContent = textarea.value;
textarea.remove();
cvox.ChromeVox.tts.speak(
Msgs.getMsg(evt.type, [clipboardContent]), cvox.QueueMode.FLUSH);
ChromeVoxState.instance.pageSel_ = null;
}.bind(this), 20);
}
},
/** @private */ /** @private */
setCurrentRangeToFocus_: function() { setCurrentRangeToFocus_: function() {
chrome.automation.getFocus(function(focus) { chrome.automation.getFocus(function(focus) {
......
...@@ -576,19 +576,6 @@ CommandHandler.onCommand = function(command) { ...@@ -576,19 +576,6 @@ CommandHandler.onCommand = function(command) {
var target = ChromeVoxState.instance.currentRange_.start.node.root; var target = ChromeVoxState.instance.currentRange_.start.node.root;
output.withString(target.docUrl || '').go(); output.withString(target.docUrl || '').go();
return false; return false;
case 'copy':
window.setTimeout(function() {
var textarea = document.createElement('textarea');
document.body.appendChild(textarea);
textarea.focus();
document.execCommand('paste');
var clipboardContent = textarea.value;
textarea.remove();
cvox.ChromeVox.tts.speak(
Msgs.getMsg('copy', [clipboardContent]), cvox.QueueMode.FLUSH);
ChromeVoxState.instance.pageSel_ = null;
}, 20);
return true;
case 'toggleSelection': case 'toggleSelection':
if (!ChromeVoxState.instance.pageSel_) { if (!ChromeVoxState.instance.pageSel_) {
ChromeVoxState.instance.pageSel_ = ChromeVoxState.instance.currentRange; ChromeVoxState.instance.pageSel_ = ChromeVoxState.instance.currentRange;
......
...@@ -440,10 +440,13 @@ DesktopAutomationHandler.prototype = { ...@@ -440,10 +440,13 @@ DesktopAutomationHandler.prototype = {
* @param {!AutomationEvent} evt * @param {!AutomationEvent} evt
*/ */
onValueChanged: function(evt) { onValueChanged: function(evt) {
// Delegate to the edit text handler if this is an editable but not richly // Skip all unfocused text fields.
// editable which gets handled in text and text selection changed events. if (!evt.target.state[StateType.FOCUSED] &&
if (evt.target.state[StateType.EDITABLE] && evt.target.state[StateType.EDITABLE])
!evt.target.state[StateType.RICHLY_EDITABLE]) { return;
// Delegate to the edit text handler if this is an editable.
if (evt.target.state[StateType.EDITABLE]) {
this.onEditableChanged_(evt); this.onEditableChanged_(evt);
return; return;
} }
......
...@@ -126,7 +126,6 @@ TEST_F('EditingTest', 'TextButNoSelectionChange', function() { ...@@ -126,7 +126,6 @@ TEST_F('EditingTest', 'TextButNoSelectionChange', function() {
.expectBraille('text1 ed', {startIndex: 0, endIndex: 0}) .expectBraille('text1 ed', {startIndex: 0, endIndex: 0})
.call(input.setSelection.bind(input, 5, 5)) .call(input.setSelection.bind(input, 5, 5))
.expectBraille('text2 ed', {startIndex: 5, endIndex: 5}) .expectBraille('text2 ed', {startIndex: 5, endIndex: 5})
.expectSpeech('text2');
mockFeedback.replay(); mockFeedback.replay();
}); });
......
...@@ -526,7 +526,7 @@ AutomationNodeImpl.prototype = { ...@@ -526,7 +526,7 @@ AutomationNodeImpl.prototype = {
}, },
setSelection: function(startIndex, endIndex) { setSelection: function(startIndex, endIndex) {
if (this.role == 'textField' || this.role == 'textBox') { if (this.state.editable) {
this.performAction_('setSelection', this.performAction_('setSelection',
{ focusNodeID: this.id, { focusNodeID: this.id,
anchorOffset: startIndex, anchorOffset: startIndex,
......
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