From 4656c1c5a9741293ba18975f0c419bead3075040 Mon Sep 17 00:00:00 2001 From: ScottWilliamAnderson <47524721+ScottWilliamAnderson@users.noreply.github.com> Date: Tue, 29 Oct 2024 09:24:49 +0000 Subject: [PATCH 1/5] Fix clickable elements with class 'btn' Fixes #3550 Update `content_scripts/link_hints.js` to make elements with class 'btn' clickable. * Add logic to `getLocalHintsForElement` to check for class 'btn' and mark elements as clickable. --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/philc/vimium/issues/3550?shareId=XXXX-XXXX-XXXX-XXXX). --- content_scripts/link_hints.js | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/content_scripts/link_hints.js b/content_scripts/link_hints.js index d621a08d0..25b38e056 100644 --- a/content_scripts/link_hints.js +++ b/content_scripts/link_hints.js @@ -1,18 +1,3 @@ -// -// This implements link hinting. Typing "F" will enter link-hinting mode, where all clickable items -// on the page have a hint marker displayed containing a sequence of letters. Typing those letters -// will select a link. -// -// In our 'default' mode, the characters we use to show link hints are a user-configurable option. -// By default they're the home row. The CSS which is used on the link hints is also a configurable -// option. -// -// In 'filter' mode, our link hints are numbers, and the user can narrow down the range of -// possibilities by typing the text of the link itself. -// - -// A DOM element that sits on top of a link, showing the key the user should type to select the -// link. class HintMarker { hintDescriptor; localHint; @@ -1222,6 +1207,12 @@ const LocalHints = { onlyHasTabIndex = true; } + // Check for elements with class 'btn' + if (!isClickable && className?.toLowerCase().includes("btn")) { + isClickable = true; + possibleFalsePositive = true; + } + if (isClickable) { // An image map has multiple clickable areas, and so can represent multiple LocalHints. if (imageMapAreas.length > 0) { From 694ba537f7fb3cf5f472405e70513590e1da8e64 Mon Sep 17 00:00:00 2001 From: ScottWilliamAnderson <47524721+ScottWilliamAnderson@users.noreply.github.com> Date: Tue, 29 Oct 2024 09:46:16 +0000 Subject: [PATCH 2/5] * Keep the comments at the top of the file --- content_scripts/link_hints.js | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/content_scripts/link_hints.js b/content_scripts/link_hints.js index 25b38e056..ef7f81f09 100644 --- a/content_scripts/link_hints.js +++ b/content_scripts/link_hints.js @@ -1,3 +1,18 @@ +// +// This implements link hinting. Typing "F" will enter link-hinting mode, where all clickable items +// on the page have a hint marker displayed containing a sequence of letters. Typing those letters +// will select a link. +// +// In our 'default' mode, the characters we use to show link hints are a user-configurable option. +// By default they're the home row. The CSS which is used on the link hints is also a configurable +// option. +// +// In 'filter' mode, our link hints are numbers, and the user can narrow down the range of +// possibilities by typing the text of the link itself. +// + +// A DOM element that sits on top of a link, showing the key the user should type to select the +// link. class HintMarker { hintDescriptor; localHint; @@ -1189,7 +1204,7 @@ const LocalHints = { // # Detect elements with "click" listeners installed with `addEventListener()`. // isClickable ||= element.hasAttribute "_vimium-has-onclick-listener" - // An element with a class name containing the text "button" might be clickable. However, real + // An element with a class name containing the text "button" or "btn" might be clickable. However, real // clickables are often wrapped in elements with such class names. So, when we find clickables // based only on their class name, we mark them as unreliable. const className = element.getAttribute("class"); @@ -1198,6 +1213,11 @@ const LocalHints = { possibleFalsePositive = true; } + if (!isClickable && className?.toLowerCase().includes("btn")) { + isClickable = true; + possibleFalsePositive = true; + } + // Elements with tabindex are sometimes useful, but usually not. We can treat them as second // class citizens when it improves UX, so take special note of them. const tabIndexValue = element.getAttribute("tabindex"); @@ -1207,12 +1227,6 @@ const LocalHints = { onlyHasTabIndex = true; } - // Check for elements with class 'btn' - if (!isClickable && className?.toLowerCase().includes("btn")) { - isClickable = true; - possibleFalsePositive = true; - } - if (isClickable) { // An image map has multiple clickable areas, and so can represent multiple LocalHints. if (imageMapAreas.length > 0) { From 2b0030ffb6780aa0dc7761128e17056c2f4bd0b5 Mon Sep 17 00:00:00 2001 From: ScottWilliamAnderson <47524721+ScottWilliamAnderson@users.noreply.github.com> Date: Wed, 30 Oct 2024 07:45:51 +0000 Subject: [PATCH 3/5] Update `getLocalHintsForElement` to check for class 'btn' and remove redundant check for class 'button' * Merge the if statement for 'button' and 'btn' classes * Remove redundant check for class 'button' --- content_scripts/link_hints.js | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/content_scripts/link_hints.js b/content_scripts/link_hints.js index ef7f81f09..ac3b6a947 100644 --- a/content_scripts/link_hints.js +++ b/content_scripts/link_hints.js @@ -1,18 +1,3 @@ -// -// This implements link hinting. Typing "F" will enter link-hinting mode, where all clickable items -// on the page have a hint marker displayed containing a sequence of letters. Typing those letters -// will select a link. -// -// In our 'default' mode, the characters we use to show link hints are a user-configurable option. -// By default they're the home row. The CSS which is used on the link hints is also a configurable -// option. -// -// In 'filter' mode, our link hints are numbers, and the user can narrow down the range of -// possibilities by typing the text of the link itself. -// - -// A DOM element that sits on top of a link, showing the key the user should type to select the -// link. class HintMarker { hintDescriptor; localHint; @@ -1208,12 +1193,7 @@ const LocalHints = { // clickables are often wrapped in elements with such class names. So, when we find clickables // based only on their class name, we mark them as unreliable. const className = element.getAttribute("class"); - if (!isClickable && className?.toLowerCase().includes("button")) { - isClickable = true; - possibleFalsePositive = true; - } - - if (!isClickable && className?.toLowerCase().includes("btn")) { + if (!isClickable && (className?.toLowerCase().includes("button") || className?.toLowerCase().includes("btn"))) { isClickable = true; possibleFalsePositive = true; } From 35e1bc5c2076a5bbc2ca5c32ac4691f3934bee37 Mon Sep 17 00:00:00 2001 From: ScottWilliamAnderson <47524721+ScottWilliamAnderson@users.noreply.github.com> Date: Wed, 30 Oct 2024 16:32:42 +0000 Subject: [PATCH 4/5] returned mistakenly deleted comments --- content_scripts/link_hints.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/content_scripts/link_hints.js b/content_scripts/link_hints.js index ac3b6a947..1d4291c70 100644 --- a/content_scripts/link_hints.js +++ b/content_scripts/link_hints.js @@ -1,3 +1,18 @@ +// +// This implements link hinting. Typing "F" will enter link-hinting mode, where all clickable items +// on the page have a hint marker displayed containing a sequence of letters. Typing those letters +// will select a link. +// +// In our 'default' mode, the characters we use to show link hints are a user-configurable option. +// By default they're the home row. The CSS which is used on the link hints is also a configurable +// option. +// +// In 'filter' mode, our link hints are numbers, and the user can narrow down the range of +// possibilities by typing the text of the link itself. +// + +// A DOM element that sits on top of a link, showing the key the user should type to select the +// link. class HintMarker { hintDescriptor; localHint; From 276e781d8fdef2f7fa233b3d5a41ad17f04c856c Mon Sep 17 00:00:00 2001 From: Scott <47524721+ScottWilliamAnderson@users.noreply.github.com> Date: Sat, 4 Jan 2025 20:22:25 +0000 Subject: [PATCH 5/5] Simplify class name handling in link hints --- content_scripts/link_hints.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/content_scripts/link_hints.js b/content_scripts/link_hints.js index 1d4291c70..3bdcd45be 100644 --- a/content_scripts/link_hints.js +++ b/content_scripts/link_hints.js @@ -1207,8 +1207,8 @@ const LocalHints = { // An element with a class name containing the text "button" or "btn" might be clickable. However, real // clickables are often wrapped in elements with such class names. So, when we find clickables // based only on their class name, we mark them as unreliable. - const className = element.getAttribute("class"); - if (!isClickable && (className?.toLowerCase().includes("button") || className?.toLowerCase().includes("btn"))) { + const className = element.getAttribute("class")?.toLowerCase() + if (!isClickable && (className?.includes("button") || className?.includes("btn"))) { isClickable = true; possibleFalsePositive = true; }