Bug 1321862 - Use charLength for word highlight in Narrate. r?Gijs draft
authorEitan Isaacson <eitan@monotonous.org>
Thu, 17 Nov 2016 11:55:12 -0800
changeset 451562 87b563bcefdf70f0ef2ca7bfe300459f25114efa
parent 451111 d4b3146a5567a7ddbcdfa5244945db55616cb8d1
child 540071 04b2189103bf914261c13728e42fc6ad7e9a83ac
push id39228
push userbmo:eitan@monotonous.org
push dateTue, 20 Dec 2016 17:32:53 +0000
reviewersGijs
bugs1321862
milestone53.0a1
Bug 1321862 - Use charLength for word highlight in Narrate. r?Gijs MozReview-Commit-ID: 8mdjtRj2ljw
toolkit/components/narrate/Narrator.jsm
toolkit/components/narrate/test/NarrateTestUtils.jsm
toolkit/components/narrate/test/browser_word_highlight.js
--- a/toolkit/components/narrate/Narrator.jsm
+++ b/toolkit/components/narrate/Narrator.jsm
@@ -215,28 +215,22 @@ Narrator.prototype = {
       });
 
       utterance.addEventListener("boundary", e => {
         if (e.name != "word") {
           // We are only interested in word boundaries for now.
           return;
         }
 
-        // Match non-whitespace. This isn't perfect, but the most universal
-        // solution for now.
-        let reWordBoundary = /\S+/g;
-        // Match the first word from the boundary event offset.
-        reWordBoundary.lastIndex = e.charIndex;
-        let firstIndex = reWordBoundary.exec(paragraph.textContent);
-        if (firstIndex) {
-          highlighter.highlight(firstIndex.index, reWordBoundary.lastIndex);
+        if (e.charLength) {
+          highlighter.highlight(e.charIndex, e.charLength);
           if (this._inTest) {
             this._sendTestEvent("wordhighlight", {
-              start: firstIndex.index,
-              end: reWordBoundary.lastIndex
+              start: e.charIndex,
+              end: e.charIndex + e.charLength
             });
           }
         }
       });
 
       this._win.speechSynthesis.speak(utterance);
     });
   },
@@ -315,21 +309,21 @@ function Highlighter(container) {
   this.container = container;
 }
 
 Highlighter.prototype = {
   /**
    * Highlight the range within offsets relative to the container.
    *
    * @param {Number} startOffset the start offset
-   * @param {Number} endOffset the end offset
+   * @param {Number} length the length in characters of the range
    */
-  highlight: function(startOffset, endOffset) {
+  highlight: function(startOffset, length) {
     let containerRect = this.container.getBoundingClientRect();
-    let range = this._getRange(startOffset, endOffset);
+    let range = this._getRange(startOffset, startOffset + length);
     let rangeRects = range.getClientRects();
     let win = this.container.ownerDocument.defaultView;
     let computedStyle = win.getComputedStyle(range.endContainer.parentNode);
     let nodes = this._getFreshHighlightNodes(rangeRects.length);
 
     let textStyle = {};
     for (let textStyleRule of kTextStylesRules) {
       textStyle[textStyleRule] = computedStyle[textStyleRule];
--- a/toolkit/components/narrate/test/NarrateTestUtils.jsm
+++ b/toolkit/components/narrate/test/NarrateTestUtils.jsm
@@ -120,18 +120,18 @@ this.NarrateTestUtils = {
         Services.prefs.removeObserver(pref, observeChange);
         resolve(Preferences.get(pref));
       }
 
       Services.prefs.addObserver(pref, observeChange, false);
     });
   },
 
-  sendBoundaryEvent: function(window, name, charIndex) {
-    let detail = { type: "boundary", args: { name, charIndex } };
+  sendBoundaryEvent: function(window, name, charIndex, charLength) {
+    let detail = { type: "boundary", args: { name, charIndex, charLength } };
     window.dispatchEvent(new window.CustomEvent("testsynthevent",
       { detail: detail }));
   },
 
   isWordHighlightGone: function(window, ok) {
     let $ = window.document.querySelector.bind(window.document);
     ok(!$(".narrate-word-highlight"), "No more word highlights exist");
   },
--- a/toolkit/components/narrate/test/browser_word_highlight.js
+++ b/toolkit/components/narrate/test/browser_word_highlight.js
@@ -34,22 +34,23 @@ add_task(function* testNarrate() {
     // Skip forward to first paragraph.
     let details;
     do {
       promiseEvent = ContentTaskUtils.waitForEvent(content, "paragraphstart");
       $(NarrateTestUtils.FORWARD).click();
       details = (yield promiseEvent).detail;
     } while (details.tag != "p");
 
-    let boundaryPat = /(\s+)\S/g;
+    let boundaryPat = /(\S+)/g;
     let position = { left: 0, top: 0 };
     let text = details.paragraph;
     for (let res = boundaryPat.exec(text); res; res = boundaryPat.exec(text)) {
       promiseEvent = ContentTaskUtils.waitForEvent(content, "wordhighlight");
-      NarrateTestUtils.sendBoundaryEvent(content, "word", res.index);
+      NarrateTestUtils.sendBoundaryEvent(content, "word",
+        res.index, res[0].length);
       let { start, end } = (yield promiseEvent).detail;
       let nodes = NarrateTestUtils.getWordHighlights(content);
       for (let node of nodes) {
         // Since this is English we can assume each word is to the right or
         // below the previous one.
         ok(node.left > position.left || node.top > position.top,
           "highlight position is moving");
         position = { left: node.left, top: node.top };