Bug 1629030 - don't close an open popup when the font changes in reader mode (due to spurious scroll events), r=jaws
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Fri, 22 May 2020 19:57:58 +0000
changeset 531707 63a158b481ea7a28c812ccc5d29bdbe5fc1812bf
parent 531706 82160fd55e1fa8106c636657cfe4ddeb16864ad4
child 531708 ae43921b1fe66df9bc8059b97232a030fc53c687
push id37442
push userncsoregi@mozilla.com
push dateSat, 23 May 2020 09:21:24 +0000
treeherdermozilla-central@bbcc193fe0f0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs1629030
milestone78.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1629030 - don't close an open popup when the font changes in reader mode (due to spurious scroll events), r=jaws This problem only happens if you're scrolled down on the page and then change font configuration settings (like line height or font size) that cause the scroll position to change. The scroll event fires and we hide popups in response. To fix this, we keep a cached height of the body, so that if the page height has changed since the last scroll event, we don't immediately close the popup. Differential Revision: https://phabricator.services.mozilla.com/D76311
toolkit/components/reader/AboutReader.jsm
toolkit/components/reader/content/aboutReader.html
toolkit/themes/shared/aboutReader.css
--- a/toolkit/components/reader/AboutReader.jsm
+++ b/toolkit/components/reader/AboutReader.jsm
@@ -104,27 +104,31 @@ var AboutReader = function(actor, articl
   );
   this._messageElementRef = Cu.getWeakReference(
     doc.querySelector(".reader-message")
   );
   this._containerElementRef = Cu.getWeakReference(
     doc.querySelector(".container")
   );
 
-  this._scrollOffset = win.pageYOffset;
-
   doc.addEventListener("mousedown", this);
   doc.addEventListener("click", this);
   doc.addEventListener("touchstart", this);
 
   win.addEventListener("pagehide", this);
-  win.addEventListener("mozvisualscroll", this, { mozSystemGroup: true });
   win.addEventListener("resize", this);
   win.addEventListener("wheel", this, { passive: false });
 
+  this._topScrollChange = this._topScrollChange.bind(this);
+  this._intersectionObs = new win.IntersectionObserver(this._topScrollChange, {
+    root: null,
+    threshold: [0, 1],
+  });
+  this._intersectionObs.observe(doc.querySelector(".top-anchor"));
+
   Services.obs.addObserver(this, "inner-window-destroyed");
 
   doc.addEventListener("visibilitychange", this);
 
   this._setupStyleDropdown();
   this._setupButton(
     "close-button",
     this._onReaderClose.bind(this),
@@ -377,36 +381,28 @@ AboutReader.prototype = {
           this._closeDropdowns();
         }
         break;
       case "click":
         if (target.classList.contains("dropdown-toggle")) {
           this._toggleDropdownClicked(aEvent);
         }
         break;
-      case "mozvisualscroll":
-        const vv = aEvent.originalTarget; // VisualViewport
-        let tbc = this._toolbarContainerElement;
-
-        if (gIsFirefoxDesktop) {
+      case "scroll":
+        let lastHeight = this._lastHeight;
+        let { windowUtils } = this._win;
+        this._lastHeight = windowUtils.getBoundsWithoutFlushing(
+          this._doc.body
+        ).height;
+        // Only close dropdowns if the scroll events are not a result of line
+        // height / font-size changes that caused a page height change.
+        if (lastHeight == this._lastHeight) {
           this._closeDropdowns(true);
-          tbc.classList.toggle("scrolled", vv.pageTop > 0);
-        } else if (this._scrollOffset != vv.pageTop) {
-          // hide the system UI and the "reader-toolbar" only if the dropdown is not opened
-          let selector = ".dropdown.open";
-          let openDropdowns = this._doc.querySelectorAll(selector);
-          if (openDropdowns.length) {
-            break;
-          }
-
-          let isScrollingUp = this._scrollOffset > vv.pageTop;
-          this._setToolbarVisibility(isScrollingUp);
         }
 
-        this._scrollOffset = vv.pageTop;
         break;
       case "resize":
         this._updateImageMargins();
         break;
 
       case "wheel":
         let doZoom =
           (aEvent.ctrlKey && zoomOnCtrl) || (aEvent.metaKey && zoomOnMeta);
@@ -441,21 +437,22 @@ AboutReader.prototype = {
         this._handleDeviceLight(aEvent.value);
         break;
 
       case "visibilitychange":
         this._handleVisibilityChange();
         break;
 
       case "pagehide":
-        // Close the Banners Font-dropdown, cleanup Android BackPressListener.
         this._closeDropdowns();
 
         this._actor.readerModeHidden();
         this.clearActor();
+        this._intersectionObs.unobserve(this._doc.querySelector(".top-anchor"));
+        delete this._intersectionObs;
         break;
     }
   },
 
   clearActor() {
     this._actor = null;
   },
 
@@ -835,38 +832,16 @@ AboutReader.prototype = {
     }
 
     this._fontType = newFontType;
     bodyClasses.add(this._fontType);
 
     AsyncPrefs.set("reader.font_type", this._fontType);
   },
 
