From a199335790aec50cf3ed7cc27c5b407875c37107 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Wed, 17 Dec 2014 10:15:54 +0000 Subject: Use the DOM rather than XPath to detect clickable elements --- content_scripts/link_hints.coffee | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 24bd7126..2ffe818f 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -36,17 +36,6 @@ LinkHints = # init: -> - # - # Generate an XPath describing what a clickable element is. - # The final expression will be something like "//button | //xhtml:button | ..." - # We use translate() instead of lower-case() because Chrome only supports XPath 1.0. - # - clickableElementsXPath: DomUtils.makeXPath( - ["a", "area[@href]", "textarea", "button", "select", - "input[not(@type='hidden' or @disabled or @readonly)]", - "*[@onclick or @tabindex or @role='link' or @role='button' or contains(@class, 'button') or " + - "@contenteditable='' or translate(@contenteditable, 'TRUE', 'true')='true']"]) - # We need this as a top-level function because our command system doesn't yet support arguments. activateModeToOpenInNewTab: -> @activateMode(OPEN_IN_NEW_BG_TAB) activateModeToOpenInNewForegroundTab: -> @activateMode(OPEN_IN_NEW_FG_TAB) @@ -141,13 +130,12 @@ LinkHints = # of digits needed to enumerate all of the links on screen. # getVisibleClickableElements: -> - resultSet = DomUtils.evaluateXPath(@clickableElementsXPath, XPathResult.ORDERED_NODE_SNAPSHOT_TYPE) + resultSet = DomUtils.getClickableElements() visibleElements = [] # Find all visible clickable elements. - for i in [0...resultSet.snapshotLength] by 1 - element = resultSet.snapshotItem(i) + for element in resultSet clientRect = DomUtils.getVisibleClientRect(element, clientRect) if (clientRect != null) visibleElements.push({element: element, rect: clientRect}) -- cgit v1.2.3 From c7e2f1cdef2d5e99761d7bb8ecbad91f89de6958 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Wed, 17 Dec 2014 11:12:58 +0000 Subject: Inline DomUtils.getClickableElements --- content_scripts/link_hints.coffee | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 2ffe818f..fe36cee0 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -130,7 +130,41 @@ LinkHints = # of digits needed to enumerate all of the links on screen. # getVisibleClickableElements: -> - resultSet = DomUtils.getClickableElements() + elements = Array::slice.call(document.documentElement.getElementsByTagName "*") + resultSet = [] + + for element in elements + isClickable = false + tagName = element.tagName.toLowerCase() + isClickable = (-> + if element.hasAttribute "onclick" + true + else if element.hasAttribute "tabindex" + true + else if element.getAttribute "role" in ["button", "link"] + true + else if element.getAttribute("class")?.toLowerCase().indexOf("button") >= 0 + true + else if element.getAttribute("contentEditable")?.toLowerCase() in ["", "contentEditable", "true"] + true + else if tagName == "a" + true + else if tagName == "img" + mapName = element.getAttribute "usemap" + if mapName + map = document.querySelector(mapName.replace /^#/, "") + areas = Array::slice.call(map.getElementsByTagName "area") + resultSet.concat areas + false + else if (tagName == "input" and DomUtils.isSelectable element) or tagName == "textarea" + not (element.disabled or element.readOnly) + else if (tagName == "input" and element.getAttribute("type")?.toLowerCase() != "hidden") or + tagName in ["button", "select"] + not element.disabled + else + false + )() + resultSet.push element if isClickable visibleElements = [] -- cgit v1.2.3 From 28c275ae4128f0a2907c7ad3d27cedc81efe129a Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Wed, 17 Dec 2014 11:41:10 +0000 Subject: Simplify finding clickable elements --- content_scripts/link_hints.coffee | 58 +++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 30 deletions(-) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index fe36cee0..18e9741d 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -134,37 +134,35 @@ LinkHints = resultSet = [] for element in elements - isClickable = false tagName = element.tagName.toLowerCase() - isClickable = (-> - if element.hasAttribute "onclick" - true - else if element.hasAttribute "tabindex" - true - else if element.getAttribute "role" in ["button", "link"] - true - else if element.getAttribute("class")?.toLowerCase().indexOf("button") >= 0 - true - else if element.getAttribute("contentEditable")?.toLowerCase() in ["", "contentEditable", "true"] - true - else if tagName == "a" - true - else if tagName == "img" - mapName = element.getAttribute "usemap" - if mapName - map = document.querySelector(mapName.replace /^#/, "") - areas = Array::slice.call(map.getElementsByTagName "area") - resultSet.concat areas - false - else if (tagName == "input" and DomUtils.isSelectable element) or tagName == "textarea" - not (element.disabled or element.readOnly) - else if (tagName == "input" and element.getAttribute("type")?.toLowerCase() != "hidden") or - tagName in ["button", "select"] - not element.disabled - else - false - )() - resultSet.push element if isClickable + + # Insert area elements that provide click functionality to an img. + if tagName == "img" + mapName = element.getAttribute "usemap" + if mapName + mapName = mapName.replace(/^#/, "").replace("\"", "\\\"") + map = document.querySelector "map[name=\"#{mapName}\"]" + areas = if map then Array::slice.call(map.getElementsByTagName "area") else [] + resultSet = resultSet.concat areas + + # Check for attributes that make an element clickable regardless of its tagName. + if (element.hasAttribute "onclick" or + element.hasAttribute "tabindex" or + element.getAttribute "role" in ["button", "link"] or + element.getAttribute("class")?.toLowerCase().indexOf("button") >= 0 or + element.getAttribute("contentEditable")?.toLowerCase() in ["", "contentEditable", "true"]) + resultSet.push element + continue + + switch tagName + when "a" + resultSet.push element + when "textarea", "input" + unless (tagName == "input" and element.getAttribute("type")?.toLowerCase() == "hidden") or + element.disabled or (element.readOnly and DomUtils.isSelectable element) + resultSet.push element + when "button", "select" + resultSet.push element unless element.disabled visibleElements = [] -- cgit v1.2.3 From 5f9290693ab0f35c46cea6cea0a9f5c06b4ee0ad Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Wed, 17 Dec 2014 12:27:40 +0000 Subject: Combine rectangle calculation and clickable element detection --- content_scripts/link_hints.coffee | 51 ++++++++++++--------------------------- 1 file changed, 16 insertions(+), 35 deletions(-) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 18e9741d..dd359a70 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -131,19 +131,23 @@ LinkHints = # getVisibleClickableElements: -> elements = Array::slice.call(document.documentElement.getElementsByTagName "*") - resultSet = [] + visibleElements = [] for element in elements tagName = element.tagName.toLowerCase() + isClickable = false # Insert area elements that provide click functionality to an img. if tagName == "img" mapName = element.getAttribute "usemap" if mapName + imgClientRects = element.getClientRects() mapName = mapName.replace(/^#/, "").replace("\"", "\\\"") map = document.querySelector "map[name=\"#{mapName}\"]" - areas = if map then Array::slice.call(map.getElementsByTagName "area") else [] - resultSet = resultSet.concat areas + if map and imgClientRects.length > 0 + areas = map.getElementsByTagName "area" + areaRects = DomUtils.getClientRectsForAreas imgClientRects[0], areas + visibleElements = visibleElements.concat areaRects # Check for attributes that make an element clickable regardless of its tagName. if (element.hasAttribute "onclick" or @@ -151,46 +155,23 @@ LinkHints = element.getAttribute "role" in ["button", "link"] or element.getAttribute("class")?.toLowerCase().indexOf("button") >= 0 or element.getAttribute("contentEditable")?.toLowerCase() in ["", "contentEditable", "true"]) - resultSet.push element - continue + isClickable = true + # Check for tagNames which are natively clickable. switch tagName when "a" - resultSet.push element + isClickable = true when "textarea", "input" unless (tagName == "input" and element.getAttribute("type")?.toLowerCase() == "hidden") or element.disabled or (element.readOnly and DomUtils.isSelectable element) - resultSet.push element + isClickable = true when "button", "select" - resultSet.push element unless element.disabled - - visibleElements = [] + isClickable = not element.disabled - # Find all visible clickable elements. - for element in resultSet - clientRect = DomUtils.getVisibleClientRect(element, clientRect) - if (clientRect != null) - visibleElements.push({element: element, rect: clientRect}) - - if (element.localName == "area") - map = element.parentElement - continue unless map - img = document.querySelector("img[usemap='#" + map.getAttribute("name") + "']") - continue unless img - imgClientRects = img.getClientRects() - continue if (imgClientRects.length == 0) - c = element.coords.split(/,/) - coords = [parseInt(c[0], 10), parseInt(c[1], 10), parseInt(c[2], 10), parseInt(c[3], 10)] - rect = { - top: imgClientRects[0].top + coords[1], - left: imgClientRects[0].left + coords[0], - right: imgClientRects[0].left + coords[2], - bottom: imgClientRects[0].top + coords[3], - width: coords[2] - coords[0], - height: coords[3] - coords[1] - } - - visibleElements.push({element: element, rect: rect}) + continue unless isClickable # If the element isn't clickable, do nothing. + clientRect = DomUtils.getVisibleClientRect element + if clientRect != null + visibleElements.push {element: element, rect: clientRect} visibleElements -- cgit v1.2.3 From 82beb23ce138505f0358ec8e15a56d20db6846dd Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Wed, 17 Dec 2014 12:42:31 +0000 Subject: Remove redundant array conversion --- content_scripts/link_hints.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index dd359a70..4b039935 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -130,7 +130,7 @@ LinkHints = # of digits needed to enumerate all of the links on screen. # getVisibleClickableElements: -> - elements = Array::slice.call(document.documentElement.getElementsByTagName "*") + elements = document.documentElement.getElementsByTagName "*" visibleElements = [] for element in elements -- cgit v1.2.3 From 855e9a4e19ab0926f5531c37272f00a715f45ed8 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Thu, 18 Dec 2014 10:33:09 +0000 Subject: Remove overlapping rects from link hints --- content_scripts/link_hints.coffee | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 4b039935..721070bb 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -173,7 +173,35 @@ LinkHints = if clientRect != null visibleElements.push {element: element, rect: clientRect} - visibleElements + # TODO(mrmr1993): Consider z-index. z-index affects behviour as follows: + # * The document has a local stacking context. + # * An element with z-index specified + # - sets its z-order position in the containing stacking context, and + # - creates a local stacking context containing its children. + # * An element (1) is shown above another element (2) if either + # - in the last stacking context which contains both an ancestor of (1) and an ancestor of (2), the + # ancestor of (1) has a higher z-index than the ancestor of (2); or + # - in the last stacking context which contains both an ancestor of (1) and an ancestor of (2), + # + the ancestors of (1) and (2) have equal z-index, and + # + the ancestor of (1) appears later in the DOM than the ancestor of (2). + # + # Remove rects from + nonOverlappingElements = [] + visibleElements = visibleElements.reverse() + while visibleElement = visibleElements.pop() + rects = [visibleElement.rect] + for {rect: negativeRect} in visibleElements + rects = Array::concat.apply [], (rects.map (rect) -> Utils.subtractRect rect, negativeRect) + if rects.length > 0 + nonOverlappingElements.push {element: visibleElement.element, rect: rects[0]} + else + # Every part of the element is covered by some other element, so just insert the whole element's + # rect. + # TODO(mrmr1993): This is probably the wrong thing to do, but we don't want to stop being able to + # click some elements that we could click before. + nonOverlappingElements.push visibleElement + + nonOverlappingElements # # Handles shift and esc keys. The other keys are passed to getMarkerMatcher().matchHintsByKey. -- cgit v1.2.3 From 4e6513c47b6be2e771b2a8db6d5506d157368602 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Thu, 18 Dec 2014 10:40:33 +0000 Subject: Add link hint support for jsaction event listeners This was adapted from PR #1316, commit 846a19efe51bfc639ae1ee84e18a5f2d3e12aaff --- content_scripts/link_hints.coffee | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 721070bb..231e4ecd 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -157,6 +157,13 @@ LinkHints = element.getAttribute("contentEditable")?.toLowerCase() in ["", "contentEditable", "true"]) isClickable = true + # Check for jsaction event listeners on the element. + if element.hasAttribute "jsaction" + jsactionRules = element.getAttribute("jsaction").split(";") + for jsactionRule in jsactionRules + ruleSplit = jsactionRule.split ":" + isClickable = true if ruleSplit[0] == "click" or (ruleSplit.length == 1 and ruleSplit[0] != "none") + # Check for tagNames which are natively clickable. switch tagName when "a" -- cgit v1.2.3 From 3132ae601b2de787f9cddd3fd77b36767e2e467e Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Thu, 18 Dec 2014 11:00:15 +0000 Subject: Complete a partially written comment --- content_scripts/link_hints.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 231e4ecd..57b46e6b 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -192,7 +192,7 @@ LinkHints = # + the ancestors of (1) and (2) have equal z-index, and # + the ancestor of (1) appears later in the DOM than the ancestor of (2). # - # Remove rects from + # Remove rects from elements where another clickable element lies above it. nonOverlappingElements = [] visibleElements = visibleElements.reverse() while visibleElement = visibleElements.pop() -- cgit v1.2.3 From 9c9c48598534c2a0cd8aec28a4a806d74f28e090 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Thu, 18 Dec 2014 11:56:53 +0000 Subject: Move rect functions to their own file --- content_scripts/link_hints.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 57b46e6b..27402250 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -198,7 +198,7 @@ LinkHints = while visibleElement = visibleElements.pop() rects = [visibleElement.rect] for {rect: negativeRect} in visibleElements - rects = Array::concat.apply [], (rects.map (rect) -> Utils.subtractRect rect, negativeRect) + rects = Array::concat.apply [], (rects.map (rect) -> Rect.subtract rect, negativeRect) if rects.length > 0 nonOverlappingElements.push {element: visibleElement.element, rect: rects[0]} else -- cgit v1.2.3 From 93a5a571cc06087b2abfe383424c2fcc6ef02358 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Thu, 18 Dec 2014 13:29:46 +0000 Subject: Split textarea and input detection in link hints --- content_scripts/link_hints.coffee | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 27402250..95026cba 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -168,12 +168,15 @@ LinkHints = switch tagName when "a" isClickable = true - when "textarea", "input" - unless (tagName == "input" and element.getAttribute("type")?.toLowerCase() == "hidden") or - element.disabled or (element.readOnly and DomUtils.isSelectable element) + when "textarea" + isClickable = not element.disabled and not element.readOnly + when "input" + unless element.getAttribute("type")?.toLowerCase() == "hidden" or + element.disabled or + (element.readOnly and DomUtils.isSelectable element) isClickable = true when "button", "select" - isClickable = not element.disabled + isClickable = true unless element.disabled continue unless isClickable # If the element isn't clickable, do nothing. clientRect = DomUtils.getVisibleClientRect element -- cgit v1.2.3 From aa047d5b4b6124f7e2d1230ab590b9244db6ebb3 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Thu, 18 Dec 2014 13:39:22 +0000 Subject: Improve comments for LinkHints.getVisibleClickableElements --- content_scripts/link_hints.coffee | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 95026cba..b1c44e42 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -125,9 +125,11 @@ LinkHints = marker # - # Returns all clickable elements that are not hidden and are in the current viewport. - # We prune invisible elements partly for performance reasons, but moreso it's to decrease the number - # of digits needed to enumerate all of the links on screen. + # Returns all clickable elements that are not hidden and are in the current viewport, along with rectangles + # at which (parts of) the elements are displayed. + # In the process, we try to find rects where elements do not overlap so that link hints are unambiguous. + # Because of this, the rects returned will frequently *NOT* be equivalent to the rects for the whole + # element. # getVisibleClickableElements: -> elements = document.documentElement.getElementsByTagName "*" @@ -197,6 +199,7 @@ LinkHints = # # Remove rects from elements where another clickable element lies above it. nonOverlappingElements = [] + # Traverse the DOM from first to last, since later elements show above earlier elements. visibleElements = visibleElements.reverse() while visibleElement = visibleElements.pop() rects = [visibleElement.rect] -- cgit v1.2.3 From e56dea52d4e0eead061f676891c04cfc07336194 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Thu, 18 Dec 2014 15:08:59 +0000 Subject: Add brackets so the code compiles as expected --- content_scripts/link_hints.coffee | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index b1c44e42..ba1603e4 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -152,9 +152,9 @@ LinkHints = visibleElements = visibleElements.concat areaRects # Check for attributes that make an element clickable regardless of its tagName. - if (element.hasAttribute "onclick" or - element.hasAttribute "tabindex" or - element.getAttribute "role" in ["button", "link"] or + if (element.hasAttribute("onclick") or + element.hasAttribute("tabindex") or + element.getAttribute("role")?.toLowerCase() in ["button", "link"] or element.getAttribute("class")?.toLowerCase().indexOf("button") >= 0 or element.getAttribute("contentEditable")?.toLowerCase() in ["", "contentEditable", "true"]) isClickable = true -- cgit v1.2.3 From 4cfdc55b2054f3b00daf2aa2d8ffd482b4e3aaf9 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Thu, 18 Dec 2014 22:46:17 +0000 Subject: Don't show a link hint for certain link hint elements Disables showing link hint for elements which * are identified as clickableonly by the tabindex attribute, and * have the entirety of their contents overlapped by other clickable elements. This removes some redundant link hints that were visible on Google+, and hopefully shouldn't remove any useful link hints. --- content_scripts/link_hints.coffee | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index ba1603e4..6934e5b8 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -138,6 +138,7 @@ LinkHints = for element in elements tagName = element.tagName.toLowerCase() isClickable = false + onlyHasTabIndex = false # Insert area elements that provide click functionality to an img. if tagName == "img" @@ -153,7 +154,6 @@ LinkHints = # Check for attributes that make an element clickable regardless of its tagName. if (element.hasAttribute("onclick") or - element.hasAttribute("tabindex") or element.getAttribute("role")?.toLowerCase() in ["button", "link"] or element.getAttribute("class")?.toLowerCase().indexOf("button") >= 0 or element.getAttribute("contentEditable")?.toLowerCase() in ["", "contentEditable", "true"]) @@ -180,10 +180,15 @@ LinkHints = when "button", "select" isClickable = true unless element.disabled + # 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. + if element.hasAttribute("tabindex") and not isClickable + isClickable = onlyHasTabIndex = true + continue unless isClickable # If the element isn't clickable, do nothing. clientRect = DomUtils.getVisibleClientRect element if clientRect != null - visibleElements.push {element: element, rect: clientRect} + visibleElements.push {element: element, rect: clientRect, onlyHasTabIndex: onlyHasTabIndex} # TODO(mrmr1993): Consider z-index. z-index affects behviour as follows: # * The document has a local stacking context. @@ -209,10 +214,10 @@ LinkHints = nonOverlappingElements.push {element: visibleElement.element, rect: rects[0]} else # Every part of the element is covered by some other element, so just insert the whole element's - # rect. + # rect. Except for elements with tabIndex set; these are often more trouble than they're worth. # TODO(mrmr1993): This is probably the wrong thing to do, but we don't want to stop being able to # click some elements that we could click before. - nonOverlappingElements.push visibleElement + nonOverlappingElements.push visibleElement unless visibleElement.onlyHasTabIndex nonOverlappingElements -- cgit v1.2.3 From 1059f98d5c9a552b2fa3fbcdddc7e44d0676056e Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Fri, 19 Dec 2014 01:10:41 +0000 Subject: Detect aria properties for disabling/hiding elements in link hints --- content_scripts/link_hints.coffee | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 6934e5b8..fa7fa937 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -152,6 +152,11 @@ LinkHints = areaRects = DomUtils.getClientRectsForAreas imgClientRects[0], areas visibleElements = visibleElements.concat areaRects + # Check aria properties to see if the element should be ignored. + if (element.getAttribute("aria-hidden")?.toLowerCase() in ["", "true"] or + element.getAttribute("aria-disabled")?.toLowerCase() in ["", "true"]) + continue # No point continuing the loop; this element should never have a link hint + # Check for attributes that make an element clickable regardless of its tagName. if (element.hasAttribute("onclick") or element.getAttribute("role")?.toLowerCase() in ["button", "link"] or -- cgit v1.2.3 From 3a688f754ebd647ce56b33d18c5744759c5efe95 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Sat, 20 Dec 2014 16:18:28 +0000 Subject: Use ||= to not ignore some clickable elements, no negative tabindex Elements with `tabindex="n"` for parseInt(n) < 0 cannot be selected by pressing the tab key, according to the spec. If we have no other reason to suspect that the element is clickable, we may as well ignore them. --- content_scripts/link_hints.coffee | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index fa7fa937..9eb7b87c 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -176,18 +176,19 @@ LinkHints = when "a" isClickable = true when "textarea" - isClickable = not element.disabled and not element.readOnly + isClickable ||= not element.disabled and not element.readOnly when "input" - unless element.getAttribute("type")?.toLowerCase() == "hidden" or - element.disabled or - (element.readOnly and DomUtils.isSelectable element) - isClickable = true + isClickable ||= not (element.getAttribute("type")?.toLowerCase() == "hidden" or + element.disabled or + (element.readOnly and DomUtils.isSelectable element)) when "button", "select" - isClickable = true unless element.disabled + isClickable ||= not element.disabled # 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. - if element.hasAttribute("tabindex") and not isClickable + tabIndexValue = element.getAttribute("tabindex") + tabIndex = if tabIndexValue == "" then 0 else parseInt tabIndexValue + unless isClickable or isNaN(tabIndex) or tabIndex < 0 isClickable = onlyHasTabIndex = true continue unless isClickable # If the element isn't clickable, do nothing. -- cgit v1.2.3 From 15d094675c4e354becf58b5f66754dd939d5daeb Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Mon, 22 Dec 2014 11:54:49 +0000 Subject: Rename a poorly named variable --- content_scripts/link_hints.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 9eb7b87c..616d40ee 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -149,8 +149,8 @@ LinkHints = map = document.querySelector "map[name=\"#{mapName}\"]" if map and imgClientRects.length > 0 areas = map.getElementsByTagName "area" - areaRects = DomUtils.getClientRectsForAreas imgClientRects[0], areas - visibleElements = visibleElements.concat areaRects + areasAndRects = DomUtils.getClientRectsForAreas imgClientRects[0], areas + visibleElements = visibleElements.concat areasAndRects # Check aria properties to see if the element should be ignored. if (element.getAttribute("aria-hidden")?.toLowerCase() in ["", "true"] or -- cgit v1.2.3 From 76af56da84753163adc4dbf943374a10f0cb8321 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Mon, 22 Dec 2014 11:56:18 +0000 Subject: Use push with a splat rather than concat --- content_scripts/link_hints.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 616d40ee..c4bf3f96 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -150,7 +150,7 @@ LinkHints = if map and imgClientRects.length > 0 areas = map.getElementsByTagName "area" areasAndRects = DomUtils.getClientRectsForAreas imgClientRects[0], areas - visibleElements = visibleElements.concat areasAndRects + visibleElements.push areasAndRects... # Check aria properties to see if the element should be ignored. if (element.getAttribute("aria-hidden")?.toLowerCase() in ["", "true"] or -- cgit v1.2.3 From c1ffbc88ed1e340a7a046e1d75499642bf220e7f Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Mon, 22 Dec 2014 12:35:13 +0000 Subject: Prefer `||=` to `= true if` --- content_scripts/link_hints.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index c4bf3f96..df442ea6 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -169,7 +169,7 @@ LinkHints = jsactionRules = element.getAttribute("jsaction").split(";") for jsactionRule in jsactionRules ruleSplit = jsactionRule.split ":" - isClickable = true if ruleSplit[0] == "click" or (ruleSplit.length == 1 and ruleSplit[0] != "none") + isClickable ||= ruleSplit[0] == "click" or (ruleSplit.length == 1 and ruleSplit[0] != "none") # Check for tagNames which are natively clickable. switch tagName -- cgit v1.2.3 From 278820f544d201b975a73ebe82d4a67792b967df Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Mon, 22 Dec 2014 12:39:47 +0000 Subject: Use a splat instead of apply --- content_scripts/link_hints.coffee | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index df442ea6..3a9e2027 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -215,7 +215,8 @@ LinkHints = while visibleElement = visibleElements.pop() rects = [visibleElement.rect] for {rect: negativeRect} in visibleElements - rects = Array::concat.apply [], (rects.map (rect) -> Rect.subtract rect, negativeRect) + # Subtract negativeRect from every rect in rects, and concatenate the arrays of rects that result. + rects = [].concat (rects.map (rect) -> Rect.subtract rect, negativeRect)... if rects.length > 0 nonOverlappingElements.push {element: visibleElement.element, rect: rects[0]} else -- cgit v1.2.3 From a78d49c8a9cac57492f78a90246ce7695cf8e036 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Mon, 29 Dec 2014 11:26:30 +0000 Subject: Add a comment clarifying why we no longer use XPath for link hints --- content_scripts/link_hints.coffee | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 3a9e2027..b605c2ec 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -135,6 +135,11 @@ LinkHints = elements = document.documentElement.getElementsByTagName "*" visibleElements = [] + # The order of elements here is important; they should appear in the order they are in the DOM, so that + # we can work out which element is on top when multiple elements overlap. Detecting elements in this loop + # is the sensible, efficient way to ensure this happens. + # NOTE(mrmr1993): Our previous method (combined XPath and DOM traversal for jsaction) couldn't provide + # this, so it's necessary to check whether elements are clickable in order, as we do below. for element in elements tagName = element.tagName.toLowerCase() isClickable = false -- cgit v1.2.3 From da04ee17472177b7ae0474712090d0604db2556e Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Mon, 29 Dec 2014 11:46:03 +0000 Subject: Move link hint clickable element detection to its own function --- content_scripts/link_hints.coffee | 127 ++++++++++++++++++++------------------ 1 file changed, 68 insertions(+), 59 deletions(-) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index b605c2ec..ea4be397 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -124,6 +124,72 @@ LinkHints = marker + # + # Determine whether the element is visible and clickable. If it is, return the element and the rect + # bounding the element in the viewport. + getVisibleClickable: (element) -> + tagName = element.tagName.toLowerCase() + isClickable = false + onlyHasTabIndex = false + + # Insert area elements that provide click functionality to an img. + if tagName == "img" + mapName = element.getAttribute "usemap" + if mapName + imgClientRects = element.getClientRects() + mapName = mapName.replace(/^#/, "").replace("\"", "\\\"") + map = document.querySelector "map[name=\"#{mapName}\"]" + if map and imgClientRects.length > 0 + areas = map.getElementsByTagName "area" + areasAndRects = DomUtils.getClientRectsForAreas imgClientRects[0], areas + visibleElements.push areasAndRects... + + # Check aria properties to see if the element should be ignored. + if (element.getAttribute("aria-hidden")?.toLowerCase() in ["", "true"] or + element.getAttribute("aria-disabled")?.toLowerCase() in ["", "true"]) + return null # This element should never have a link hint. + + # Check for attributes that make an element clickable regardless of its tagName. + if (element.hasAttribute("onclick") or + element.getAttribute("role")?.toLowerCase() in ["button", "link"] or + element.getAttribute("class")?.toLowerCase().indexOf("button") >= 0 or + element.getAttribute("contentEditable")?.toLowerCase() in ["", "contentEditable", "true"]) + isClickable = true + + # Check for jsaction event listeners on the element. + if element.hasAttribute "jsaction" + jsactionRules = element.getAttribute("jsaction").split(";") + for jsactionRule in jsactionRules + ruleSplit = jsactionRule.split ":" + isClickable ||= ruleSplit[0] == "click" or (ruleSplit.length == 1 and ruleSplit[0] != "none") + + # Check for tagNames which are natively clickable. + switch tagName + when "a" + isClickable = true + when "textarea" + isClickable ||= not element.disabled and not element.readOnly + when "input" + isClickable ||= not (element.getAttribute("type")?.toLowerCase() == "hidden" or + element.disabled or + (element.readOnly and DomUtils.isSelectable element)) + when "button", "select" + isClickable ||= not element.disabled + + # 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. + tabIndexValue = element.getAttribute("tabindex") + tabIndex = if tabIndexValue == "" then 0 else parseInt tabIndexValue + unless isClickable or isNaN(tabIndex) or tabIndex < 0 + isClickable = onlyHasTabIndex = true + + return null unless isClickable # The element isn't clickable. + clientRect = DomUtils.getVisibleClientRect element + if clientRect == null + null + else + {element: element, rect: clientRect, onlyHasTabIndex: onlyHasTabIndex} + # # Returns all clickable elements that are not hidden and are in the current viewport, along with rectangles # at which (parts of) the elements are displayed. @@ -141,65 +207,8 @@ LinkHints = # NOTE(mrmr1993): Our previous method (combined XPath and DOM traversal for jsaction) couldn't provide # this, so it's necessary to check whether elements are clickable in order, as we do below. for element in elements - tagName = element.tagName.toLowerCase() - isClickable = false - onlyHasTabIndex = false - - # Insert area elements that provide click functionality to an img. - if tagName == "img" - mapName = element.getAttribute "usemap" - if mapName - imgClientRects = element.getClientRects() - mapName = mapName.replace(/^#/, "").replace("\"", "\\\"") - map = document.querySelector "map[name=\"#{mapName}\"]" - if map and imgClientRects.length > 0 - areas = map.getElementsByTagName "area" - areasAndRects = DomUtils.getClientRectsForAreas imgClientRects[0], areas - visibleElements.push areasAndRects... - - # Check aria properties to see if the element should be ignored. - if (element.getAttribute("aria-hidden")?.toLowerCase() in ["", "true"] or - element.getAttribute("aria-disabled")?.toLowerCase() in ["", "true"]) - continue # No point continuing the loop; this element should never have a link hint - - # Check for attributes that make an element clickable regardless of its tagName. - if (element.hasAttribute("onclick") or - element.getAttribute("role")?.toLowerCase() in ["button", "link"] or - element.getAttribute("class")?.toLowerCase().indexOf("button") >= 0 or - element.getAttribute("contentEditable")?.toLowerCase() in ["", "contentEditable", "true"]) - isClickable = true - - # Check for jsaction event listeners on the element. - if element.hasAttribute "jsaction" - jsactionRules = element.getAttribute("jsaction").split(";") - for jsactionRule in jsactionRules - ruleSplit = jsactionRule.split ":" - isClickable ||= ruleSplit[0] == "click" or (ruleSplit.length == 1 and ruleSplit[0] != "none") - - # Check for tagNames which are natively clickable. - switch tagName - when "a" - isClickable = true - when "textarea" - isClickable ||= not element.disabled and not element.readOnly - when "input" - isClickable ||= not (element.getAttribute("type")?.toLowerCase() == "hidden" or - element.disabled or - (element.readOnly and DomUtils.isSelectable element)) - when "button", "select" - isClickable ||= not element.disabled - - # 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. - tabIndexValue = element.getAttribute("tabindex") - tabIndex = if tabIndexValue == "" then 0 else parseInt tabIndexValue - unless isClickable or isNaN(tabIndex) or tabIndex < 0 - isClickable = onlyHasTabIndex = true - - continue unless isClickable # If the element isn't clickable, do nothing. - clientRect = DomUtils.getVisibleClientRect element - if clientRect != null - visibleElements.push {element: element, rect: clientRect, onlyHasTabIndex: onlyHasTabIndex} + visibleElement = @getVisibleClickable element + visibleElements.push visibleElement if visibleElement? # TODO(mrmr1993): Consider z-index. z-index affects behviour as follows: # * The document has a local stacking context. -- cgit v1.2.3 From ecdf878c890bcc4ac67d2bb147dcca2e5c20dd27 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Mon, 29 Dec 2014 18:01:42 +0000 Subject: Return an array from getVisibleClickable, to restore img map support --- content_scripts/link_hints.coffee | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index ea4be397..70e6a626 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -131,6 +131,7 @@ LinkHints = tagName = element.tagName.toLowerCase() isClickable = false onlyHasTabIndex = false + visibleElements = [] # Insert area elements that provide click functionality to an img. if tagName == "img" @@ -183,12 +184,12 @@ LinkHints = unless isClickable or isNaN(tabIndex) or tabIndex < 0 isClickable = onlyHasTabIndex = true - return null unless isClickable # The element isn't clickable. - clientRect = DomUtils.getVisibleClientRect element - if clientRect == null - null - else - {element: element, rect: clientRect, onlyHasTabIndex: onlyHasTabIndex} + if isClickable + clientRect = DomUtils.getVisibleClientRect element + if clientRect != null + visibleElements.push {element: element, rect: clientRect, onlyHasTabIndex: onlyHasTabIndex} + + visibleElements # # Returns all clickable elements that are not hidden and are in the current viewport, along with rectangles @@ -208,7 +209,7 @@ LinkHints = # this, so it's necessary to check whether elements are clickable in order, as we do below. for element in elements visibleElement = @getVisibleClickable element - visibleElements.push visibleElement if visibleElement? + visibleElements.push visibleElement... # TODO(mrmr1993): Consider z-index. z-index affects behviour as follows: # * The document has a local stacking context. -- cgit v1.2.3 From c3df7699527f88c660e0d61fafdd1ad334236d77 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Tue, 30 Dec 2014 15:09:53 +0000 Subject: Minor changes to link-hint code. --- content_scripts/link_hints.coffee | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 70e6a626..8d476529 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -125,8 +125,10 @@ LinkHints = marker # - # Determine whether the element is visible and clickable. If it is, return the element and the rect - # bounding the element in the viewport. + # Determine whether the element is visible and clickable. If it is, return the element and the rect bounding + # the element in the viewport. There may be more than one part of element which is clickable (for example, + # if it's an image), therefore we return a list of element/rect pairs. + # getVisibleClickable: (element) -> tagName = element.tagName.toLowerCase() isClickable = false @@ -148,7 +150,7 @@ LinkHints = # Check aria properties to see if the element should be ignored. if (element.getAttribute("aria-hidden")?.toLowerCase() in ["", "true"] or element.getAttribute("aria-disabled")?.toLowerCase() in ["", "true"]) - return null # This element should never have a link hint. + return [] # This element should never have a link hint. # Check for attributes that make an element clickable regardless of its tagName. if (element.hasAttribute("onclick") or @@ -187,7 +189,7 @@ LinkHints = if isClickable clientRect = DomUtils.getVisibleClientRect element if clientRect != null - visibleElements.push {element: element, rect: clientRect, onlyHasTabIndex: onlyHasTabIndex} + visibleElements.push {element: element, rect: clientRect, secondClassCitizen: onlyHasTabIndex} visibleElements @@ -236,10 +238,11 @@ LinkHints = nonOverlappingElements.push {element: visibleElement.element, rect: rects[0]} else # Every part of the element is covered by some other element, so just insert the whole element's - # rect. Except for elements with tabIndex set; these are often more trouble than they're worth. + # rect. Except for elements with tabIndex set (second class citizens); these are often more trouble + # than they're worth. # TODO(mrmr1993): This is probably the wrong thing to do, but we don't want to stop being able to # click some elements that we could click before. - nonOverlappingElements.push visibleElement unless visibleElement.onlyHasTabIndex + nonOverlappingElements.push visibleElement unless visibleElement.secondClassCitizen nonOverlappingElements -- cgit v1.2.3 From 8f998f5b4cd1d8600b62ae7faac8afb91c4d2dab Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Tue, 30 Dec 2014 15:59:26 +0000 Subject: Update comment in getVisibleClickable. --- content_scripts/link_hints.coffee | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'content_scripts/link_hints.coffee') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 8d476529..9f21d109 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -125,9 +125,9 @@ LinkHints = marker # - # Determine whether the element is visible and clickable. If it is, return the element and the rect bounding - # the element in the viewport. There may be more than one part of element which is clickable (for example, - # if it's an image), therefore we return a list of element/rect pairs. + # Determine whether the element is visible and clickable. If it is, find the rect bounding the element in + # the viewport. There may be more than one part of element which is clickable (for example, if it's an + # image), therefore we always return a array of element/rect pairs (which may also be a singleton or empty). # getVisibleClickable: (element) -> tagName = element.tagName.toLowerCase() -- cgit v1.2.3