From 2c1202f1dd9565f18ac13b60e4c969c6404f2d89 Mon Sep 17 00:00:00 2001 From: Tyler Elliott Date: Thu, 9 Mar 2017 14:28:16 -0800 Subject: [PATCH] Include elements with pointer cursor style as clickable links. (this is a follow up to https://github.com/philc/vimium/pull/2410) The pointer cursor is a UI hint that something can be clicked on the page. This is often the simplest way to detect if a non-input, non-anchor element should have a hint. For example, the side bar in Feedly or the month overview in the top left of Google Calendar. This change uses `computedStyleMap` to determine the value of an element's cursor. We only want to know the first element that sets its cursor to pointer. All children of that first element will also inherit the style and because this method includes inherited styles we need a special check to make sure a given style is in fact the first pointer in the tree. As for performance, computing the style of an element is relatively expensive. Using `computedStyleMap` over `getComputedStyle` yields a ~25% performance improvement. I also experimented with a cache that saves class names that did not cause an element to have a pointer style. Depending on the number of elements, this improved performance by another ~10% . Ultimately, I left out the caching solution as it comes at the cost of memory, additional code complexity, and increases the number of false negatives, e.g. a `menu` class appears on both the `ul` and `li` tags but only creates a pointer on the `li`. Another option would be to limit the number of times we run `computedStyleMap` to something like 10k times per call to `getLinkHints` I did a similar test suite as in the previous pull request, this time with 100 loops. Every page experienced worse performance (as expected), however all pages (except twitter) had median execution times under 15ms. Twitter's 20k element test had a median of 90ms (control of 43ms) and the 50k element test went from 136ms to 274ms (the caching solution was most helpful here at 195ms). --- CREDITS | 1 + content_scripts/link_hints.coffee | 10 ++++++++++ 2 files changed, 11 insertions(+) mode change 100644 => 100755 CREDITS diff --git a/CREDITS b/CREDITS old mode 100644 new mode 100755 index a7877ff81..f7e51f1e6 --- a/CREDITS +++ b/CREDITS @@ -49,5 +49,6 @@ Contributors: Darryl Pogue (github: dpogue) tobimensch Ramiro Araujo (github: ramiroaraujo) + Tyler Elliott (github: tylercal) Feel free to add real names in addition to GitHub usernames. diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 849aa4c50..3a52e7446 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -710,6 +710,16 @@ LocalHints = # # Detect elements with "click" listeners installed with `addEventListener()`. # isClickable ||= element.hasAttribute "_vimium-has-onclick-listener" + # in the meantime, use the more expensive computedStyleMap to see if elements have a "cursor: pointer" style + # NOTE: because we are optimizing by checking the className attribute of the element, we will miss elements + # that get their pointer style from combinators and tag styles, e.g. li > span, .menu > div + isClickable ||= element.style.cursor == "pointer" or + element.className != "" and + element.computedStyleMap().get("cursor").value == "pointer" and + element.parentElement.computedStyleMap().get("cursor").value != "pointer" + + + # An element with a class name containing the text "button" 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.