-  _setToolbarVisibility(visible) {
-    let tb = this._toolbarElement;
-
-    if (visible) {
-      if (tb.style.opacity != "1") {
-        tb.removeAttribute("hidden");
-        tb.style.opacity = "1";
-      }
-    } else if (tb.style.opacity != "0") {
-      tb.addEventListener(
-        "transitionend",
-        evt => {
-          if (tb.style.opacity == "0") {
-            tb.setAttribute("hidden", "");
-          }
-        },
-        { once: true }
-      );
-      tb.style.opacity = "0";
-    }
-  },
-
   async _loadArticle() {
     let url = this._getOriginalUrl();
     this._showProgressDelayed();
 
     let article;
     if (this._articlePromise) {
       article = await this._articlePromise;
     } else {
@@ -1249,19 +1224,24 @@ AboutReader.prototype = {
    */
   _openDropdown(dropdown, window) {
     if (dropdown.classList.contains("open")) {
       return;
     }
 
     this._closeDropdowns();
 
-    // Trigger BackPressListener initialization in Android.
+    // Get the height of the doc and start handling scrolling:
+    let { windowUtils } = this._win;
+    this._lastHeight = windowUtils.getBoundsWithoutFlushing(
+      this._doc.body
+    ).height;
+    this._doc.addEventListener("scroll", this);
+
     dropdown.classList.add("open");
-    let { windowUtils } = this._winRef.get();
     let toggle = dropdown.querySelector(".dropdown-toggle");
     let anchorWidth = windowUtils.getBoundsWithoutFlushing(toggle).width;
     dropdown.style.setProperty("--popup-anchor-width", anchorWidth + "px");
   },
 
   /*
    * If the ReaderView has open dropdowns, close them. If we are closing the
    * dropdowns because the page is scrolling, allow popups to stay open with
@@ -1272,16 +1252,30 @@ AboutReader.prototype = {
     if (scrolling) {
       selector += ":not(.keep-open)";
     }
 
     let openDropdowns = this._doc.querySelectorAll(selector);
     for (let dropdown of openDropdowns) {
       dropdown.classList.remove("open");
     }
+
+    // Stop handling scrolling:
+    this._doc.removeEventListener("scroll", this);
+  },
+
+  _topScrollChange(entries) {
+    if (!entries.length) {
+      return;
+    }
+    // If we don't intersect the item at the top of the document, we're
+    // scrolled down:
+    let scrolled = !entries[entries.length - 1].isIntersecting;
+    let tbc = this._toolbarContainerElement;
+    tbc.classList.toggle("scrolled", scrolled);
   },
 
   /*
    * Scroll reader view to a reference
    */
   _goToReference(ref) {
     if (ref) {
       this._win.location.hash = ref;
--- a/toolkit/components/reader/content/aboutReader.html
+++ b/toolkit/components/reader/content/aboutReader.html
@@ -8,16 +8,17 @@
 <head>
   <meta http-equiv="Content-Security-Policy" content="default-src chrome:; img-src data: *; media-src *; object-src 'none'" />
   <meta content="text/html; charset=UTF-8" http-equiv="content-type" />
   <meta name="viewport" content="width=device-width; user-scalable=0" />
   <link rel="stylesheet" href="chrome://global/skin/aboutReader.css" type="text/css"/>
 </head>
 
 <body>
+  <div class="top-anchor"></div>
   <div class="container">
     <div class="header reader-header">
       <a class="domain reader-domain"></a>
       <div class="domain-border"></div>
       <h1 class="reader-title"></h1>
       <div class="credits reader-credits"></div>
       <div class="meta-data">
         <div class="reader-estimated-time"></div>
--- a/toolkit/themes/shared/aboutReader.css
+++ b/toolkit/themes/shared/aboutReader.css
@@ -182,16 +182,25 @@ body:not(.loaded) .toolbar-container {
 .reader-message {
   margin-top: 40px;
   display: none;
   text-align: center;
   width: 100%;
   font-size: 0.9em;
 }
 
+/* Detector element to see if we're at the top of the doc or not. */
+.top-anchor {
+  position: absolute;
+  top: 0;
+  width: 0;
+  height: 5px;
+  pointer-events: none;
+}
+
 /* Header */
 
 .header {
   text-align: start;
   display: none;
 }
 
 .domain {