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 ++-------------- lib/dom_utils.coffee | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 14 deletions(-) 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}) diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee index a0ac0bd3..26fa9b81 100644 --- a/lib/dom_utils.coffee +++ b/lib/dom_utils.coffee @@ -41,6 +41,42 @@ DomUtils = if (namespace == "xhtml") then "http://www.w3.org/1999/xhtml" else null document.evaluate(xpath, document.documentElement, namespaceResolver, resultType, null) + # + # Returns all the clickable element children of contextNode. This also can include contextNode itself. + # + getClickableElements: (contextNode = document.documentElement) -> + elements = Array::slice.call(contextNode?.getElementsByTagName "*") + elements.unshift contextNode # Check the contextNode as well. + clickableElements = [] + 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 == "area" + element.hasAttribute "href" + else if (tagName == "input" and DomUtils.isSelectable element) or tagName == "textarea" + not (element.disabled or element.hasAttribute "readonly") + else if (tagName == "input" and element.getAttribute("type")?.toLowerCase() != "hidden") or + tagName in ["button", "select"] + not element.disabled + else + false + )() + clickableElements.push element if isClickable + clickableElements + # # Returns the first visible clientRect of an element if it exists. Otherwise it returns null. # -- cgit v1.2.3 From c80ad2c367f873f2b2547b60cebe49715a85ffe4 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Wed, 17 Dec 2014 10:40:02 +0000 Subject: Treat area elements as being at the point of their img element --- lib/dom_utils.coffee | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee index 26fa9b81..152a378e 100644 --- a/lib/dom_utils.coffee +++ b/lib/dom_utils.coffee @@ -64,8 +64,13 @@ DomUtils = true else if tagName == "a" true - else if tagName == "area" - element.hasAttribute "href" + else if tagName == "img" + mapName = element.getAttribute "usemap" + if mapName + map = document.querySelector(mapName.replace /^#/, "") + areas = Array::slice.call(map.getElementsByTagName "area") + elements.concat areas + false else if (tagName == "input" and DomUtils.isSelectable element) or tagName == "textarea" not (element.disabled or element.hasAttribute "readonly") else if (tagName == "input" and element.getAttribute("type")?.toLowerCase() != "hidden") or -- cgit v1.2.3 From 8c8ec835d673f0ec1cce242cf26cca077c845064 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Wed, 17 Dec 2014 11:08:07 +0000 Subject: Use element.readOnly instead of getAttribute "readonly" --- lib/dom_utils.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee index 152a378e..46bf3639 100644 --- a/lib/dom_utils.coffee +++ b/lib/dom_utils.coffee @@ -72,7 +72,7 @@ DomUtils = elements.concat areas false else if (tagName == "input" and DomUtils.isSelectable element) or tagName == "textarea" - not (element.disabled or element.hasAttribute "readonly") + not (element.disabled or element.readOnly) else if (tagName == "input" and element.getAttribute("type")?.toLowerCase() != "hidden") or tagName in ["button", "select"] not element.disabled -- 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 +++++++++++++++++++++++++++++++++- lib/dom_utils.coffee | 41 --------------------------------------- 2 files changed, 35 insertions(+), 42 deletions(-) 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 = [] diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee index 46bf3639..a0ac0bd3 100644 --- a/lib/dom_utils.coffee +++ b/lib/dom_utils.coffee @@ -41,47 +41,6 @@ DomUtils = if (namespace == "xhtml") then "http://www.w3.org/1999/xhtml" else null document.evaluate(xpath, document.documentElement, namespaceResolver, resultType, null) - # - # Returns all the clickable element children of contextNode. This also can include contextNode itself. - # - getClickableElements: (contextNode = document.documentElement) -> - elements = Array::slice.call(contextNode?.getElementsByTagName "*") - elements.unshift contextNode # Check the contextNode as well. - clickableElements = [] - 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") - elements.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 - )() - clickableElements.push element if isClickable - clickableElements - # # Returns the first visible clientRect of an element if it exists. Otherwise it returns null. # -- 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(-) 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 ++++++++++++--------------------------- lib/dom_utils.coffee | 28 +++++++++++++++++++++ 2 files changed, 44 insertions(+), 35 deletions(-) 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 diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee index a0ac0bd3..3d7e805f 100644 --- a/lib/dom_utils.coffee +++ b/lib/dom_utils.coffee @@ -90,6 +90,34 @@ DomUtils = return childClientRect null + getClientRectsForAreas: (imgClientRect, areas) -> + rects = [] + for area in areas + coords = area.coords.split(",").map((coord) -> parseInt(coord, 10)) + shape = area.shape.toLowerCase() + if shape == "rect" + [x1, y1, x2, y2] = coords + else if shape == "circle" + [x, y, r] = coords + x1 = x - r + x2 = x + r + y1 = y - r + y2 = y + r + else # For polygons and unknown shapes, don't return a rectangle. + # TODO(mrmr1993): revisit this. + continue + + rect = + top: imgClientRect.top + y1 + left: imgClientRect.left + x1 + right: imgClientRect.left + x2 + bottom: imgClientRect.top + y2 + width: x2 - x1 + height: y2 - y1 + + rects.push {element: area, rect: rect} unless isNaN rect.top + rects + # # Selectable means that we should use the simulateSelect method to activate the element instead of a click. # -- cgit v1.2.3 From 62686e83d690919f00afe1ac7f5955cecb1d2b2f Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Wed, 17 Dec 2014 12:31:09 +0000 Subject: Try to make image map rectangles work better --- lib/dom_utils.coffee | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee index 3d7e805f..842dda0f 100644 --- a/lib/dom_utils.coffee +++ b/lib/dom_utils.coffee @@ -95,17 +95,18 @@ DomUtils = for area in areas coords = area.coords.split(",").map((coord) -> parseInt(coord, 10)) shape = area.shape.toLowerCase() - if shape == "rect" + if shape == "rect" or coords.length == 4 [x1, y1, x2, y2] = coords - else if shape == "circle" + else if shape == "circle" or coords.length == 3 [x, y, r] = coords x1 = x - r x2 = x + r y1 = y - r y2 = y + r - else # For polygons and unknown shapes, don't return a rectangle. - # TODO(mrmr1993): revisit this. - continue + else + # Just consider the rectangle surrounding the first two points in a polygon. It's possible to do + # something more sophisticated, but likely not worth the effort. + [x1, y1, x2, y2] = coords rect = top: imgClientRect.top + y1 -- cgit v1.2.3 From c2ab9aaa27b5fbf1f065743772c6f04dd3c5f39d Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Wed, 17 Dec 2014 12:39:22 +0000 Subject: Don't show link hints for offscreen image maps --- lib/dom_utils.coffee | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee index 842dda0f..8ade58bb 100644 --- a/lib/dom_utils.coffee +++ b/lib/dom_utils.coffee @@ -52,18 +52,9 @@ DomUtils = } for clientRect in element.getClientRects()) for clientRect in clientRects - if (clientRect.top < 0) - clientRect.height += clientRect.top - clientRect.top = 0 + clientRect = @cropRectToVisible clientRect - if (clientRect.left < 0) - clientRect.width += clientRect.left - clientRect.left = 0 - - if (clientRect.top >= window.innerHeight - 4 || clientRect.left >= window.innerWidth - 4) - continue - - if (clientRect.width < 3 || clientRect.height < 3) + if (!clientRect || clientRect.width < 3 || clientRect.height < 3) continue # eliminate invisible elements (see test_harnesses/visibility_test.html) @@ -90,6 +81,21 @@ DomUtils = return childClientRect null + cropRectToVisible: (rect) -> + if (rect.top < 0) + rect.height += rect.top + rect.top = 0 + + if (rect.left < 0) + rect.width += rect.left + rect.left = 0 + + if (rect.top >= window.innerHeight - 4 || rect.left >= window.innerWidth - 4) + null + else + rect + + getClientRectsForAreas: (imgClientRect, areas) -> rects = [] for area in areas @@ -108,7 +114,7 @@ DomUtils = # something more sophisticated, but likely not worth the effort. [x1, y1, x2, y2] = coords - rect = + rect = @cropRectToVisible top: imgClientRect.top + y1 left: imgClientRect.left + x1 right: imgClientRect.left + x2 @@ -116,7 +122,7 @@ DomUtils = width: x2 - x1 height: y2 - y1 - rects.push {element: area, rect: rect} unless isNaN rect.top + rects.push {element: area, rect: rect} unless not rect or isNaN rect.top rects # -- 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(-) 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 833942ae06f680bc1949a7bced4719b707950568 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Wed, 17 Dec 2014 12:47:38 +0000 Subject: Stop ignoring clickable opacity: none; elements Some websites (notably Facebook) use `opacity: none;` to show an image in the place of a less-customisable element (eg. an ``). To not show link hints for such transparent elements is confusing and often the wrong thing to do. --- lib/dom_utils.coffee | 3 +-- tests/dom_tests/dom_utils_test.coffee | 6 ------ 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee index 8ade58bb..1e2cc812 100644 --- a/lib/dom_utils.coffee +++ b/lib/dom_utils.coffee @@ -60,8 +60,7 @@ DomUtils = # eliminate invisible elements (see test_harnesses/visibility_test.html) computedStyle = window.getComputedStyle(element, null) if (computedStyle.getPropertyValue('visibility') != 'visible' || - computedStyle.getPropertyValue('display') == 'none' || - computedStyle.getPropertyValue('opacity') == '0') + computedStyle.getPropertyValue('display') == 'none') continue return clientRect diff --git a/tests/dom_tests/dom_utils_test.coffee b/tests/dom_tests/dom_utils_test.coffee index 130a3014..ad8bde3c 100644 --- a/tests/dom_tests/dom_utils_test.coffee +++ b/tests/dom_tests/dom_utils_test.coffee @@ -50,12 +50,6 @@ context "Check visibility", assert.isTrue (DomUtils.getVisibleClientRect document.getElementById 'foo') != null assert.isTrue (DomUtils.getVisibleClientRect document.getElementById 'bar') != null - should "detect opacity:0 links as hidden", -> - document.getElementById("test-div").innerHTML = """ - test - """ - assert.equal null, DomUtils.getVisibleClientRect document.getElementById 'foo' - should "detect links that contain only floated / absolutely-positioned divs as visible", -> document.getElementById("test-div").innerHTML = """ -- cgit v1.2.3 From 158b3f09fd222b0e93510dc17521833de73bcf88 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Wed, 17 Dec 2014 13:49:29 +0000 Subject: Unify two loops into one --- lib/dom_utils.coffee | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee index 1e2cc812..aaa93923 100644 --- a/lib/dom_utils.coffee +++ b/lib/dom_utils.coffee @@ -51,20 +51,6 @@ DomUtils = width: clientRect.width, height: clientRect.height } for clientRect in element.getClientRects()) - for clientRect in clientRects - clientRect = @cropRectToVisible clientRect - - if (!clientRect || clientRect.width < 3 || clientRect.height < 3) - continue - - # eliminate invisible elements (see test_harnesses/visibility_test.html) - computedStyle = window.getComputedStyle(element, null) - if (computedStyle.getPropertyValue('visibility') != 'visible' || - computedStyle.getPropertyValue('display') == 'none') - continue - - return clientRect - for clientRect in clientRects # If the link has zero dimensions, it may be wrapping visible # but floated elements. Check for this. @@ -78,6 +64,21 @@ DomUtils = childClientRect = @getVisibleClientRect(child) continue if (childClientRect == null) return childClientRect + + else + clientRect = @cropRectToVisible clientRect + + if (!clientRect || clientRect.width < 3 || clientRect.height < 3) + continue + + # eliminate invisible elements (see test_harnesses/visibility_test.html) + computedStyle = window.getComputedStyle(element, null) + if (computedStyle.getPropertyValue('visibility') != 'visible' || + computedStyle.getPropertyValue('display') == 'none') + continue + + return clientRect + null cropRectToVisible: (rect) -> -- 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 +++++++++++++++++++- lib/dom_utils.coffee | 9 ++---- lib/utils.coffee | 60 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 8 deletions(-) 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. diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee index aaa93923..7e19a7fc 100644 --- a/lib/dom_utils.coffee +++ b/lib/dom_utils.coffee @@ -114,13 +114,8 @@ DomUtils = # something more sophisticated, but likely not worth the effort. [x1, y1, x2, y2] = coords - rect = @cropRectToVisible - top: imgClientRect.top + y1 - left: imgClientRect.left + x1 - right: imgClientRect.left + x2 - bottom: imgClientRect.top + y2 - width: x2 - x1 - height: y2 - y1 + rect = Utils.shiftRect (Utils.createRect x1, y1, x2, y2), imgClientRect.left, imgClientRect.top + rect = @cropRectToVisible rect rects.push {element: area, rect: rect} unless not rect or isNaN rect.top rects diff --git a/lib/utils.coffee b/lib/utils.coffee index b7f8731a..6cc45f32 100644 --- a/lib/utils.coffee +++ b/lib/utils.coffee @@ -136,6 +136,66 @@ Utils = # locale-sensitive uppercase detection hasUpperCase: (s) -> s.toLowerCase() != s + # Create a rect given the top left and bottom right corners. + createRect: (x1, y1, x2, y2) -> + bottom: y2 + top: y1 + left: x1 + right: x2 + width: x2 - x1 + height: y2 - y1 + + # Translate a rect by x horizontally and y vertically. + shiftRect: (rect, x, y) -> + bottom: rect.bottom + y + top: rect.top + y + left: rect.left + x + right: rect.right + x + width: rect.width + height: rect.height + + # Subtract rect2 from rect1, returning an array of rects which are in rect1 but not rect2. + subtractRect: (rect1, rect2_) -> + # Bound rect2 by rect1 + rect2 = {} + rect2 = @createRect( + Math.max(rect1.left, rect2_.left), + Math.max(rect1.top, rect2_.top), + Math.min(rect1.right, rect2_.right), + Math.min(rect1.bottom, rect2_.bottom) + ) + + # If bounding rect2 has made the width or height negative, rect1 does not contain rect2. + return [rect1] if rect2.width < 0 or rect2.height < 0 + + # + # All the possible rects, in the order + # +-+-+-+ + # |1|2|3| + # +-+-+-+ + # |4| |5| + # +-+-+-+ + # |6|7|8| + # +-+-+-+ + # where the outer rectangle is rect1 and the inner rectangle is rect 2. Note that the rects may be of + # width or height 0. + # + rects = [ + # Top row. + @createRect rect1.left, rect1.top, rect2.left, rect2.top + @createRect rect2.left, rect1.top, rect2.right, rect2.top + @createRect rect2.right, rect1.top, rect1.right, rect2.top + # Middle row. + @createRect rect1.left, rect2.top, rect2.left, rect2.bottom + @createRect rect2.right, rect2.top, rect1.right, rect2.bottom + # Bottom row. + @createRect rect1.left, rect2.bottom, rect2.left, rect1.bottom + @createRect rect2.left, rect2.bottom, rect2.right, rect1.bottom + @createRect rect2.right, rect2.bottom, rect1.right, rect1.bottom + ] + + rects.filter (rect) -> rect.height > 0 and rect.width > 0 + # This creates a new function out of an existing function, where the new function takes fewer arguments. This # allows us to pass around functions instead of functions + a partial list of arguments. Function::curry = -> -- 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(+) 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(-) 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 +- lib/dom_utils.coffee | 3 +- lib/rect.coffee | 64 +++++++++++++++++++++++++++++++++++++++ lib/utils.coffee | 60 ------------------------------------ manifest.json | 1 + tests/dom_tests/dom_tests.html | 1 + 6 files changed, 68 insertions(+), 63 deletions(-) create mode 100644 lib/rect.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 diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee index 7e19a7fc..ebbed006 100644 --- a/lib/dom_utils.coffee +++ b/lib/dom_utils.coffee @@ -95,7 +95,6 @@ DomUtils = else rect - getClientRectsForAreas: (imgClientRect, areas) -> rects = [] for area in areas @@ -114,7 +113,7 @@ DomUtils = # something more sophisticated, but likely not worth the effort. [x1, y1, x2, y2] = coords - rect = Utils.shiftRect (Utils.createRect x1, y1, x2, y2), imgClientRect.left, imgClientRect.top + rect = Rect.translate (Rect.create x1, y1, x2, y2), imgClientRect.left, imgClientRect.top rect = @cropRectToVisible rect rects.push {element: area, rect: rect} unless not rect or isNaN rect.top diff --git a/lib/rect.coffee b/lib/rect.coffee new file mode 100644 index 00000000..67c9de7c --- /dev/null +++ b/lib/rect.coffee @@ -0,0 +1,64 @@ +# Commands for manipulating rects. +Rect = + # Create a rect given the top left and bottom right corners. + create: (x1, y1, x2, y2) -> + bottom: y2 + top: y1 + left: x1 + right: x2 + width: x2 - x1 + height: y2 - y1 + + # Translate a rect by x horizontally and y vertically. + translate: (rect, x, y) -> + bottom: rect.bottom + y + top: rect.top + y + left: rect.left + x + right: rect.right + x + width: rect.width + height: rect.height + + # Subtract rect2 from rect1, returning an array of rects which are in rect1 but not rect2. + subtract: (rect1, rect2_) -> + # Bound rect2 by rect1 + rect2 = {} + rect2 = @create( + Math.max(rect1.left, rect2_.left), + Math.max(rect1.top, rect2_.top), + Math.min(rect1.right, rect2_.right), + Math.min(rect1.bottom, rect2_.bottom) + ) + + # If bounding rect2 has made the width or height negative, rect1 does not contain rect2. + return [rect1] if rect2.width < 0 or rect2.height < 0 + + # + # All the possible rects, in the order + # +-+-+-+ + # |1|2|3| + # +-+-+-+ + # |4| |5| + # +-+-+-+ + # |6|7|8| + # +-+-+-+ + # where the outer rectangle is rect1 and the inner rectangle is rect 2. Note that the rects may be of + # width or height 0. + # + rects = [ + # Top row. + @create rect1.left, rect1.top, rect2.left, rect2.top + @create rect2.left, rect1.top, rect2.right, rect2.top + @create rect2.right, rect1.top, rect1.right, rect2.top + # Middle row. + @create rect1.left, rect2.top, rect2.left, rect2.bottom + @create rect2.right, rect2.top, rect1.right, rect2.bottom + # Bottom row. + @create rect1.left, rect2.bottom, rect2.left, rect1.bottom + @create rect2.left, rect2.bottom, rect2.right, rect1.bottom + @create rect2.right, rect2.bottom, rect1.right, rect1.bottom + ] + + rects.filter (rect) -> rect.height > 0 and rect.width > 0 + +root = exports ? window +root.Rect = Rect diff --git a/lib/utils.coffee b/lib/utils.coffee index 6cc45f32..b7f8731a 100644 --- a/lib/utils.coffee +++ b/lib/utils.coffee @@ -136,66 +136,6 @@ Utils = # locale-sensitive uppercase detection hasUpperCase: (s) -> s.toLowerCase() != s - # Create a rect given the top left and bottom right corners. - createRect: (x1, y1, x2, y2) -> - bottom: y2 - top: y1 - left: x1 - right: x2 - width: x2 - x1 - height: y2 - y1 - - # Translate a rect by x horizontally and y vertically. - shiftRect: (rect, x, y) -> - bottom: rect.bottom + y - top: rect.top + y - left: rect.left + x - right: rect.right + x - width: rect.width - height: rect.height - - # Subtract rect2 from rect1, returning an array of rects which are in rect1 but not rect2. - subtractRect: (rect1, rect2_) -> - # Bound rect2 by rect1 - rect2 = {} - rect2 = @createRect( - Math.max(rect1.left, rect2_.left), - Math.max(rect1.top, rect2_.top), - Math.min(rect1.right, rect2_.right), - Math.min(rect1.bottom, rect2_.bottom) - ) - - # If bounding rect2 has made the width or height negative, rect1 does not contain rect2. - return [rect1] if rect2.width < 0 or rect2.height < 0 - - # - # All the possible rects, in the order - # +-+-+-+ - # |1|2|3| - # +-+-+-+ - # |4| |5| - # +-+-+-+ - # |6|7|8| - # +-+-+-+ - # where the outer rectangle is rect1 and the inner rectangle is rect 2. Note that the rects may be of - # width or height 0. - # - rects = [ - # Top row. - @createRect rect1.left, rect1.top, rect2.left, rect2.top - @createRect rect2.left, rect1.top, rect2.right, rect2.top - @createRect rect2.right, rect1.top, rect1.right, rect2.top - # Middle row. - @createRect rect1.left, rect2.top, rect2.left, rect2.bottom - @createRect rect2.right, rect2.top, rect1.right, rect2.bottom - # Bottom row. - @createRect rect1.left, rect2.bottom, rect2.left, rect1.bottom - @createRect rect2.left, rect2.bottom, rect2.right, rect1.bottom - @createRect rect2.right, rect2.bottom, rect1.right, rect1.bottom - ] - - rects.filter (rect) -> rect.height > 0 and rect.width > 0 - # This creates a new function out of an existing function, where the new function takes fewer arguments. This # allows us to pass around functions instead of functions + a partial list of arguments. Function::curry = -> diff --git a/manifest.json b/manifest.json index 3cd88d1e..e5271692 100644 --- a/manifest.json +++ b/manifest.json @@ -35,6 +35,7 @@ "js": ["lib/utils.js", "lib/keyboard_utils.js", "lib/dom_utils.js", + "lib/rect.js", "lib/handler_stack.js", "lib/clipboard.js", "content_scripts/link_hints.js", diff --git a/tests/dom_tests/dom_tests.html b/tests/dom_tests/dom_tests.html index feddafac..d8232892 100644 --- a/tests/dom_tests/dom_tests.html +++ b/tests/dom_tests/dom_tests.html @@ -32,6 +32,7 @@ + -- cgit v1.2.3 From 91bb7d7b85df3b90882e92aeae2fa2021f61733e Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Thu, 18 Dec 2014 12:59:35 +0000 Subject: Add tests for lib/rect --- lib/rect.coffee | 34 ++++-- tests/unit_tests/rect_test.coffee | 232 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 258 insertions(+), 8 deletions(-) create mode 100644 tests/unit_tests/rect_test.coffee diff --git a/lib/rect.coffee b/lib/rect.coffee index 67c9de7c..adc1fc36 100644 --- a/lib/rect.coffee +++ b/lib/rect.coffee @@ -9,8 +9,16 @@ Rect = width: x2 - x1 height: y2 - y1 + copy: (rect) -> + bottom: rect.bottom + top: rect.top + left: rect.left + right: rect.right + width: rect.width + height: rect.height + # Translate a rect by x horizontally and y vertically. - translate: (rect, x, y) -> + translate: (rect, x = 0, y = 0) -> bottom: rect.bottom + y top: rect.top + y left: rect.left + x @@ -19,18 +27,17 @@ Rect = height: rect.height # Subtract rect2 from rect1, returning an array of rects which are in rect1 but not rect2. - subtract: (rect1, rect2_) -> + subtract: (rect1, rect2) -> # Bound rect2 by rect1 - rect2 = {} rect2 = @create( - Math.max(rect1.left, rect2_.left), - Math.max(rect1.top, rect2_.top), - Math.min(rect1.right, rect2_.right), - Math.min(rect1.bottom, rect2_.bottom) + Math.max(rect1.left, rect2.left), + Math.max(rect1.top, rect2.top), + Math.min(rect1.right, rect2.right), + Math.min(rect1.bottom, rect2.bottom) ) # If bounding rect2 has made the width or height negative, rect1 does not contain rect2. - return [rect1] if rect2.width < 0 or rect2.height < 0 + return [Rect.copy rect1] if rect2.width < 0 or rect2.height < 0 # # All the possible rects, in the order @@ -60,5 +67,16 @@ Rect = rects.filter (rect) -> rect.height > 0 and rect.width > 0 + contains: (rect1, rect2) -> + rect1.right > rect2.left and + rect1.left < rect2.right and + rect1.bottom > rect2.top and + rect1.top < rect2.bottom + + equals: (rect1, rect2) -> + for property in ["top", "bottom", "left", "right", "width", "height"] + return false if rect1[property] != rect2[property] + true + root = exports ? window root.Rect = Rect diff --git a/tests/unit_tests/rect_test.coffee b/tests/unit_tests/rect_test.coffee new file mode 100644 index 00000000..cfb26b05 --- /dev/null +++ b/tests/unit_tests/rect_test.coffee @@ -0,0 +1,232 @@ +require "./test_helper.js" +extend(global, require "../../lib/rect.js") + +context "Rect", + should "set rect properties correctly", -> + [x1, y1, x2, y2] = [1, 2, 3, 4] + rect = Rect.create x1, y1, x2, y2 + assert.equal rect.left, x1 + assert.equal rect.top, y1 + assert.equal rect.right, x2 + assert.equal rect.bottom, y2 + assert.equal rect.width, x2 - x1 + assert.equal rect.height, y2 - y1 + + should "translate rect horizontally", -> + [x1, y1, x2, y2] = [1, 2, 3, 4] + x = 5 + rect1 = Rect.create x1, y1, x2, y2 + rect2 = Rect.translate rect1, x + + assert.equal rect1.left + x, rect2.left + assert.equal rect1.right + x, rect2.right + + assert.equal rect1.width, rect2.width + assert.equal rect1.height, rect2.height + assert.equal rect1.top, rect2.top + assert.equal rect1.bottom, rect2.bottom + + should "translate rect vertically", -> + [x1, y1, x2, y2] = [1, 2, 3, 4] + y = 5 + rect1 = Rect.create x1, y1, x2, y2 + rect2 = Rect.translate rect1, undefined, y + + assert.equal rect1.top + y, rect2.top + assert.equal rect1.bottom + y, rect2.bottom + + assert.equal rect1.width, rect2.width + assert.equal rect1.height, rect2.height + assert.equal rect1.left, rect2.left + assert.equal rect1.right, rect2.right + +context "Rect subtraction", + context "unchanged by rects outside", + should "left, above", -> + rect1 = Rect.create 0, 0, 1, 1 + rect2 = Rect.create -2, -2, -1, -1 + + rects = Rect.subtract rect1, rect2 + assert.equal rects.length, 1 + rect = rects[0] + assert.isTrue Rect.equals rect1, rect + + should "left", -> + rect1 = Rect.create 0, 0, 1, 1 + rect2 = Rect.create -2, 0, -1, 1 + + rects = Rect.subtract rect1, rect2 + assert.equal rects.length, 1 + rect = rects[0] + assert.isTrue Rect.equals rect1, rect + + should "left, below", -> + rect1 = Rect.create 0, 0, 1, 1 + rect2 = Rect.create -2, 2, -1, 3 + + rects = Rect.subtract rect1, rect2 + assert.equal rects.length, 1 + rect = rects[0] + assert.isTrue Rect.equals rect1, rect + + should "right, above", -> + rect1 = Rect.create 0, 0, 1, 1 + rect2 = Rect.create 2, -2, 3, -1 + + rects = Rect.subtract rect1, rect2 + assert.equal rects.length, 1 + rect = rects[0] + assert.isTrue Rect.equals rect1, rect + + should "right", -> + rect1 = Rect.create 0, 0, 1, 1 + rect2 = Rect.create 2, 0, 3, 1 + + rects = Rect.subtract rect1, rect2 + assert.equal rects.length, 1 + rect = rects[0] + assert.isTrue Rect.equals rect1, rect + + should "right, below", -> + rect1 = Rect.create 0, 0, 1, 1 + rect2 = Rect.create 2, 2, 3, 3 + + rects = Rect.subtract rect1, rect2 + assert.equal rects.length, 1 + rect = rects[0] + assert.isTrue Rect.equals rect1, rect + + should "above", -> + rect1 = Rect.create 0, 0, 1, 1 + rect2 = Rect.create 0, -2, 1, -1 + + rects = Rect.subtract rect1, rect2 + assert.equal rects.length, 1 + rect = rects[0] + assert.isTrue Rect.equals rect1, rect + + should "below", -> + rect1 = Rect.create 0, 0, 1, 1 + rect2 = Rect.create 0, 2, 1, 3 + + rects = Rect.subtract rect1, rect2 + assert.equal rects.length, 1 + rect = rects[0] + assert.isTrue Rect.equals rect1, rect + + context "unchanged by rects touching", + should "left, above", -> + rect1 = Rect.create 0, 0, 1, 1 + rect2 = Rect.create -1, -1, 0, 0 + + rects = Rect.subtract rect1, rect2 + assert.equal rects.length, 1 + rect = rects[0] + assert.isTrue Rect.equals rect1, rect + + should "left", -> + rect1 = Rect.create 0, 0, 1, 1 + rect2 = Rect.create -1, 0, 0, 1 + + rects = Rect.subtract rect1, rect2 + assert.equal rects.length, 1 + rect = rects[0] + assert.isTrue Rect.equals rect1, rect + + should "left, below", -> + rect1 = Rect.create 0, 0, 1, 1 + rect2 = Rect.create -1, 1, 0, 2 + + rects = Rect.subtract rect1, rect2 + assert.equal rects.length, 1 + rect = rects[0] + assert.isTrue Rect.equals rect1, rect + + should "right, above", -> + rect1 = Rect.create 0, 0, 1, 1 + rect2 = Rect.create 1, -1, 2, 0 + + rects = Rect.subtract rect1, rect2 + assert.equal rects.length, 1 + rect = rects[0] + assert.isTrue Rect.equals rect1, rect + + should "right", -> + rect1 = Rect.create 0, 0, 1, 1 + rect2 = Rect.create 1, 0, 2, 1 + + rects = Rect.subtract rect1, rect2 + assert.equal rects.length, 1 + rect = rects[0] + assert.isTrue Rect.equals rect1, rect + + should "right, below", -> + rect1 = Rect.create 0, 0, 1, 1 + rect2 = Rect.create 1, 1, 2, 2 + + rects = Rect.subtract rect1, rect2 + assert.equal rects.length, 1 + rect = rects[0] + assert.isTrue Rect.equals rect1, rect + + should "above", -> + rect1 = Rect.create 0, 0, 1, 1 + rect2 = Rect.create 0, -1, 1, 0 + + rects = Rect.subtract rect1, rect2 + assert.equal rects.length, 1 + rect = rects[0] + assert.isTrue Rect.equals rect1, rect + + should "below", -> + rect1 = Rect.create 0, 0, 1, 1 + rect2 = Rect.create 0, 1, 1, 2 + + rects = Rect.subtract rect1, rect2 + assert.equal rects.length, 1 + rect = rects[0] + assert.isTrue Rect.equals rect1, rect + + should "have nothing when subtracting itself", -> + rect = Rect.create 0, 0, 1, 1 + rects = Rect.subtract rect, rect + assert.equal rects.length, 0 + + should "not overlap subtracted rect", -> + rect = Rect.create 0, 0, 3, 3 + for x in [-2..2] + for y in [-2..2] + for width in [1..3] + for height in [1..3] + subtractRect = Rect.create x, y, (x + width), (y + height) + resultRects = Rect.subtract rect, subtractRect + for resultRect in resultRects + assert.isFalse Rect.contains subtractRect, resultRect + + should "be contained in original rect", -> + rect = Rect.create 0, 0, 3, 3 + for x in [-2..2] + for y in [-2..2] + for width in [1..3] + for height in [1..3] + subtractRect = Rect.create x, y, (x + width), (y + height) + resultRects = Rect.subtract rect, subtractRect + for resultRect in resultRects + assert.isTrue Rect.contains rect, resultRect + + should "contain the subtracted rect in the original minus the results", -> + rect = Rect.create 0, 0, 3, 3 + for x in [-2..2] + for y in [-2..2] + for width in [1..3] + for height in [1..3] + subtractRect = Rect.create x, y, (x + width), (y + height) + resultRects = Rect.subtract rect, subtractRect + resultComplement = [Rect.copy rect] + for resultRect in resultRects + resultComplement = Array::concat.apply [], + (resultComplement.map (rect) -> Rect.subtract rect, resultRect) + assert.isTrue (resultComplement.length == 0 or resultComplement.length == 1) + if resultComplement.length == 1 + complementRect = resultComplement[0] + assert.isTrue Rect.contains subtractRect, complementRect -- cgit v1.2.3 From ef863e5748c088f80ec9a0ffcaa06201c42e6c98 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Thu, 18 Dec 2014 13:23:25 +0000 Subject: Make some minor changes/tweaks to rect handling in dom_utils --- lib/dom_utils.coffee | 46 +++++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee index ebbed006..df1db3b9 100644 --- a/lib/dom_utils.coffee +++ b/lib/dom_utils.coffee @@ -68,8 +68,7 @@ DomUtils = else clientRect = @cropRectToVisible clientRect - if (!clientRect || clientRect.width < 3 || clientRect.height < 3) - continue + continue unless clientRect # eliminate invisible elements (see test_harnesses/visibility_test.html) computedStyle = window.getComputedStyle(element, null) @@ -81,33 +80,42 @@ DomUtils = null + # + # Bounds the rect by the current viewport dimensions. If the rect is offscreen or has a height or width < 3 + # then null is returned instead of a rect. + # cropRectToVisible: (rect) -> - if (rect.top < 0) - rect.height += rect.top - rect.top = 0 - - if (rect.left < 0) - rect.width += rect.left - rect.left = 0 - - if (rect.top >= window.innerHeight - 4 || rect.left >= window.innerWidth - 4) + boundedRect = Rect.create( + Math.max(rect.left, 0), + Math.max(rect.top, 0), + Math.min(rect.right, window.innerWidth), + Math.min(rect.bottom, window.innerHeight) + ) + if boundedRect.width < 3 or boundedRect.height < 3 null else - rect + boundedRect + # + # Get the client rects for the elements in a based on the position of the element using + # the map. Returns an array of rects. + # getClientRectsForAreas: (imgClientRect, areas) -> rects = [] for area in areas coords = area.coords.split(",").map((coord) -> parseInt(coord, 10)) shape = area.shape.toLowerCase() - if shape == "rect" or coords.length == 4 + if shape in ["rect", "rectangle"] # "rectangle" is an IE non-standard. [x1, y1, x2, y2] = coords - else if shape == "circle" or coords.length == 3 + else if shape in ["circle", "circ"] # "circ" is an IE non-standard. [x, y, r] = coords - x1 = x - r - x2 = x + r - y1 = y - r - y2 = y + r + diff = r / Math.sqrt 2 # Gives us an inner square + x1 = x - diff + x2 = x + diff + y1 = y - diff + y2 = y + diff + else if shape == "default" + [x1, y1, x2, y2] = [0, 0, imgClientRect.width, imgClientRect.height] else # Just consider the rectangle surrounding the first two points in a polygon. It's possible to do # something more sophisticated, but likely not worth the effort. @@ -116,7 +124,7 @@ DomUtils = rect = Rect.translate (Rect.create x1, y1, x2, y2), imgClientRect.left, imgClientRect.top rect = @cropRectToVisible rect - rects.push {element: area, rect: rect} unless not rect or isNaN rect.top + rects.push {element: area, rect: rect} if rect and not isNaN rect.top rects # -- cgit v1.2.3 From 845fd65e1c1a52329352f5068e3c7f0ef7b26154 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Thu, 18 Dec 2014 13:25:21 +0000 Subject: Use Rect.copy instead of literal member by member copy of a rect --- lib/dom_utils.coffee | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee index df1db3b9..7fd126b8 100644 --- a/lib/dom_utils.coffee +++ b/lib/dom_utils.coffee @@ -46,10 +46,7 @@ DomUtils = # getVisibleClientRect: (element) -> # Note: this call will be expensive if we modify the DOM in between calls. - clientRects = ({ - top: clientRect.top, right: clientRect.right, bottom: clientRect.bottom, left: clientRect.left, - width: clientRect.width, height: clientRect.height - } for clientRect in element.getClientRects()) + clientRects = (Rect.copy clientRect for clientRect in element.getClientRects()) for clientRect in clientRects # If the link has zero dimensions, it may be wrapping visible -- 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(-) 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(-) 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(-) 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(-) 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(+) 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(-) 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