Skip to content

Commit

Permalink
Fix performance issue when calculating if element has significant sib…
Browse files Browse the repository at this point in the history
…lings (#166)

* Remove <option> element from hintable selector as we cannot display hints for them

* Fix performance issue when calculating if element has significant siblings
  • Loading branch information
david-tejada authored Aug 4, 2023
1 parent 170e87c commit 8169143
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 8 deletions.
2 changes: 1 addition & 1 deletion src/content/hints/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { retrieve, store } from "../../common/storage";

const defaultSelector =
// Elements
"button, a, input, summary, textarea, select, option, label, " +
"button, a, input, summary, textarea, select, label, " +
// Roles
"[role='button'], [role='link'], [role='treeitem'], [role='tab'], [role='option'], " +
"[role='radio'], [role='checkbox'], [role='menuitem'], [role='menuitemradio'], [role='menuitemcheckbox'], " +
Expand Down
24 changes: 17 additions & 7 deletions src/content/utils/isHintable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,27 @@ import {
} from "../hints/selectors";
import { isVisible } from "./isVisible";

/**
* Returns true if the element has any Element siblings (that are not hints) or
* any Text sibling with content. For performance reasons if the element has
* more than 9 siblings of any kind it will return true.
*/
function hasSignificantSiblings(target: Node) {
if (!target.parentNode) return false;

const significantSiblingsIncludingSelf = [
...target.parentNode.childNodes,
].filter(
// This is to improve performance in case the parent has many child nodes. In
// that case we can safely assume the element has significant siblings.
if (target.parentNode.childNodes.length > 10) return true;

return [...target.parentNode.childNodes].some(
(node) =>
node !== target &&
// We need to exclude hint divs for when we display/remove extra hints
(node instanceof Element && node.className !== "rango-hint-wrapper") ||
(node instanceof Text && node.textContent && /\S/.test(node.textContent))
((node instanceof Element && node.className !== "rango-hint") ||
(node instanceof Text &&
node.textContent &&
/\S/.test(node.textContent)))
);

return significantSiblingsIncludingSelf.length > 1;
}

function isRedundant(target: Element) {
Expand All @@ -38,6 +46,8 @@ function isRedundant(target: Element) {
return false;
}

// This catches instances of hintables that are wrapped within another
// hintable. For example a <div role="button"> that only contains a <button>.
if (
target.parentElement &&
matchesHintableSelector(target.parentElement) &&
Expand Down

0 comments on commit 8169143

Please sign in to comment.