Merge pull request #158 from hypothesis/import-latest-browser-range

Fix normalization of ranges that start at an element node with no children

h/static/scripts/annotator/anchoring/pdf.coffee

 seek = require('dom-seek')
 
-Annotator = require('annotator')
-xpathRange = Annotator.Range
+xpathRange = require('./range')
 
 html = require('./html')
 RenderingStates = require('../pdfjs-rendering-states')
@@ -244,7 +243,7 @@ exports.anchor = (root, selectors, options = {}) ->
 
 
 exports.describe = (root, range, options = {}) ->
-  
+
   range = new xpathRange.BrowserRange(range).normalize()
 
   startTextLayer = getNodeTextLayer(range.start)

h/static/scripts/annotator/anchoring/test/html-anchoring-fixture.html

@@ -22,3 +22,8 @@
 <p><span><!--anchor--></span><!--after-->
   Fusce fermentum sit amet augue sed rutrum.
 </p>
+
+<!-- See hypothesis/client#73 !-->
+<gh-73>
+  <div></div><span>Test</span>
+</gh-73>

h/static/scripts/annotator/anchoring/test/html-test.js

@@ -72,6 +72,8 @@ var rangeSpecs = [
   // Where the *Container nodes are expressed as either an XPath relative to
   // the container or the index into the list of text nodes within the container
   // node
+
+  // Test cases from Annotator v1.2.x's range_spec.coffee
   [ 0,           13,  0,           27,  'habitant morbi',                                    'Partial node contents.' ],
   [ 0,           0,   0,           37,  'Pellentesque habitant morbi tristique',             'Full node contents, textNode refs.' ],
   [ '/p/strong', 0,   '/p/strong', 1,   'Pellentesque habitant morbi tristique',             'Full node contents, elementNode refs.' ],
@@ -98,6 +100,9 @@ var rangeSpecs = [
   [ '/div[2]',   2,'/div[2]/text()[2]',28,'Lorem sed do eiusmod tempor.',                    'Text between br tags, with <p><br/></p> at the start'],
   [ '/div[2]',   1,'/div[2]/text()[2]',28,'Lorem sed do eiusmod tempor.',                    'Text between br tags, with <br/><p><br/></p> at the start'],
   [ '/h2[2]',    0,'/p[4]', 0, 'Header Level 2\n\n\n  Mauris lacinia ipsum nulla, id iaculis quam egestas quis.\n\n\n', 'No text node at the end and offset 0'],
+
+  // Additional test cases
+  [ '/gh-73/div', 0, '/gh-73/span/text()', 4, 'Test', 'Range starting at an element node with no children'],
 ];
 
 /**
@@ -117,7 +122,8 @@ var expectedFailures = [
   ['Text between br tags, with <br/><p><br/></p> at end', {position: true, quote: true}],
   ['Text between br tags, with <p><br/></p> at the start', {position: true, quote: true}],
   ['Text between br tags, with <br/><p><br/></p> at the start', {position: true, quote: true}],
-  ['No text node at the end and offset 0', {position: true, quote: true, range: true}],
+  ['No text node at the end and offset 0', {position: true, quote: true}],
+  ['Range starting at an element node with no children', {position: true, quote: true}],
 ];
 
 describe('HTML anchoring', function () {

h/static/scripts/annotator/anchoring/types.coffee

-Annotator = require('annotator')
-xpathRange = Annotator.Range
+xpathRange = require('./range')
 
 
 # Helper function for throwing common errors

h/static/scripts/annotator/anchoring/util.coffee

+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included in
+# all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+# THE SOFTWARE.
+#
+# This is a modified copy of
+# https://github.com/openannotation/annotator/blob/v1.2.x/src/util.coffee
+
+$ = require('jquery')
+
+{ simpleXPathJQuery, simpleXPathPure } = require('./xpath')
+
+Util = {}
+
+# Public: Flatten a nested array structure
+#
+# Returns an array
+Util.flatten = (array) ->
+  flatten = (ary) ->
+    flat = []
+
+    for el in ary
+      flat = flat.concat(if el and $.isArray(el) then flatten(el) else el)
+
+    return flat
+
+  flatten(array)
+
+# Public: Finds all text nodes within the elements in the current collection.
+#
+# Returns a new jQuery collection of text nodes.
+Util.getTextNodes = (jq) ->
+  getTextNodes = (node) ->
+    if node and node.nodeType != Node.TEXT_NODE
+      nodes = []
+
+      # If not a comment then traverse children collecting text nodes.
+      # We traverse the child nodes manually rather than using the .childNodes
+      # property because IE9 does not update the .childNodes property after
+      # .splitText() is called on a child text node.
+      if node.nodeType != Node.COMMENT_NODE
+        # Start at the last child and walk backwards through siblings.
+        node = node.lastChild
+        while node
+          nodes.push getTextNodes(node)
+          node = node.previousSibling
+
+      # Finally reverse the array so that nodes are in the correct order.
+      return nodes.reverse()
+    else
+      return node
+
+  jq.map -> Util.flatten(getTextNodes(this))
+
+# Public: determine the last text node inside or before the given node
+Util.getLastTextNodeUpTo = (n) ->
+  switch n.nodeType
+    when Node.TEXT_NODE
+      return n # We have found our text node.
+    when Node.ELEMENT_NODE
+      # This is an element, we need to dig in
+      if n.lastChild? # Does it have children at all?
+        result = Util.getLastTextNodeUpTo n.lastChild
+        if result? then return result
+    else
+      # Not a text node, and not an element node.
+  # Could not find a text node in current node, go backwards
+  n = n.previousSibling
+  if n?
+    Util.getLastTextNodeUpTo n
+  else
+    null
+
+# Public: determine the first text node in or after the given jQuery node.
+Util.getFirstTextNodeNotBefore = (n) ->
+  switch n.nodeType
+    when Node.TEXT_NODE
+      return n # We have found our text node.
+    when Node.ELEMENT_NODE
+      # This is an element, we need to dig in
+      if n.firstChild? # Does it have children at all?
+        result = Util.getFirstTextNodeNotBefore n.firstChild
+        if result? then return result
+    else
+      # Not a text or an element node.
+  # Could not find a text node in current node, go forward
+  n = n.nextSibling
+  if n?
+    Util.getFirstTextNodeNotBefore n
+  else
+    null
+
+Util.xpathFromNode = (el, relativeRoot) ->
+  try
+    result = simpleXPathJQuery.call el, relativeRoot
+  catch exception
+    console.log "jQuery-based XPath construction failed! Falling back to manual."
+    result = simpleXPathPure.call el, relativeRoot
+  result
+
+Util.nodeFromXPath = (xp, root) ->
+  steps = xp.substring(1).split("/")
+  node = root
+  for step in steps
+    [name, idx] = step.split "["
+    idx = if idx? then parseInt (idx?.split "]")[0] else 1
+    node = findChild node, name.toLowerCase(), idx
+
+  node
+
+module.exports = {
+  nodeFromXPath: Util.nodeFromXPath,
+  xpathFromNode: Util.xpathFromNode,
+  getTextNodes: Util.getTextNodes,
+  getFirstTextNodeNotBefore: Util.getFirstTextNodeNotBefore,
+  getLastTextNodeUpTo: Util.getLastTextNodeUpTo,
+}

h/static/scripts/annotator/anchoring/xpath.coffee

+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included in
+# all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+# THE SOFTWARE.
+#
+# This is a modified copy of
+# https://github.com/openannotation/annotator/blob/v1.2.x/src/xpath.coffee
+
+$ = require('jquery')
+
+# A simple XPath evaluator using jQuery which can evaluate queries of
+simpleXPathJQuery = (relativeRoot) ->
+  jq = this.map ->
+    path = ''
+    elem = this
+
+    while elem?.nodeType == Node.ELEMENT_NODE and elem isnt relativeRoot
+      tagName = elem.tagName.replace(":", "\\:")
+      idx = $(elem.parentNode).children(tagName).index(elem) + 1
+
+      idx  = "[#{idx}]"
+      path = "/" + elem.tagName.toLowerCase() + idx + path
+      elem = elem.parentNode
+
+    path
+
+  jq.get()
+
+# A simple XPath evaluator using only standard DOM methods which can
+# evaluate queries of the form /tag[index]/tag[index].
+simpleXPathPure = (relativeRoot) ->
+
+  getPathSegment = (node) ->
+    name = getNodeName node
+    pos = getNodePosition node
+    "#{name}[#{pos}]"
+
+  rootNode = relativeRoot
+
+  getPathTo = (node) ->
+    xpath = '';
+    while node != rootNode
+      unless node?
+        throw new Error "Called getPathTo on a node which was not a descendant of @rootNode. " + rootNode
+      xpath = (getPathSegment node) + '/' + xpath
+      node = node.parentNode
+    xpath = '/' + xpath
+    xpath = xpath.replace /\/$/, ''
+    xpath
+
+  jq = this.map ->
+    path = getPathTo this
+
+    path
+
+  jq.get()
+
+findChild = (node, type, index) ->
+  unless node.hasChildNodes()
+    throw new Error "XPath error: node has no children!"
+  children = node.childNodes
+  found = 0
+  for child in children
+    name = getNodeName child
+    if name is type
+      found += 1
+      if found is index
+        return child
+  throw new Error "XPath error: wanted child not found."
+
+# Get the node name for use in generating an xpath expression.
+getNodeName = (node) ->
+    nodeName = node.nodeName.toLowerCase()
+    switch nodeName
+      when "#text" then return "text()"
+      when "#comment" then return "comment()"
+      when "#cdata-section" then return "cdata-section()"
+      else return nodeName
+
+# Get the index of the node as it appears in its parent's child list
+getNodePosition = (node) ->
+  pos = 0
+  tmp = node
+  while tmp
+    if tmp.nodeName is node.nodeName
+      pos++
+    tmp = tmp.previousSibling
+  pos
+
+module.exports = {
+  simpleXPathJQuery,
+  simpleXPathPure,
+}

h/static/scripts/annotator/guest.coffee

@@ -10,6 +10,7 @@ adder = require('./adder')
 highlighter = require('./highlighter')
 rangeUtil = require('./range-util')
 selections = require('./selections')
+xpathRange = require('./anchoring/range')
 
 animationPromise = (fn) ->
   return new Promise (resolve, reject) ->
@@ -214,7 +215,7 @@ module.exports = class Guest extends Annotator
       # Highlight the range for an anchor.
       return anchor unless anchor.range?
       return animationPromise ->
-        range = Annotator.Range.sniff(anchor.range)
+        range = xpathRange.sniff(anchor.range)
         normedRange = range.normalize(root)
         highlights = highlighter.highlightRange(normedRange)
 
@@ -438,4 +439,3 @@ module.exports = class Guest extends Annotator
       @element.removeClass(SHOW_HIGHLIGHTS_CLASS)
 
     @visibleHighlights = shouldShowHighlights
-

h/static/scripts/annotator/highlighter.coffee

-Annotator = require('annotator')
-$ = Annotator.$
+$ = require('jquery')
 
 
 # Public: Wraps the DOM Nodes within the provided range with a highlight

h/static/scripts/annotator/test/highlighter-test.coffee

-Annotator = require('annotator')
-$ = Annotator.$
+Range = require('../anchoring/range')
+$ = require('jquery')
 
 highlighter = require('../highlighter')
 
@@ -8,7 +8,7 @@ describe "highlightRange", ->
     txt = document.createTextNode('test highlight span')
     el = document.createElement('span')
     el.appendChild(txt)
-    r = new Annotator.Range.NormalizedRange({
+    r = new Range.NormalizedRange({
       commonAncestor: el,
       start: txt,
       end: txt
@@ -26,7 +26,7 @@ describe "highlightRange", ->
     el.appendChild(txt)
     el.appendChild(blank)
     el.appendChild(txt2)
-    r = new Annotator.Range.NormalizedRange({
+    r = new Range.NormalizedRange({
       commonAncestor: el,
       start: txt,
       end: txt2