Adds new check to see if an element is covered by another

(This message is written by @smblott-github.)

This is @poacher2k's philc#2251 rebased, tweaked and squashed into a single
commit.  It provides a new method of detecting when an overlay hides a
clickable element.

There is some possibility that this may have to be reverted (for
example, if we find we're missing too many important hints).  For that
reason, I squashed this into a single commit.

Closes philc#2251.

Adds another check to find out if elem is visible

Also adds 0.1 pixels inwards to corner coordinates to handle some edge
cases where another element would be found instead

Removes redundant contains-check

Reverts change that made tests fail

Tweak philc#2251 (better overlay detection).

Author: Daniel Skogly (poacher2k)

CREDITS

@@ -48,5 +48,6 @@ Contributors:
   Scott Pinkelman <scott@scottpinkelman.com> (github: sco-tt)
   Darryl Pogue <darryl@dpogue.ca> (github: dpogue)
   tobimensch
+  Daniel Skogly <daniel@pnkt.no> (github: poacher2k)
 
 Feel free to add real names in addition to GitHub usernames.

content_scripts/link_hints.coffee

@@ -558,7 +558,6 @@ LocalHints =
   getVisibleClickable: (element) ->
     tagName = element.tagName.toLowerCase()
     isClickable = false
-    onlyHasTabIndex = false
     possibleFalsePositive = false
     visibleElements = []
     reason = null
@@ -650,18 +649,17 @@ LocalHints =
     if not isClickable and 0 <= element.getAttribute("class")?.toLowerCase().indexOf "button"
       possibleFalsePositive = isClickable = true
 
-    # Elements with tabindex are sometimes useful, but usually not. We can treat them as second class
-    # citizens when it improves UX, so take special note of them.
-    tabIndexValue = element.getAttribute("tabindex")
-    tabIndex = if tabIndexValue == "" then 0 else parseInt tabIndexValue
-    unless isClickable or isNaN(tabIndex) or tabIndex < 0
-      isClickable = onlyHasTabIndex = true
+    isClickable ?= do ->
+      # Elements with tabindex are sometimes useful.
+      # NOTE(smblott) We should consider removing these, they are noise more often than not.
+      tabIndexValue = element.getAttribute "tabindex"
+      tabIndex = if tabIndexValue == "" then 0 else parseInt tabIndexValue
+      not isNaN(tabIndex) and 0 <= tabIndex
 
     if isClickable
-      clientRect = DomUtils.getVisibleClientRect element, true
-      if clientRect != null
-        visibleElements.push {element: element, rect: clientRect, secondClassCitizen: onlyHasTabIndex,
-          possibleFalsePositive, reason}
+      rect = DomUtils.getVisibleClientRect element, true
+      if rect != null
+        visibleElements.push {element, rect, possibleFalsePositive, reason}
 
     visibleElements
 
@@ -709,34 +707,26 @@ LocalHints =
           false # This is not a false positive.
         element
 
-    # TODO(mrmr1993): Consider z-index. z-index affects behaviour 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 elements where another clickable element lies above it.
-    localHints = nonOverlappingElements = []
-    while visibleElement = visibleElements.pop()
-      rects = [visibleElement.rect]
-      for {rect: negativeRect} in visibleElements
-        # 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 extend visibleElement, 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 (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.secondClassCitizen
+    # Only keep elements where one of the corners or the center is visible.
+    localHints = nonOverlappingElements =
+      for visibleElement in visibleElements
+        {rect, element} = visibleElement
+
+        # We use a small offset inwards to handle some edge cases where another element would be found
+        # instead.
+        xs = [rect.left + 0.1, rect.right - 0.1]
+        ys = [rect.top + 0.1, rect.bottom - 0.1]
+
+        # We check the center and then the four corners.
+        points = [[rect.left + (rect.width * 0.5), rect.top + (rect.height * 0.5)],
+          [xs[0], ys[0]], [xs[0], ys[1]], [xs[1], ys[0]], [xs[1], ys[1]]]
+
+        continue unless do ->
+          for [x, y] in points
+            return true if DomUtils.elementIsVisible element, x, y
+          false
+
+        visibleElement
 
     # Position the rects within the window.
     for hint in nonOverlappingElements

lib/dom_utils.coffee

@@ -178,6 +178,15 @@ DomUtils =
       rects.push {element: area, rect: rect} if rect and not isNaN rect.top
     rects
 
+  # Determine whether element is visible at point (x,y); that is, that no other element is overlaying it.
+  # document.elementFromPoint() finds an element at an x,y location.  Node.contains() checks whether an
+  # element contains another. Note that someNode.contains(someNode) is true.  If we do not find our element as
+  # a descendant of any element we find, then we assume that it's completely covered.
+  elementIsVisible: (element, x, y) ->
+    elementFromPoint = document.elementFromPoint x, y
+    elementFromPoint and
+      (element.contains(elementFromPoint) or elementFromPoint.contains element)
+
   #
   # Selectable means that we should use the simulateSelect method to activate the element instead of a click.
   #