Backed out changeset 9a03728016ff (bug 1265304) for failing browser_narrate.js. r=backout

toolkit/components/reader/AboutReader.jsm

@@ -17,8 +17,7 @@ XPCOMUtils.defineLazyModuleGetter(this, "NarrateControls", "resource://gre/modul
 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.defineLazyServiceGetter(this, "gChromeRegistry",
+XPCOMUtils.defineLazyModuleGetter(this, "LanguageDetector", "resource:///modules/translation/LanguageDetector.jsm");
-                                   "@mozilla.org/chrome/chrome-registry;1", Ci.nsIXULChromeRegistry);
 
 var gStrings = Services.strings.createBundle("chrome://global/locale/aboutReader.properties");
 
@@ -60,7 +59,6 @@ var AboutReader = function(mm, win, 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"));
-  this._readTimeElementRef = Cu.getWeakReference(doc.getElementById("reader-estimated-time"));
   this._creditsElementRef = Cu.getWeakReference(doc.getElementById("reader-credits"));
   this._contentElementRef = Cu.getWeakReference(doc.getElementById("moz-reader-content"));
   this._toolbarElementRef = Cu.getWeakReference(doc.getElementById("reader-toolbar"));
@@ -153,10 +151,6 @@ AboutReader.prototype = {
     return this._titleElementRef.get();
   },
 
-  get _readTimeElement() {
-    return this._readTimeElementRef.get();
-  },
-
   get _creditsElement() {
     return this._creditsElementRef.get();
   },
@@ -728,11 +722,14 @@ AboutReader.prototype = {
       // Set "dir" attribute on content
       this._contentElement.setAttribute("dir", article.dir);
       this._headerElement.setAttribute("dir", article.dir);
-
+    } else {
-      // The native locale could be set differently than the article's text direction.
+      this._languagePromise.then(language => {
-      var localeDirection = gChromeRegistry.isLocaleRTL("global") ? "rtl" : "ltr";
+        // TODO: Remove the hardcoded language codes below once bug 1320265 is resolved.
-      this._readTimeElement.setAttribute("dir", localeDirection);
+        if (["ar", "fa", "he", "ug", "ur"].includes(language)) {
-      this._readTimeElement.style.textAlign = article.dir == "rtl" ? "right" : "left";
+          this._contentElement.setAttribute("dir", "rtl");
+          this._headerElement.setAttribute("dir", "rtl");
+        }
+      });
     }
   },
 
@@ -751,14 +748,6 @@ AboutReader.prototype = {
     }
   },
 
