Merge pull request #12464 from baloone/Fix_getVisibleElements_in_rtl_…

…direction Fix getVisibleElements helper in RTL-locales
mozilla · Oct 24, 2020 · 0d1a874 · 0d1a874
2 parents e8e029d + b7b048e
commit 0d1a874
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 11 deletions.
diff --git a/test/unit/ui_utils_spec.js b/test/unit/ui_utils_spec.js
@@ -639,20 +639,20 @@ describe("ui_utils", function () {
     // This function takes a fixed layout of pages and compares the system under
     // test to the slower implementation above, for a range of scroll viewport
     // sizes and positions.
-    function scrollOverDocument(pages, horizontally = false) {
+    function scrollOverDocument(pages, horizontally = false, rtl = false) {
       const size = pages.reduce(function (max, { div }) {
         return Math.max(
           max,
           horizontally
-            ? div.offsetLeft + div.clientLeft + div.clientWidth
+            ? Math.abs(div.offsetLeft + div.clientLeft + div.clientWidth)
             : div.offsetTop + div.clientTop + div.clientHeight
         );
       }, 0);
       // The numbers (7 and 5) are mostly arbitrary, not magic: increase them to
       // make scrollOverDocument tests faster, decrease them to make the tests
       // more scrupulous, and keep them coprime to reduce the chance of missing
       // weird edge case bugs.
-      for (let i = 0; i < size; i += 7) {
+      for (let i = -size; i < size; i += 7) {
         // The screen height (or width) here (j - i) doubles on each inner loop
         // iteration; again, this is just to test an interesting range of cases
         // without slowing the tests down to check every possible case.
@@ -671,7 +671,7 @@ describe("ui_utils", function () {
                 clientWidth: 10000,
               };
           expect(
-            getVisibleElements(scroll, pages, false, horizontally)
+            getVisibleElements(scroll, pages, false, horizontally, rtl)
           ).toEqual(slowGetVisibleElements(scroll, pages));
         }
       }
@@ -737,6 +737,17 @@ describe("ui_utils", function () {
       scrollOverDocument(pages, true);
     });
 
+    it("works with horizontal scrolling with RTL-documents", function () {
+      const pages = makePages([
+        [
+          [-10, 50],
+          [-20, 20],
+          [-30, 10],
+        ],
+      ]);
+      scrollOverDocument(pages, true, true);
+    });
+
     it("handles `sortByVisibility` correctly", function () {
       const scrollEl = {
         scrollTop: 75,

diff --git a/web/base_viewer.js b/web/base_viewer.js
@@ -1006,6 +1006,10 @@ class BaseViewer {
       : this._scrollMode === ScrollMode.HORIZONTAL;
   }
 
+  get _isContainerRtl() {
+    return getComputedStyle(this.container).direction === "rtl";
+  }
+
   get isInPresentationMode() {
     return this.presentationModeState === PresentationModeState.FULLSCREEN;
   }
@@ -1055,7 +1059,8 @@ class BaseViewer {
       this.container,
       this._pages,
       true,
-      this._isScrollModeHorizontal
+      this._isScrollModeHorizontal,
+      this._isScrollModeHorizontal && this._isContainerRtl
     );
   }
 

diff --git a/web/ui_utils.js b/web/ui_utils.js
@@ -442,7 +442,8 @@ function getVisibleElements(
   scrollEl,
   views,
   sortByVisibility = false,
-  horizontal = false
+  horizontal = false,
+  rtl = false
 ) {
   const top = scrollEl.scrollTop,
     bottom = top + scrollEl.clientHeight;
@@ -465,11 +466,11 @@ function getVisibleElements(
       element.offsetTop + element.clientTop + element.clientHeight;
     return elementBottom > top;
   }
-  function isElementRightAfterViewLeft(view) {
+  function isElementNextAfterViewHorizontally(view) {
     const element = view.div;
-    const elementRight =
-      element.offsetLeft + element.clientLeft + element.clientWidth;
-    return elementRight > left;
+    const elementLeft = element.offsetLeft + element.clientLeft;
+    const elementRight = elementLeft + element.clientWidth;
+    return rtl ? elementLeft < right : elementRight > left;
   }
 
   const visible = [],
@@ -479,7 +480,9 @@ function getVisibleElements(
       ? 0
       : binarySearchFirstItem(
           views,
-          horizontal ? isElementRightAfterViewLeft : isElementBottomAfterViewTop
+          horizontal
+            ? isElementNextAfterViewHorizontally
+            : isElementBottomAfterViewTop
         );
 
   // Please note the return value of the `binarySearchFirstItem` function when