hterm: Improve accessibility command output
This improves accessibility command output in several ways:
1) Rather than using multiple elements for rendering the output to a
live region, instead we use a single <p> element which we manipulate the
aria-label attribute of. This is far more performant than the existing
solution. Because of this, it's no longer necessary to limit the amount
of output that we add per iteration which also simplifies logic.
2) As a result of the above change, it's necessary to introduce the
additional step of clearing the previous value of the aria-label prior
to setting its new value. This is necessary so that if the same string
is repeated multiple times, the screen reader will actually register it
as an attribute change. This adds an additional delay between iterations
however this can be optimized later if necessary.
3) Rather than inserting newlines between all strings passed to
announce() we instead only do this when there is actually a newline in
the terminal output. This avoids unwanted interruptions in text that
should appear together in the terminal.
Bug: 822490, 646690
Change-Id: I024d13d24f126d31dae3fe0b2a0d8f818740354a
Reviewed-on: https://chromium-review.googlesource.com/1060716
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Tested-by: Raymes Khoury <raymes@chromium.org>
diff --git a/hterm/js/hterm_accessibility_reader.js b/hterm/js/hterm_accessibility_reader.js
index 1d387f9..7ac6fe0 100644
--- a/hterm/js/hterm_accessibility_reader.js
+++ b/hterm/js/hterm_accessibility_reader.js
@@ -21,14 +21,17 @@
this.document_ = div.ownerDocument;
// The live region element to add text to.
- this.liveRegion_ = this.document_.createElement('div');
- this.liveRegion_.id = 'hterm:accessibility-live-region';
- this.liveRegion_.setAttribute('aria-live', 'polite');
- this.liveRegion_.style.cssText = `position: absolute;
- width: 0; height: 0;
- overflow: hidden;
- left: 0; top: 0;`;
- div.appendChild(this.liveRegion_);
+ const liveRegion = this.document_.createElement('div');
+ liveRegion.id = 'hterm:accessibility-live-region';
+ liveRegion.setAttribute('aria-live', 'polite');
+ liveRegion.style.cssText = `position: absolute;
+ width: 0; height: 0;
+ overflow: hidden;
+ left: 0; top: 0;`;
+ div.appendChild(liveRegion);
+ this.liveElement_ = this.document_.createElement('p');
+ this.liveElement_.setAttribute('aria-label', '');
+ liveRegion.appendChild(this.liveElement_);
// A queue of updates to announce.
this.queue_ = [];
@@ -41,40 +44,16 @@
};
/**
- * Initial delay in ms to use for merging strings to output.
+ * Delay in ms to use for merging strings to output.
*
- * Only used if no output has been spoken recently. We want this to be
- * relatively short so there's not a big delay between typing/executing commands
- * and hearing output.
+ * We merge strings together to avoid hanging the terminal and to ensure that
+ * aria updates make it to the screen reader. We want this to be short so
+ * there's not a big delay between typing/executing commands and hearing output.
*
* @constant
* @type {integer}
*/
-hterm.AccessibilityReader.INITIAL_DELAY = 50;
-
-/**
- * Delay for bufferring subsequent strings of output in ms.
-
- * This can be longer because text is already being spoken. Having too small a
- * delay interferes with performance for large amounts of output. A larger delay
- * may cause interruptions to speech.
- *
- * @constant
- * @type {integer}
- */
-hterm.AccessibilityReader.SUBSEQUENT_DELAY = 100;
-
-/**
- * The maximum number of strings to add to the live region in a single pass.
- *
- * If this is too large performance will suffer. If it is too small, it will
- * take too long to add text to the live region and may cause interruptions
- * to speech.
- *
- * @constant
- * @type {integer}
- */
-hterm.AccessibilityReader.MAX_ITEMS_TO_ADD = 100;
+hterm.AccessibilityReader.DELAY = 90;
/**
* Announce the command output.
@@ -82,10 +61,18 @@
* @param {string} str The string to announce using a live region.
*/
hterm.AccessibilityReader.prototype.announce = function(str) {
- // TODO(raymes): If the string being added is on the same line as previous
- // strings in the queue, merge them so that the reading of the text doesn't
- // stutter.
- this.queue_.push(str);
+ if (this.queue_.length == 0) {
+ this.queue_.push(str);
+ } else {
+ // We put a space between strings that appear on the same line.
+ // TODO(raymes): We should check the location on the row and not add a space
+ // if the strings are joined together.
+ let padding = '';
+ if (this.queue_[this.queue_.length - 1].length != 0) {
+ padding = ' ';
+ }
+ this.queue_[this.queue_.length - 1] += padding + str;
+ }
// If we've already scheduled text being added to the live region, wait for it
// to happen.
@@ -98,7 +85,7 @@
// related strings.
if (this.queue_.length == 1) {
this.nextReadTimer_ = setTimeout(this.onNextReadTimer_.bind(this),
- hterm.AccessibilityReader.INITIAL_DELAY);
+ hterm.AccessibilityReader.DELAY / 2);
} else {
throw new Error(
'Expected only one item in queue_ or nextReadTimer_ to be running.');
@@ -106,54 +93,58 @@
};
/**
- * Clear the live region.
+ * Add a newline to the text that will be announced to the live region.
+ */
+hterm.AccessibilityReader.prototype.newLine = function() {
+ this.queue_.push('');
+};
+
+/**
+ * Clear the live region and any in-flight announcements.
*/
hterm.AccessibilityReader.prototype.clear = function() {
- while (this.liveRegion_.firstChild) {
- this.liveRegion_.firstChild.remove();
- }
+ this.liveElement_.setAttribute('aria-label', '');
+ this.queue_ = [];
};
/**
* Add text from queue_ to the live region.
*
- * This limits the amount of text that will be added in one go and schedules
- * another call to addLiveRegion_ afterwards to continue adding text until
- * queue_ is empty.
*/
hterm.AccessibilityReader.prototype.addToLiveRegion_ = function() {
if (this.nextReadTimer_) {
throw new Error('Expected nextReadTimer_ not to be running.');
}
- // Clear the live region so it doesn't grow indefinitely. As soon as elements
- // are added to the DOM, the screen reader will be informed so we don't need
- // to keep elements around after that.
- this.clear();
-
- for (let i = 0; i < hterm.AccessibilityReader.MAX_ITEMS_TO_ADD; ++i) {
- const str = this.queue_.shift();
- const liveElement = this.document_.createElement('p');
- liveElement.innerText = str;
- this.liveRegion_.appendChild(liveElement);
- if (this.queue_.length == 0) {
- break;
- }
- }
-
- if (this.queue_.length > 0) {
- this.nextReadTimer_ = setTimeout(
- this.onNextReadTimer_.bind(this),
- hterm.AccessibilityReader.SUBSEQUENT_DELAY);
- }
+ // As soon as the aria-label is changed, the screen reader will be informed so
+ // we can re-use the same element.
+ // TODO(raymes): One downside of this approach is that strings that span two
+ // calls to addToLiveRegion_ will have a newline placed between them. We could
+ // try to use heuristics to avoid this but it would be more complicated and it
+ // should only happen for large amounts of output.
+ this.liveElement_.setAttribute('aria-label', this.queue_.join('\n'));
+ this.queue_ = [];
};
/**
* Fired when nextReadTimer_ finishes.
*
- * This clears the timer and calls addToLiveRegion_.
+ * This clears the aria-label attribute and sets up a call to onClearFinished_.
*/
hterm.AccessibilityReader.prototype.onNextReadTimer_ = function() {
+ this.liveElement_.setAttribute('aria-label', '');
+ // We need to wait for the screen reader to register that the attribute is
+ // cleared. This is only necessary if the string to be announced is identical
+ // to the previous string to be announced.
+ // TODO(raymes): Optimize for the above case if necessary.
+ setTimeout(this.onClearFinished_.bind(this),
+ hterm.AccessibilityReader.DELAY / 2);
+};
+
+/**
+ * Fired when sufficient time has passed to clear the aria-label attribute.
+ */
+hterm.AccessibilityReader.prototype.onClearFinished_ = function() {
this.nextReadTimer_ = null;
this.addToLiveRegion_();
};
diff --git a/hterm/js/hterm_accessibility_reader_tests.js b/hterm/js/hterm_accessibility_reader_tests.js
index 10462dc..a5cb497 100644
--- a/hterm/js/hterm_accessibility_reader_tests.js
+++ b/hterm/js/hterm_accessibility_reader_tests.js
@@ -27,7 +27,7 @@
div.style.width = '100%';
this.accessibilityReader = new hterm.AccessibilityReader(div);
- this.liveRegion = div.firstChild;
+ this.liveElement = div.firstChild.firstChild;
document.body.appendChild(div);
};
@@ -41,29 +41,43 @@
'a11y-live-region-single-delay', function(result, cx) {
this.accessibilityReader.announce('Some test output');
this.accessibilityReader.announce('Some other test output');
+ this.accessibilityReader.newLine();
+ this.accessibilityReader.announce('More output');
- result.assertEQ(0, this.liveRegion.children.length);
+ result.assertEQ('', this.liveElement.getAttribute('aria-label'));
+
+ const checkClear = () => {
+ result.assertEQ('',
+ this.liveElement.getAttribute('aria-label'));
+ return true;
+ };
+
+ const checkFirstAnnounce = () => {
+ result.assertEQ('Some test output Some other test output\nMore output',
+ this.liveElement.getAttribute('aria-label'));
+ return true;
+ };
+
+ const checksToComplete = [checkClear, checkFirstAnnounce];
const observer = new MutationObserver(() => {
- if (this.liveRegion.children.length < 2) {
- return;
+ if (checksToComplete[0]()) {
+ checksToComplete.shift();
}
- result.assertEQ('Some test output',
- this.liveRegion.children[0].innerHTML);
- result.assertEQ('Some other test output',
- this.liveRegion.children[1].innerHTML);
-
- observer.disconnect();
- result.pass();
+ if (checksToComplete.length == 0) {
+ observer.disconnect();
+ result.pass();
+ }
});
- observer.observe(this.liveRegion, {childList: true});
- // This should only need to be 1x the initial delay but we wait longer to
+ observer.observe(this.liveElement, {attributes: true});
+ // This should only need to be 2x the initial delay but we wait longer to
// avoid flakiness.
result.requestTime(500);
});
+
/**
* Test that after text has been added to the live region, there is again a
* delay before adding more text.
@@ -72,35 +86,36 @@
'a11y-live-region-double-delay', function(result, cx) {
this.accessibilityReader.announce('Some test output');
this.accessibilityReader.announce('Some other test output');
+ this.accessibilityReader.newLine();
+ this.accessibilityReader.announce('More output');
- result.assertEQ(0, this.liveRegion.children.length);
+ result.assertEQ('', this.liveElement.getAttribute('aria-label'));
+
+ const checkClear = () => {
+ result.assertEQ('', this.liveElement.getAttribute('aria-label'));
+ return true;
+ };
const checkFirstAnnounce = () => {
- if (this.liveRegion.children.length < 2) {
- return false;
- }
-
- result.assertEQ('Some test output',
- this.liveRegion.children[0].innerHTML);
- result.assertEQ('Some other test output',
- this.liveRegion.children[1].innerHTML);
+ result.assertEQ('Some test output Some other test output\nMore output',
+ this.liveElement.getAttribute('aria-label'));
this.accessibilityReader.announce('more text');
+ this.accessibilityReader.newLine();
this.accessibilityReader.announce('...and more');
return true;
};
const checkSecondAnnounce = () => {
- if (this.liveRegion.children.length < 2) {
- return false;
- }
-
- result.assertEQ('more text', this.liveRegion.children[0].innerHTML);
- result.assertEQ('...and more', this.liveRegion.children[1].innerHTML);
+ result.assertEQ('more text\n...and more',
+ this.liveElement.getAttribute('aria-label'));
return true;
};
- const checksToComplete = [checkFirstAnnounce, checkSecondAnnounce];
+ const checksToComplete = [checkClear,
+ checkFirstAnnounce,
+ checkClear,
+ checkSecondAnnounce];
const observer = new MutationObserver(() => {
if (checksToComplete[0]()) {
@@ -113,62 +128,8 @@
}
});
- observer.observe(this.liveRegion, {childList: true});
+ observer.observe(this.liveElement, {attributes: true});
// This should only need to be 2x the initial delay but we wait longer to
// avoid flakiness.
result.requestTime(500);
});
-
-/**
- * Test that when adding a large amount of text, it will get buffered into the
- * live region.
- */
-hterm.AccessibilityReader.Tests.addTest(
- 'a11y-live-region-large-text', function(result, cx) {
- for (let i = 0; i < hterm.AccessibilityReader.MAX_ITEMS_TO_ADD; ++i) {
- this.accessibilityReader.announce('First pass');
- }
- this.accessibilityReader.announce('Second pass');
-
- result.assertEQ(0, this.liveRegion.children.length);
-
- const checkFirstAnnounce = () => {
- if (this.liveRegion.children.length <
- hterm.AccessibilityReader.MAX_ITEMS_TO_ADD) {
- return false;
- }
-
- for (let i = 0; i < hterm.AccessibilityReader.MAX_ITEMS_TO_ADD; ++i) {
- result.assertEQ('First pass', this.liveRegion.children[i].innerHTML);
- }
-
- return true;
- };
-
- const checkSecondAnnounce = () => {
- if (this.liveRegion.children.length < 1) {
- return false;
- }
-
- result.assertEQ('Second pass', this.liveRegion.children[0].innerHTML);
- return true;
- };
-
- const checksToComplete = [checkFirstAnnounce, checkSecondAnnounce];
-
- const observer = new MutationObserver(() => {
- if (checksToComplete[0]()) {
- checksToComplete.shift();
- }
-
- if (checksToComplete.length == 0) {
- observer.disconnect();
- result.pass();
- }
- });
-
- observer.observe(this.liveRegion, {childList: true});
- // This should only need to be the initial delay plus the subsequent delay
- // but we use a longer delay to avoid flakiness.
- result.requestTime(500);
-});
diff --git a/hterm/js/hterm_terminal.js b/hterm/js/hterm_terminal.js
index 306f87b..4b7791a 100644
--- a/hterm/js/hterm_terminal.js
+++ b/hterm/js/hterm_terminal.js
@@ -1776,8 +1776,9 @@
*/
hterm.Terminal.prototype.print = function(str) {
// Basic accessibility output for the screen reader.
- if (this.accessibilityEnabled_)
+ if (this.accessibilityEnabled_) {
this.accessibilityReader_.announce(str);
+ }
var startOffset = 0;
@@ -1790,7 +1791,7 @@
while (startOffset < strWidth) {
if (this.options_.wraparound && this.screen_.cursorPosition.overflow) {
this.screen_.commitLineOverflow();
- this.newLine();
+ this.newLine(true);
}
var count = strWidth - startOffset;
@@ -1903,8 +1904,14 @@
* buffer.
*
* Otherwise, this moves the cursor to column zero of the next row.
+ *
+ * @param {boolean=} dueToOverflow Whether the newline is due to wraparound of
+ * the terminal.
*/
-hterm.Terminal.prototype.newLine = function() {
+hterm.Terminal.prototype.newLine = function(dueToOverflow = false) {
+ if (!dueToOverflow)
+ this.accessibilityReader_.newLine();
+
var cursorAtEndOfScreen = (this.screen_.cursorPosition.row ==
this.screen_.rowsArray.length - 1);