-  _formatReadTime(slowEstimate, fastEstimate) {
-    if (slowEstimate == fastEstimate) {
-      return gStrings.formatStringFromName("aboutReader.estimatedReadTimeValue", [slowEstimate], 1);
-    }
-
-    return gStrings.formatStringFromName("aboutReader.estimatedReadTimeRange", [fastEstimate, slowEstimate], 2);
-  },
-
   _showError: function() {
     this._headerElement.style.display = "none";
     this._contentElement.style.display = "none";
@@ -800,7 +789,6 @@ AboutReader.prototype = {
     this._creditsElement.textContent = article.byline;
 
     this._titleElement.textContent = article.title;
-    this._readTimeElement.textContent = this._formatReadTime(article.readingTimeMinsSlow, article.readingTimeMinsFast);
     this._doc.title = article.title;
 
     this._headerElement.style.display = "block";
@@ -812,6 +800,7 @@ AboutReader.prototype = {
     this._contentElement.innerHTML = "";
     this._contentElement.appendChild(contentFragment);
     this._fixLocalLinks();
+    this._findLanguage(article.textContent);
     this._maybeSetTextDirection(article);
 
     this._contentElement.style.display = "block";
@@ -828,6 +817,14 @@ AboutReader.prototype = {
       new this._win.CustomEvent("AboutReaderContentReady", { bubbles: true, cancelable: false }));
   },
 
+  _findLanguage: function(textContent) {
+    if (gIsFirefoxDesktop) {
+      LanguageDetector.detectLanguage(textContent).then(result => {
+        this._foundLanguage(result.confident ? result.language : null);
+      });
+    }
+  },
+
   _hideContent: function() {
     this._headerElement.style.display = "none";
     this._contentElement.style.display = "none";

toolkit/components/reader/ReaderMode.jsm

@@ -29,7 +29,6 @@ XPCOMUtils.defineLazyModuleGetter(this, "OS", "resource://gre/modules/osfile.jsm
 XPCOMUtils.defineLazyModuleGetter(this, "ReaderWorker", "resource://gre/modules/reader/ReaderWorker.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Task", "resource://gre/modules/Task.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "TelemetryStopwatch", "resource://gre/modules/TelemetryStopwatch.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "LanguageDetector", "resource:///modules/translation/LanguageDetector.jsm");
 
 XPCOMUtils.defineLazyGetter(this, "Readability", function() {
   let scope = {};
@@ -38,8 +37,6 @@ XPCOMUtils.defineLazyGetter(this, "Readability", function() {
   return scope["Readability"];
 });
 
-const gIsFirefoxDesktop = Services.appinfo.ID == "{ec8030f7-c20a-464f-9b0e-13a3a9e97384}";
-
 this.ReaderMode = {
   // Version of the cache schema.
   CACHE_VERSION: 1,
@@ -464,12 +461,6 @@ this.ReaderMode = {
     let flags = Ci.nsIDocumentEncoder.OutputSelectionOnly | Ci.nsIDocumentEncoder.OutputAbsoluteLinks;
     article.title = Cc["@mozilla.org/parserutils;1"].getService(Ci.nsIParserUtils)
                                                     .convertToPlainText(article.title, flags, 0);
-    if (gIsFirefoxDesktop) {
-      yield this._findLanguage(article.textContent);
-      this._maybeAssignTextDirection(article);
-    }
-
-    this._assignReadTime(article);
 
     histogram.add(PARSE_SUCCESS);
     return article;
@@ -519,73 +510,5 @@ this.ReaderMode = {
       }
       return undefined;
     });
-  },
+  }
-
-  /**
-   * Sets a global language string value if the result is confident
-   *
-   * @return Promise
-   * @resolves when the language is detected
-   */
-  _findLanguage(textContent) {
-    return LanguageDetector.detectLanguage(textContent).then(result => {
-      this._foundLanguage = result.confident ? result.language : null;
-    });
-  },
-
-  _maybeAssignTextDirection(article) {
-    // TODO: Remove the hardcoded language codes below once bug 1320265 is resolved.
-    if (!article.dir && ["ar", "fa", "he", "ug", "ur"].includes(this._foundLanguage)) {
-      article.dir = "rtl";
-    }
-  },
-
-  /**
-   * Assigns the estimated reading time range of the article to the article object.
-   *
-   * @param article the article object to assign the reading time estimate to.
-   */
-  _assignReadTime(article) {
-    let lang = this._foundLanguage || "en";
-    const readingSpeed = this._getReadingSpeedForLanguage(lang);
-    const charactersPerMinuteLow = readingSpeed.cpm - readingSpeed.variance;
-    const charactersPerMinuteHigh = readingSpeed.cpm + readingSpeed.variance;
-    const length = article.length;
-
-    article.readingTimeMinsSlow = Math.ceil(length / charactersPerMinuteLow);
-    article.readingTimeMinsFast  = Math.ceil(length / charactersPerMinuteHigh);
-  },
-
-  /**
-   * Returns the reading speed of a selection of languages with likely variance.
-   *
-   * Reading speed estimated from a study done on reading speeds in various languages.
-   * study can be found here: http://iovs.arvojournals.org/article.aspx?articleid=2166061
-   *
-   * @return object with characters per minute and variance. Defaults to English
-   *         if no suitable language is found in the collection.
-   */
-  _getReadingSpeedForLanguage(lang) {
-    const readingSpeed = new Map([
-      [ "en", {cpm: 987,  variance: 118 } ],
-      [ "ar", {cpm: 612,  variance: 88 } ],
-      [ "de", {cpm: 920,  variance: 86 } ],
-      [ "es", {cpm: 1025, variance: 127 } ],
-      [ "fi", {cpm: 1078, variance: 121 } ],
-      [ "fr", {cpm: 998,  variance: 126 } ],
-      [ "he", {cpm: 833,  variance: 130 } ],
-      [ "it", {cpm: 950,  variance: 140 } ],
-      [ "jw", {cpm: 357,  variance: 56 } ],
-      [ "nl", {cpm: 978,  variance: 143 } ],
-      [ "pl", {cpm: 916,  variance: 126 } ],
-      [ "pt", {cpm: 913,  variance: 145 } ],
-      [ "ru", {cpm: 986,  variance: 175 } ],
-      [ "sk", {cpm: 885,  variance: 145 } ],
-      [ "sv", {cpm: 917,  variance: 156 } ],
-      [ "tr", {cpm: 1054, variance: 156 } ],
-      [ "zh", {cpm: 255,  variance: 29 } ],
-    ]);
-
-    return readingSpeed.get(lang) || readingSpeed.get("en");
-  },
 };

toolkit/components/reader/content/aboutReader.html

@@ -20,13 +20,8 @@
       <div class="domain-border"></div>
       <h1 id="reader-title"></h1>
       <div id="reader-credits" class="credits"></div>
-      <div id="meta-data" class="meta-data">
-        <div id="reader-estimated-time"></div>
-      </div>
     </div>
 
-    <hr>
-
     <div class="content">
       <style scoped>
         @import url("chrome://global/skin/aboutReaderContent.css");
@@ -51,23 +46,23 @@
       <li><button class="dropdown-toggle button style-button"/></li>
       <li id="reader-popup" class="dropdown-popup">
         <div id="font-type-buttons"></div>
-        <hr>
+        <hr></hr>
         <div id="font-size-buttons">
           <button id="font-size-minus" class="minus-button"/>
           <button id="font-size-sample"/>
           <button id="font-size-plus" class="plus-button"/>
         </div>
-        <hr>
+        <hr></hr>
         <div id="content-width-buttons">
           <button id="content-width-minus" class="content-width-minus-button"/>
           <button id="content-width-plus" class="content-width-plus-button"/>
         </div>
-        <hr>
+        <hr></hr>
         <div id="line-height-buttons">
           <button id="line-height-minus" class="line-height-minus-button"/>
           <button id="line-height-plus" class="line-height-plus-button"/>
         </div>
-        <hr>
+        <hr></hr>
         <div id="color-scheme-buttons"></div>
         <div class="dropdown-arrow"/>
       </li>

toolkit/components/reader/test/browser.ini

@@ -13,7 +13,3 @@ support-files =
 [browser_bug1124271_readerModePinnedTab.js]
 support-files =
   readerModeArticle.html
-[browser_readerMode_readingTime.js]
-support-files =
-  readerModeArticle.html
-  readerModeArticleShort.html

toolkit/components/reader/test/browser_readerMode_readingTime.js

@@ -1,45 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-"use strict";
-
-const TEST_PATH = getRootDirectory(gTestPath).replace("chrome://mochitests/content", "http://example.com");
-
-/**
- * Test that the reader mode correctly calculates and displays the
- * estimated reading time for a normal length article
- */
-add_task(function* () {
-  yield BrowserTestUtils.withNewTab(TEST_PATH + "readerModeArticle.html", function* (browser) {
-    let pageShownPromise = BrowserTestUtils.waitForContentEvent(browser, "AboutReaderContentReady");
-    let readerButton = document.getElementById("reader-mode-button");
-    readerButton.click();
-    yield pageShownPromise;
-    yield ContentTask.spawn(browser, null, function* () {
-      // make sure there is a reading time on the page and that it displays the correct information
-      let readingTimeElement = content.document.getElementById("reader-estimated-time");
-      ok(readingTimeElement, "Reading time element should be in document");
-      is(readingTimeElement.textContent, "9-12 min", "Reading time should be '9-12 min'");
-    });
-  });
-});
-
-/**
- * Test that the reader mode correctly calculates and displays the
- * estimated reading time for a short article
- */
-add_task(function* () {
-  yield BrowserTestUtils.withNewTab(TEST_PATH + "readerModeArticleShort.html", function* (browser) {
-    let pageShownPromise = BrowserTestUtils.waitForContentEvent(browser, "AboutReaderContentReady");
-    let readerButton = document.getElementById("reader-mode-button");
-    readerButton.click();
-    yield pageShownPromise;
-    yield ContentTask.spawn(browser, null, function* () {
-      // make sure there is a reading time on the page and that it displays the correct information
-      let readingTimeElement = content.document.getElementById("reader-estimated-time");
-      ok(readingTimeElement, "Reading time element should be in document");
-      is(readingTimeElement.textContent, "1 min", "Reading time should be '1 min'");
-    });
-  });
-});

toolkit/components/reader/test/readerModeArticleShort.html

@@ -1,14 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<title>Article title</title>
-<meta name="description" content="This is the article description." />
-</head>
-<body>
-<header>Site header</header>
-<div>
-<h1>Article title</h1>
-<h2 class="author">by Jane Doe</h2>
-<p>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec a diam lectus. Sed sit amet ipsum mauris. Maecenas congue ligula ac quam viverra nec consectetur ante hendrerit. Donec et mollis dolor. Praesent et diam eget libero egestas mattis sit amet vitae augue. Nam tincidunt congue enim, ut porta lorem lacinia consectetur. Donec ut libero sed arcu vehicula ultricies a non tortor. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean ut gravida lorem. Ut turpis felis, pulvinar a semper sed, adipiscing id dolor. Pellentesque auctor nisi id magna consequat sagittis. Curabitur dapibus enim sit amet elit pharetra tincidunt feugiat nisl imperdiet. Ut convallis libero in urna ultrices accumsan. Donec sed odio eros. Donec viverra mi quis quam pulvinar at malesuada arcu rhoncus. Cum sociis natoque penatibus et magnis dis parturient montes, nascetu</p>
-</body>
-</html>

toolkit/locales/en-US/chrome/global/aboutReader.properties

@@ -13,9 +13,6 @@ aboutReader.colorScheme.dark=Dark
 aboutReader.colorScheme.sepia=Sepia
 aboutReader.colorScheme.auto=Auto
 
-aboutReader.estimatedReadTimeValue=%S min
-aboutReader.estimatedReadTimeRange=%S-%S min
-
 # LOCALIZATION NOTE (aboutReader.fontType.serif, aboutReader.fontType.sans-serif):
 # These are the styles of typeface that are options in the reader view controls.
 aboutReader.fontType.serif=Serif

toolkit/themes/shared/aboutReaderControls.css

@@ -63,16 +63,11 @@
 .header > .credits {
   font-size: 0.9em;
   line-height: 1.48em;
-  margin: 0 0 10px 0;
+  margin: 0 0 30px 0;
   padding: 0;
   font-style: italic;
 }
 
-.header > .meta-data {
-  font-size: 0.65em;
-  margin: 0 0 15px 0;
-}
-
 /*======= Controls toolbar =======*/
 
 .toolbar {