From 6a99e688f7c9dcf36e131162c25e3cd91aca6dc4 Mon Sep 17 00:00:00 2001
From: Shin'ya Ueoka <ueokande@i-beam.org>
Date: Sat, 10 Feb 2024 08:29:05 +0000
Subject: [PATCH] fix: use checkVisibility() (#348)

The checkVisibility is a standard API to enable checking if the element
is visible.

https://developer.mozilla.org/en-US/docs/Web/API/Element/checkVisibility
---
 src/content/presenters/FocusPresenter.ts      |  8 +++--
 src/content/presenters/HintPresenter.ts       | 10 +++----
 src/content/presenters/ScrollPresenter.ts     |  8 +++--
 src/shared/utils/dom.ts                       | 29 +------------------
 test/content/presenters/HintPresenter.test.ts |  5 ++++
 test/main.ts                                  |  4 +++
 6 files changed, 27 insertions(+), 37 deletions(-)

diff --git a/src/content/presenters/FocusPresenter.ts b/src/content/presenters/FocusPresenter.ts
index 587c9c52..b208dcda 100644
--- a/src/content/presenters/FocusPresenter.ts
+++ b/src/content/presenters/FocusPresenter.ts
@@ -1,5 +1,4 @@
 import { injectable } from "inversify";
-import * as doms from "../../shared/utils/dom";
 
 export default interface FocusPresenter {
   focusFirstElement(): boolean;
@@ -15,7 +14,12 @@ export class FocusPresenterImpl implements FocusPresenter {
     const targets = window.document.querySelectorAll(
       inputSelector + ",input:not([type]),textarea",
     );
-    const target = Array.from(targets).find(doms.isVisible);
+    const target = Array.from(targets).find((e) =>
+      e.checkVisibility({
+        checkOpacity: true,
+        checkVisibilityCSS: true,
+      }),
+    );
     if (target instanceof HTMLInputElement) {
       target.focus();
       return true;
diff --git a/src/content/presenters/HintPresenter.ts b/src/content/presenters/HintPresenter.ts
index 9ca401e2..1314600b 100644
--- a/src/content/presenters/HintPresenter.ts
+++ b/src/content/presenters/HintPresenter.ts
@@ -161,13 +161,13 @@ export class HintPresenterImpl implements HintPresenter {
     viewSize: Size,
     framePosition: Point,
   ): boolean {
-    const style = window.getComputedStyle(element);
-
     // AREA's 'display' in Browser style is 'none'
     return (
-      (element.tagName === "AREA" || style.display !== "none") &&
-      style.visibility !== "hidden" &&
-      (element as HTMLInputElement).type !== "hidden" &&
+      element.checkVisibility({
+        checkOpacity: true,
+        checkVisibilityCSS: true,
+      }) &&
+      element.offsetWidth > 0 &&
       element.offsetHeight > 0 &&
       !isAriaHiddenOrAriaDisabled(window, element) &&
       inViewport(window, element, viewSize, framePosition)
diff --git a/src/content/presenters/ScrollPresenter.ts b/src/content/presenters/ScrollPresenter.ts
index ee939516..4038d247 100644
--- a/src/content/presenters/ScrollPresenter.ts
+++ b/src/content/presenters/ScrollPresenter.ts
@@ -1,5 +1,4 @@
 import { injectable } from "inversify";
-import * as doms from "../../shared/utils/dom";
 
 const SCROLL_DELTA_X = 64;
 const SCROLL_DELTA_Y = 64;
@@ -42,7 +41,12 @@ const findScrollable = (element: Element): Element | null => {
     return element;
   }
 
-  const children = Array.from(element.children).filter(doms.isVisible);
+  const children = Array.from(element.children).filter((e) =>
+    e.checkVisibility({
+      checkOpacity: true,
+      checkVisibilityCSS: true,
+    }),
+  );
   for (const child of children) {
     const scrollable = findScrollable(child);
     if (scrollable) {
diff --git a/src/shared/utils/dom.ts b/src/shared/utils/dom.ts
index 4d2082c9..e37e0db4 100644
--- a/src/shared/utils/dom.ts
+++ b/src/shared/utils/dom.ts
@@ -92,31 +92,4 @@ const viewportRect = (e: Element): Rect => {
   };
 };
 
-const isVisible = (element: Element): boolean => {
-  const rect = element.getBoundingClientRect();
-  const style = window.getComputedStyle(element);
-
-  if (style.overflow !== "visible" && (rect.width === 0 || rect.height === 0)) {
-    return false;
-  }
-  if (rect.right < 0 && rect.bottom < 0) {
-    return false;
-  }
-  if (window.innerWidth < rect.left && window.innerHeight < rect.top) {
-    return false;
-  }
-  if (
-    element instanceof HTMLInputElement &&
-    element.type.toLowerCase() === "hidden"
-  ) {
-    return false;
-  }
-
-  const { display, visibility } = window.getComputedStyle(element);
-  if (display === "none" || visibility === "hidden") {
-    return false;
-  }
-  return true;
-};
-
-export { isContentEditable, viewportRect, isVisible };
+export { isContentEditable, viewportRect };
diff --git a/test/content/presenters/HintPresenter.test.ts b/test/content/presenters/HintPresenter.test.ts
index b18f97f0..680acc4a 100644
--- a/test/content/presenters/HintPresenter.test.ts
+++ b/test/content/presenters/HintPresenter.test.ts
@@ -15,6 +15,11 @@ describe("HintPresenterImpl", () => {
 
     // HintPresenterImpl checks if the element is visible by the offsetHeight.
     Object.defineProperties(window.HTMLElement.prototype, {
+      offsetWidth: {
+        get() {
+          return 10;
+        },
+      },
       offsetHeight: {
         get() {
           return 10;
diff --git a/test/main.ts b/test/main.ts
index 55e9579c..5f391a82 100644
--- a/test/main.ts
+++ b/test/main.ts
@@ -53,3 +53,7 @@ global.chrome = {
     },
   },
 } as unknown as typeof chrome;
+
+if (global.Element) {
+  global.Element.prototype.checkVisibility = () => true;
+}