From 5a56c888494aae3a7b39a8905eeecb7e86e61ade Mon Sep 17 00:00:00 2001 From: Simon Lydell Date: Sun, 18 Aug 2024 00:21:13 +0200 Subject: [PATCH] Remove support for scrollable elements Firefox has sadly removed support for the `overflow` and `underflow` events which powered this feature: https://bugzilla.mozilla.org/show_bug.cgi?id=1888737 I don't know of any other way to implement it, so I removed it. --- docs/index.tsx | 1 - .../intersection-observer-cover.html | 1 - html/hint-position/index.html | 17 --- html/scroll/index.html | 117 ---------------- html/untrusted-events/index.html | 1 - src/background/Program.ts | 1 - src/renderer/Program.ts | 6 - src/shared/hints.ts | 1 - src/worker/ElementManager.ts | 127 +----------------- src/worker/Program.ts | 24 +--- 10 files changed, 10 insertions(+), 286 deletions(-) delete mode 100644 html/scroll/index.html diff --git a/docs/index.tsx b/docs/index.tsx index 8e579581..630ee1df 100644 --- a/docs/index.tsx +++ b/docs/index.tsx @@ -133,7 +133,6 @@ const SECTIONS = [ .
  • Tracking of elements with click listeners.
    • -
  • Tracking of scrollable elements in Firefox.
  • Tracking elements and updating hints while showing them.
  • Text based hint positioning.
  • diff --git a/html/experiments/intersection-observer-cover.html b/html/experiments/intersection-observer-cover.html index 40bf74ac..6b4c6204 100644 --- a/html/experiments/intersection-observer-cover.html +++ b/html/experiments/intersection-observer-cover.html @@ -61,7 +61,6 @@ observer.observe(cover); window.addEventListener("resize", updateCover); - window.addEventListener("underflow", updateCover); function updateCover() { const viewport = getViewport(); diff --git a/html/hint-position/index.html b/html/hint-position/index.html index 983f437f..33f3aa29 100644 --- a/html/hint-position/index.html +++ b/html/hint-position/index.html @@ -479,17 +479,6 @@ margin: 0; padding-bottom: 50px; } - - .scroll { - border: 1px solid black; - overflow: auto; - height: 100px; - width: 200px; - } - - .scroll > p { - margin-bottom: 200px; - } @@ -1065,12 +1054,6 @@

    Images taller than their link

    -

    Scrollable element

    -

    The hint should be placed at the edge, not at children text.

    -
    -

    Scrollable element

    -
    -

    Selectable elements

    diff --git a/html/scroll/index.html b/html/scroll/index.html deleted file mode 100644 index eca0ab16..00000000 --- a/html/scroll/index.html +++ /dev/null @@ -1,117 +0,0 @@ - - - - - scroll - - - - - -

    - Tests for scrollable and almost-seemingly scrollable elements. The boxes - are resizable so that switching between scrollable and unscrollable can be - tested. -

    - -
    -
    simple scroll
    -
    - -
    -
    horizontal scroll
    -
    - -
    -
    both scroll
    -
    - -
    -
    one scroll, one clipped
    -
    - -
    -
    overlay scroll
    -
    - -
    -
    clipped scroll
    -
    - -
    -
    no scroll but scrollbars
    -
    - -
    -
    overflow but no scrollbars
    -
    - -
    -
    - hidden overflow-x
    - auto overflow-y but no scrollbar -
    -
    - -
    -
    - auto overflow-x but no scrollbar
    - hidden overflow-y -
    -
    - - - - diff --git a/html/untrusted-events/index.html b/html/untrusted-events/index.html index c6273831..5631f367 100644 --- a/html/untrusted-events/index.html +++ b/html/untrusted-events/index.html @@ -77,7 +77,6 @@ "clickable-event", "label", "link", - "scrollable", "textarea", ], viewports: [ diff --git a/src/background/Program.ts b/src/background/Program.ts index b89447c4..0c1112ba 100644 --- a/src/background/Program.ts +++ b/src/background/Program.ts @@ -2369,7 +2369,6 @@ const CLICK_TYPES: ElementTypes = [ "clickable-event", "label", "link", - "scrollable", "textarea", ]; diff --git a/src/renderer/Program.ts b/src/renderer/Program.ts index 2695d540..3c6e85bd 100644 --- a/src/renderer/Program.ts +++ b/src/renderer/Program.ts @@ -431,12 +431,6 @@ export default class RendererProgram { "resize", this.onResize.bind(this), "RendererProgram#onResize" - ), - addEventListener( - window, - "underflow", - this.onResize.bind(this), - "RendererProgram#onResize" ) ); diff --git a/src/shared/hints.ts b/src/shared/hints.ts index 917daaea..010b146b 100644 --- a/src/shared/hints.ts +++ b/src/shared/hints.ts @@ -6,7 +6,6 @@ export const ElementType = stringUnion({ clickable: null, label: null, link: null, - scrollable: null, selectable: null, textarea: null, }); diff --git a/src/worker/ElementManager.ts b/src/worker/ElementManager.ts index d5d09a46..dc296cf5 100644 --- a/src/worker/ElementManager.ts +++ b/src/worker/ElementManager.ts @@ -99,12 +99,12 @@ export const t = { ELEMENT_TYPES_LOW_QUALITY: elementTypeSet(new Set(["clickable-event"])), - // Give worse hints to scrollable elements and (selectable) frames. They are - // usually very large by nature, but not that commonly used. Also give worse - // hints to elements with click listeners only. They often wrap text inputs, - // covering the hint for the input. + // Give worse hints to selectable frames. They are usually very large by + // nature, but not that commonly used. Also give worse hints to elements with + // click listeners only. They often wrap text inputs, covering the hint for + // the input. ELEMENT_TYPES_WORSE: elementTypeSet( - new Set(["clickable-event", "scrollable", "selectable"]) + new Set(["clickable-event", "selectable"]) ), // Elements this many pixels high or taller always get their hint placed at the @@ -181,8 +181,6 @@ export const t = { ]) ), - VALUES_SCROLLABLE_OVERFLOW: stringSet(new Set(["auto", "scroll"])), - MIN_SIZE_FRAME: unsignedFloat(6), // px MIN_SIZE_TEXT_RECT: unsignedFloat(2), // px MIN_SIZE_ICON: unsignedFloat(10), // px @@ -237,10 +235,6 @@ type QueueItem = target: EventTarget; clickable: boolean; } - | { - type: "OverflowChanged"; - target: EventTarget; - } | { type: "Records"; records: Array | Array; @@ -284,8 +278,6 @@ export default class ElementManager { elementsWithClickListeners = new WeakSet(); - elementsWithScrollbars = new WeakSet(); - shadowRoots = new WeakMap(); idleCallbackId: IdleCallbackID | undefined = undefined; @@ -354,21 +346,6 @@ export default class ElementManager { ); } - this.resets.add( - addEventListener( - window, - "overflow", - this.onOverflowChange.bind(this), - "ElementManager#onOverflowChange" - ), - addEventListener( - window, - "underflow", - this.onOverflowChange.bind(this), - "ElementManager#onOverflowChange" - ) - ); - this.injectScript(); // Wait for tweakable values to load before starting the MutationObserver, @@ -416,7 +393,6 @@ export default class ElementManager { this.visibleFrames.clear(); // `WeakSet`s don’t have a `.clear()` method. this.elementsWithClickListeners = new WeakSet(); - this.elementsWithScrollbars = new WeakSet(); this.shadowRoots = new WeakMap(); this.idleCallbackId = undefined; this.resets.reset(); @@ -702,11 +678,6 @@ export default class ElementManager { } } - onOverflowChange(event: UIEvent): void { - const target = getTarget(event); - this.queueItem({ type: "OverflowChanged", target }); - } - onInjectedMessage(message: FromInjected): void { switch (message.type) { case "ClickableChanged": @@ -748,21 +719,6 @@ export default class ElementManager { mutationObserve(mutationObserver, shadowRoot); - resets.add( - addEventListener( - shadowRoot, - "overflow", - this.onOverflowChange.bind(this), - "ElementManager#onOverflowChange" - ), - addEventListener( - shadowRoot, - "underflow", - this.onOverflowChange.bind(this), - "ElementManager#onOverflowChange" - ) - ); - this.shadowRoots.set(shadowRoot.host, { shadowRoot, mutationObserver, @@ -1104,26 +1060,6 @@ export default class ElementManager { } break; } - - case "OverflowChanged": { - const element = item.target; - if (element instanceof HTMLElement) { - // An element might have `overflow-x: hidden; overflow-y: auto;`. The events - // don't tell which direction changed its overflow, so we must check that - // ourselves. We're only interested in elements with scrollbars, not with - // hidden overflow. - if (isScrollable(element)) { - if (!this.elementsWithScrollbars.has(element)) { - this.elementsWithScrollbars.add(element); - this.addOrRemoveElement("changed", element); - } - } else if (this.elementsWithScrollbars.has(element)) { - this.elementsWithScrollbars.delete(element); - this.addOrRemoveElement("changed", element); - } - } - break; - } } } @@ -1391,19 +1327,6 @@ export default class ElementManager { return "textarea"; } - if ( - this.elementsWithScrollbars.has(element) && - // Allow `` (or ``) to get hints only if they are - // scrollable and in a frame. This allows focusing frames to scroll - // them. In Chrome, `iframeElement.focus()` allows for scrolling a - // specific frame, but I haven’t found a good way to show hints only - // for _scrollable_ frames. Chrome users can use the "select element" - // command instead. See `getElementTypeSelectable`. - !(element === document.scrollingElement && window.top === window) - ) { - return "scrollable"; - } - // `` and `` might have click listeners or role attributes // etc., but we never want hints for them. if (element === document.documentElement || element === document.body) { @@ -1808,18 +1731,6 @@ function getSingleRectPoint({ range: Range; time: TimeTracker; }): Point { - // Scrollbars are usually on the right side, so put the hint there, making it - // easier to see that the hint is for scrolling and reducing overlap. - time.start("getSingleRectPoint:scrollable"); - if (elementType === "scrollable") { - return { - ...getXY(visibleBox), - x: visibleBox.x + visibleBox.width - 1, - align: "right", - debug: "getSingleRectPoint scrollable", - }; - } - // Always put hints for "tall" elements at the left-center edge – except in // selectable mode (long paragraphs). Then it is nicer to put the marker at // the start of the text. @@ -2258,29 +2169,6 @@ function replaceConstants(code: string): string { ); } -function isScrollable(element: HTMLElement): boolean { - const computedStyle = window.getComputedStyle(element); - - // `.scrollLeftMax` and `.scrollTopMax` are Firefox-only, but this function is - // only called from the "overflow" and "underflow" event listeners, and those - // are Firefox-only as well. Those properties are the easiest way to check if - // an element overflows in either the X or Y direction. - return ( - // @ts-expect-error See above. - (element.scrollLeftMax > 0 && - (t.VALUES_SCROLLABLE_OVERFLOW.value.has( - computedStyle.getPropertyValue("overflow-x") - ) || - element === document.scrollingElement)) || - // @ts-expect-error See above. - (element.scrollTopMax > 0 && - (t.VALUES_SCROLLABLE_OVERFLOW.value.has( - computedStyle.getPropertyValue("overflow-y") - ) || - element === document.scrollingElement)) - ); -} - function hasClickListenerProp(element: HTMLElement): boolean { // Adding a `onclick="..."` attribute in HTML automatically sets // `.onclick` of the element to a function. But in Chrome, `.onclick` @@ -2333,9 +2221,8 @@ function hintWeight( // Use logarithms too make the difference between small and large elements // smaller. Instead of an “image card” being 10 times heavier than a // navigation link, it’ll only be about 3 times heavier. Give worse hints to - // some types, such as scrollable elements, by using a logarithm with a higher - // base. A tall scrollable element (1080px) gets a weight slightly smaller - // than that of a small link (12px high). + // some types by using a logarithm with a higher base. A tall element (1080px) + // gets a weight slightly smaller than that of a small link (12px high). const lg = t.ELEMENT_TYPES_WORSE.value.has(elementType) ? Math.log10 : Math.log2; diff --git a/src/worker/Program.ts b/src/worker/Program.ts index 9dfaf1e1..8aa2e51b 100644 --- a/src/worker/Program.ts +++ b/src/worker/Program.ts @@ -1,6 +1,5 @@ import type { ElementReport, - ElementType, ElementTypes, VisibleElement, } from "../shared/hints"; @@ -282,7 +281,6 @@ export default class WorkerProgram { const rects = elements.flatMap((elementData) => getTextRectsHelper({ element: elementData.element, - type: elementData.type, viewports: current.viewports, words: wordsSet, }) @@ -865,10 +863,8 @@ export default class WorkerProgram { if (maybeItem === undefined || !current.indexes.includes(index)) { return []; } - const { element, type } = maybeItem; return getTextRectsHelper({ - element, - type, + element: maybeItem.element, viewports: current.viewports, words: wordsSet, }); @@ -1319,7 +1315,7 @@ function visibleElementToElementReport( ): ElementReport { const text = textContent ? element.textContent ?? "" - : extractTextHelper(element, type); + : extractTextHelper(element); return { type, index, @@ -1367,14 +1363,7 @@ function updateElementsWithEqualOnes( } } -function extractTextHelper(element: HTMLElement, type: ElementType): string { - // Scrollable elements do have `.textContent`, but it’s not intuitive to - // filter them by text (especially since the matching text might be scrolled - // away). Treat them more like frames (where you can’t look inside). - if (type === "scrollable") { - return ""; - } - +function extractTextHelper(element: HTMLElement): string { // For text inputs, textareas, checkboxes, selects, etc., use their label text // for filtering. For buttons with a label, use both the button text and the // label text. @@ -1396,22 +1385,15 @@ function normalizeWhitespace(string: string): string { export function getTextRectsHelper({ element, - type, viewports, words, checkElementAtPoint, }: { element: HTMLElement; - type: ElementType; viewports: Array; words: Set; checkElementAtPoint?: boolean; }): Array { - // See `extractTextHelper`. - if (type === "scrollable") { - return []; - } - // See `extractTextHelper`. const labels = getLabels(element); if (labels !== undefined) {