From e3f8edef7875792c2037c9013a8e09f7892a66a4 Mon Sep 17 00:00:00 2001 From: Simon Lydell Date: Thu, 21 Jan 2016 07:47:18 +0100 Subject: [PATCH] Recognize more elements as clickable - Elements with `data-` attributes used for clickable things by Bootstrap are now recognized. This is a huge win since Bootstrap is very popular and many developers use it with semantically unclickable elements (even though the Bootstrap documentation always reminds of accessibility concerns). Since these elements are or _should have been_ semantically clickable elements, they are always treated as such, in order not to give them too bad hints. - A few `aria-` attributes are now recognized as semantically clickable. - ARIA roles are now treated as _semantically_ clickable, instead of just clickable. - "Close" buttons are now attempted to be recognized. This is useful for many of those obtrusive EU cookie law notices. Recognizing Bootstrap elements also helps greatly here. - Finally, elements with 'click' event listeners (added using `element.addEventListener('click', ...)`) are now recognized, by using a Firefox API. This is really useful on DuckDuckGo's image search. Fixes #671 and fixes #672. Note that event delegation is not supported, since that would require inspecting the code of the event listeners. One _could_ inspect the global `jQuery` object (if any) to get to solve that problem to a great extent (like Firefox's devtools do), but I'm not sure if that's a good idea. It should also be noted that I tried recognizing elements matching `/\bjs-/.test(element.className)`, but that proved give too many false positives. --- extension/lib/commands-frame.coffee | 41 +++++++++++++++++++++++------ extension/lib/utils.coffee | 17 ++++++++++++ 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/extension/lib/commands-frame.coffee b/extension/lib/commands-frame.coffee index 33a98e4..3700854 100644 --- a/extension/lib/commands-frame.coffee +++ b/extension/lib/commands-frame.coffee @@ -138,10 +138,27 @@ commands.follow = helper_follow.bind(null, {id: 'normal'}, type = null semantic = true switch + # Bootstrap. Match these before regular links, because especially slider + # “buttons” often get the same hint otherwise. + when element.hasAttribute('data-toggle') or + element.hasAttribute('data-dismiss') or + element.hasAttribute('data-slide') or + element.hasAttribute('data-slide-to') + # Some elements may not be semantic, but _should be_ and still deserve a + # good hint. + type = 'clickable' when isProperLink(element) type = 'link' when isTypingElement(element) type = 'text' + when element.getAttribute('role') in CLICKABLE_ARIA_ROLES or + # + element.hasAttribute('aria-controls') or + element.hasAttribute('aria-pressed') or + element.hasAttribute('aria-checked') or + (element.hasAttribute('aria-haspopup') and + element.getAttribute('role') != 'menu') + type = 'clickable' when element.tabIndex > -1 and # Google Drive Documents. The hint for this element would cover the # real hint that allows you to focus the document to start typing. @@ -158,7 +175,6 @@ commands.follow = helper_follow.bind(null, {id: 'normal'}, element.hasAttribute('onmousedown') or element.hasAttribute('onmouseup') or element.hasAttribute('oncommand') or - element.getAttribute('role') in CLICKABLE_ARIA_ROLES or # Twitter. element.classList.contains('js-new-tweets-bar') or # Feedly. @@ -187,16 +203,25 @@ commands.follow = helper_follow.bind(null, {id: 'normal'}, element.querySelector('input, textarea, select') if input and not getElementShape(input) type = 'clickable' - # Elements that have “button” somewhere in the class might be clickable, - # unless they contain a real link or button or yet an element with - # “button” somewhere in the class, in which case they likely are - # “button-wrapper”s. (`.className` is not a string!) - when not isXUL and typeof element.className == 'string' and - element.className.toLowerCase().includes('button') + # Last resort checks for elements that might be clickable because of + # JavaScript. + when (not isXUL and + # It is common to listen for clicks on `` or ``. Don’t + # waste time on them. + element not in [document.documentElement, document.body]) and + (utils.includes(element.className, 'button') or + utils.includes(element.className, 'close') or + utils.includes(element.getAttribute('aria-label'), 'close') or + # Do this last as it’s a potentially expensive check. + utils.hasEventListeners(element, 'click')) + # Make a quick check for likely clickable descendants, to reduce the + # number of false positives. the element might be a “button-wrapper” or + # a large element with a click-tracking event listener. unless element.querySelector('a, button, input, [class*=button]') type = 'clickable' semantic = false - # When viewing an image it should get a marker to toggle zoom. + # When viewing an image it should get a marker to toggle zoom. This is the + # most unlikely rule to match, so keep it last. when document.body?.childElementCount == 1 and element.nodeName == 'IMG' and (element.classList.contains('overflowing') or diff --git a/extension/lib/utils.coffee b/extension/lib/utils.coffee index 5a1bd0b..bc94aec 100644 --- a/extension/lib/utils.coffee +++ b/extension/lib/utils.coffee @@ -25,6 +25,8 @@ nsIClipboardHelper = Cc['@mozilla.org/widget/clipboardhelper;1'] .getService(Ci.nsIClipboardHelper) nsIDomUtils = Cc['@mozilla.org/inspector/dom-utils;1'] .getService(Ci.inIDOMUtils) +nsIEventListenerService = Cc['@mozilla.org/eventlistenerservice;1'] + .getService(Ci.nsIEventListenerService) nsIFocusManager = Cc['@mozilla.org/focus-manager;1'] .getService(Ci.nsIFocusManager) nsIStyleSheetService = Cc['@mozilla.org/content/style-sheet-service;1'] @@ -338,6 +340,13 @@ class EventEmitter has = (obj, prop) -> Object::hasOwnProperty.call(obj, prop) +# Check if `search` exists in `string` (case insensitively). Returns `false` if +# `string` doesn’t exist or isn’t a string, such as `.className`. +includes = (string, search) -> + return false unless typeof string == 'string' + return string.toLowerCase().includes(search) + + nextTick = (window, fn) -> window.setTimeout(fn, 0) regexEscape = (s) -> s.replace(/[|\\{}()[\]^$+*?.]/g, '\\$&') @@ -379,6 +388,12 @@ getCurrentLocation = -> getCurrentWindow = -> nsIWindowMediator.getMostRecentWindow('navigator:browser') +hasEventListeners = (element, type) -> + for listener in nsIEventListenerService.getListenerInfoFor(element) + if listener.listenerObject and listener.type == type + return true + return false + loadCss = (name) -> uri = Services.io.newURI("chrome://vimfx/skin/#{name}.css", null, null) method = nsIStyleSheetService.AUTHOR_SHEET @@ -447,6 +462,7 @@ module.exports = { Counter EventEmitter has + includes nextTick regexEscape removeDuplicates @@ -456,6 +472,7 @@ module.exports = { formatError getCurrentLocation getCurrentWindow + hasEventListeners loadCss observe openPopup -- 2.39.3