Backed out changeset cbde1c9e78d5 (bug 1318605) for failing test_reader_view.html and test_session_scroll_position.html on Android. r=backout
authorSebastian Hengst <archaeopteryx@coole-files.de>
Tue, 29 Nov 2016 18:25:46 +0100
changeset 324686 52a5ff34b3b36e789ed21c2665889b8833ec61b0
parent 324685 5e3fb7348212c811644fb8449304214e8d63587c
child 324687 12718a74c79f3b720f5a0cef171242ab2e10b8b8
push id24
push usermaklebus@msu.edu
push dateTue, 20 Dec 2016 03:11:33 +0000
reviewersbackout
bugs1318605
milestone53.0a1
backs outcbde1c9e78d55907934892db41c6d2d5ac32a5e5
Backed out changeset cbde1c9e78d5 (bug 1318605) for failing test_reader_view.html and test_session_scroll_position.html on Android. r=backout
toolkit/components/narrate/NarrateControls.jsm
toolkit/components/narrate/Narrator.jsm
toolkit/components/reader/AboutReader.jsm
--- a/toolkit/components/narrate/NarrateControls.jsm
+++ b/toolkit/components/narrate/NarrateControls.jsm
@@ -11,20 +11,19 @@ Cu.import("resource://gre/modules/narrat
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/AsyncPrefs.jsm");
 Cu.import("resource://gre/modules/TelemetryStopwatch.jsm");
 
 this.EXPORTED_SYMBOLS = ["NarrateControls"];
 
 var gStrings = Services.strings.createBundle("chrome://global/locale/narrate.properties");
 
-function NarrateControls(mm, win, languagePromise) {
+function NarrateControls(mm, win) {
   this._mm = mm;
   this._winRef = Cu.getWeakReference(win);
-  this._languagePromise = languagePromise;
 
   win.addEventListener("unload", this);
 
   // Append content style sheet in document head
   let style = win.document.createElement("link");
   style.rel = "stylesheet";
   style.href = "chrome://global/skin/narrate.css";
   win.document.head.appendChild(style);
@@ -106,17 +105,17 @@ function NarrateControls(mm, win, langua
       <div id="narrate-rate" class="narrate-row">
         <input id="narrate-rate-input" value="0" title="${"speed"}"
                step="5" max="100" min="-100" type="range">
       </div>
       <div id="narrate-voices" class="narrate-row"></div>
       <div class="dropdown-arrow"></div>
     </li>`;
 
-  this.narrator = new Narrator(win, languagePromise);
+  this.narrator = new Narrator(win);
 
   let branch = Services.prefs.getBranch("narrate.");
   let selectLabel = gStrings.GetStringFromName("selectvoicelabel");
   this.voiceSelect = new VoiceSelect(win, selectLabel);
   this.voiceSelect.element.addEventListener("change", this);
   this.voiceSelect.element.id = "voice-select";
   win.speechSynthesis.addEventListener("voiceschanged", this);
   dropdown.querySelector("#narrate-voices").appendChild(
@@ -159,17 +158,17 @@ NarrateControls.prototype = {
         break;
     }
   },
 
   /**
    * Returns true if synth voices are available.
    */
   _setupVoices: function() {
-    return this._languagePromise.then(language => {
+    return this.narrator.languagePromise.then(language => {
       this.voiceSelect.clear();
       let win = this._win;
       let voicePrefs = this._getVoicePref();
       let selectedVoice = voicePrefs[language || "default"];
       let comparer = win.Intl ?
         (new Intl.Collator()).compare : (a, b) => a.localeCompare(b);
       let filter = !Services.prefs.getBoolPref("narrate.filter-voices");
       let options = win.speechSynthesis.getVoices().filter(v => {
@@ -223,17 +222,17 @@ NarrateControls.prototype = {
   _onRateInput: function(evt) {
     AsyncPrefs.set("narrate.rate", parseInt(evt.target.value, 10));
     this.narrator.setRate(this._convertRate(evt.target.value));
   },
 
   _onVoiceChange: function() {
     let voice = this.voice;
     this.narrator.setVoice(voice);
-    this._languagePromise.then(language => {
+    this.narrator.languagePromise.then(language => {
       if (language) {
         let voicePref = this._getVoicePref();
         voicePref[language || "default"] = voice;
         AsyncPrefs.set("narrate.voice", JSON.stringify(voicePref));
       }
     });
   },
 
--- a/toolkit/components/narrate/Narrator.jsm
+++ b/toolkit/components/narrate/Narrator.jsm
@@ -3,37 +3,55 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const { interfaces: Ci, utils: Cu } = Components;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
+XPCOMUtils.defineLazyModuleGetter(this, "LanguageDetector",
+  "resource:///modules/translation/LanguageDetector.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Services",
   "resource://gre/modules/Services.jsm");
 
 this.EXPORTED_SYMBOLS = [ "Narrator" ];
 
 // Maximum time into paragraph when pressing "skip previous" will go
 // to previous paragraph and not the start of current one.
 const PREV_THRESHOLD = 2000;
 // All text-related style rules that we should copy over to the highlight node.
 const kTextStylesRules = ["font-family", "font-kerning", "font-size",
   "font-size-adjust", "font-stretch", "font-variant", "font-weight",
   "line-height", "letter-spacing", "text-orientation",
   "text-transform", "word-spacing"];
 
-function Narrator(win, languagePromise) {
+function Narrator(win) {
   this._winRef = Cu.getWeakReference(win);
-  this._languagePromise = languagePromise;
   this._inTest = Services.prefs.getBoolPref("narrate.test");
   this._speechOptions = {};
   this._startTime = 0;
   this._stopped = false;
+
+  this.languagePromise = new Promise(resolve => {
+    let detect = () => {
+      win.document.removeEventListener("AboutReaderContentReady", detect);
+      let sampleText = this._doc.getElementById(
+        "moz-reader-content").textContent.substring(0, 60 * 1024);
+      LanguageDetector.detectLanguage(sampleText).then(result => {
+        resolve(result.confident ? result.language : null);
+      });
+    };
+
+    if (win.document.body.classList.contains("loaded")) {
+      detect();
+    } else {
+      win.document.addEventListener("AboutReaderContentReady", detect);
+    }
+  });
 }
 
 Narrator.prototype = {
   get _doc() {
     return this._winRef.get().document;
   },
 
   get _win() {
@@ -243,17 +261,17 @@ Narrator.prototype = {
 
   start: function(speechOptions) {
     this._speechOptions = {
       rate: speechOptions.rate,
       voice: this._getVoice(speechOptions.voice)
     };
 
     this._stopped = false;
-    return this._languagePromise.then(language => {
+    return this.languagePromise.then(language => {
       if (!this._speechOptions.voice) {
         this._speechOptions.lang = language;
       }
 
       let tw = this._treeWalker;
       if (!this._isParagraphInView(tw.currentNode)) {
         tw.currentNode = tw.root;
         while (tw.nextNode()) {
--- a/toolkit/components/reader/AboutReader.jsm
+++ b/toolkit/components/reader/AboutReader.jsm
@@ -12,17 +12,16 @@ Cu.import("resource://gre/modules/Reader
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "AsyncPrefs", "resource://gre/modules/AsyncPrefs.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "NarrateControls", "resource://gre/modules/narrate/NarrateControls.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Rect", "resource://gre/modules/Geometry.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Task", "resource://gre/modules/Task.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "UITelemetry", "resource://gre/modules/UITelemetry.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "LanguageDetector", "resource:///modules/translation/LanguageDetector.jsm");
 
 var gStrings = Services.strings.createBundle("chrome://global/locale/aboutReader.properties");
 
 var AboutReader = function(mm, win, articlePromise) {
   let url = this._getOriginalUrl(win);
   if (!(url.startsWith("http://") || url.startsWith("https://"))) {
     let errorMsg = "Only http:// and https:// URLs can be loaded in about:reader.";
     if (Services.prefs.getBoolPref("reader.errors.includeURLs"))
@@ -41,19 +40,16 @@ var AboutReader = function(mm, win, arti
   this._mm.addMessageListener("Reader:GetStoredArticleData", this);
 
   this._docRef = Cu.getWeakReference(doc);
   this._winRef = Cu.getWeakReference(win);
   this._innerWindowId = win.QueryInterface(Ci.nsIInterfaceRequestor)
     .getInterface(Ci.nsIDOMWindowUtils).currentInnerWindowID;
 
   this._article = null;
-  this._languagePromise = new Promise(resolve => {
-    this._foundLanguage = resolve;
-  });
 
   if (articlePromise) {
     this._articlePromise = articlePromise;
   }
 
   this._headerElementRef = Cu.getWeakReference(doc.getElementById("reader-header"));
   this._domainElementRef = Cu.getWeakReference(doc.getElementById("reader-domain"));
   this._titleElementRef = Cu.getWeakReference(doc.getElementById("reader-title"));
@@ -113,17 +109,17 @@ var AboutReader = function(mm, win, arti
 
   this._setupFontSizeButtons();
 
   this._setupContentWidthButtons();
 
   this._setupLineHeightButtons();
 
   if (win.speechSynthesis && Services.prefs.getBoolPref("narrate.enabled")) {
-    new NarrateControls(mm, win, this._languagePromise);
+    new NarrateControls(mm, win);
   }
 
   this._loadArticle();
 };
 
 AboutReader.prototype = {
   _BLOCK_IMAGES_SELECTOR: ".content p > img:only-child, " +
                           ".content p > a:only-child > img:only-child, " +
@@ -712,29 +708,22 @@ AboutReader.prototype = {
         img.onload = function() {
           setImageMargins(img);
         };
       }
     }
   },
 
   _maybeSetTextDirection: function Read_maybeSetTextDirection(article) {
-    if (article.dir) {
-      // Set "dir" attribute on content
-      this._contentElement.setAttribute("dir", article.dir);
-      this._headerElement.setAttribute("dir", article.dir);
-    } else {
-      this._languagePromise.then(language => {
-        // TODO: Remove the hardcoded language codes below once bug 1320265 is resolved.
-        if (["ar", "fa", "he", "ug", "ur"].includes(language)) {
-          this._contentElement.setAttribute("dir", "rtl");
-          this._headerElement.setAttribute("dir", "rtl");
-        }
-      });
-    }
+    if (!article.dir)
+      return;
+
+    // Set "dir" attribute on content
+    this._contentElement.setAttribute("dir", article.dir);
+    this._headerElement.setAttribute("dir", article.dir);
   },
 
   _fixLocalLinks() {
     // We need to do this because preprocessing the content through nsIParserUtils
     // gives back a DOM with a <base> element. That influences how these URLs get
     // resolved, making them no longer match the document URI (which is
     // about:reader?url=...). To fix this, make all the hash URIs absolute. This
     // is hacky, but the alternative of removing the base element has potential
@@ -794,39 +783,32 @@ AboutReader.prototype = {
 
     let parserUtils = Cc["@mozilla.org/parserutils;1"].getService(Ci.nsIParserUtils);
     let contentFragment = parserUtils.parseFragment(article.content,
       Ci.nsIParserUtils.SanitizerDropForms | Ci.nsIParserUtils.SanitizerAllowStyle,
       false, articleUri, this._contentElement);
     this._contentElement.innerHTML = "";
     this._contentElement.appendChild(contentFragment);
     this._fixLocalLinks();
-    this._findLanguage(article.textContent);
     this._maybeSetTextDirection(article);
 
     this._contentElement.style.display = "block";
     this._updateImageMargins();
 
     this._requestFavicon();
     this._doc.body.classList.add("loaded");
 
     this._goToReference(articleUri.ref);
 
     Services.obs.notifyObservers(this._win, "AboutReader:Ready", "");
 
     this._doc.dispatchEvent(
       new this._win.CustomEvent("AboutReaderContentReady", { bubbles: true, cancelable: false }));
   },
 
-  _findLanguage: function(textContent) {
-    LanguageDetector.detectLanguage(textContent).then(result => {
-      this._foundLanguage(result.confident ? result.language : null);
-    });
-  },
-
   _hideContent: function() {
     this._headerElement.style.display = "none";
     this._contentElement.style.display = "none";
   },
 
   _showProgressDelayed: function() {
     this._win.setTimeout(function() {
       // No need to show progress if the article has been loaded,