openannotation / xpath-range Goto Github PK

As explained by @nickstenning:

I'm concerned that we've squashed the history of the range module in the new git repo.
If at all possible, I'd like to see the git history of the range module preserved.
You can do this with git-filter-branch. Here's one possible approach: https://gist.github.com/ssp/1663093.

As explained by @aron:

For documentation I generally like the readme to have at least the following:

• Introduction (what is this repo and why does it exist)
• Usage (how do I install and use this in my project)
• Development (steps to pulling down and getting started working on the repo)
• Testing (how to setup and run the tests, and how to add more).
• Maintainers (or point of contact, e.g. the mailing list)

The current version is missing out on most of the above.

Right now the offsets are assumed to be text offsets.

The implementation could evaluate whether the container expression evaluates to a Text or Element and then treat the offset as appropriate.

Per openannotation/annotator#428 it looks like the definition for contains doesn't work in IE8 as-is. I'm not very familiar with this code, but this looks really similar to Jquery#9815 ? For context, this is being generated by the unit tests.

Hi all,

I hope you're well :).

I was wondering if you would consider PRs with:

• vanilla JS instead of jQuery, and
• ES6/7 instead of coffee code.

Thanks :)

In IE8, things like document.createRange and window.getSelection don't exist. Per the discussion over on Annotator openannotation/annotator#412 (comment) we're suggesting users use the Rangy plugin to fill these gaps - but that doesn't polyfill window or document, it just provides a standard interface instead a la jQuery.

For instances like this and this I think it would make sense to test if rangy is being used and if not, default to the current methods. E.g.:

win = rangy ? Util.getGlobal()
sel = win.getSelection() # Get the browser selection object

Relates to openannotation/annotator#428

• Decide whether ignoreSelector belongs here. Is something simpler possible?
• Decide whether to remove limit which doesn't seem to belong here.

It looks like /tools/build was designed for annotator and hasn't been modified since, maybe? Just running it without flags doesn't appear to build anything, and most of the flags I see listed just throw errors (e.g. using -p throws plugin is not defined).

The toNode method has a bug.

# Public: Finds an Element Node using an XPath relative to the document root.
#
# If the document is served as application/xhtml+xml it will try and resolve
# any namespaces within the XPath.
#
# path - An XPath String to query.
#
# Examples
#
#   node = toNode('/html/body/div/p[2]')
#   if node
#     # Do something with the node.
#
# Returns the Node if found otherwise null.
toNode = (path, root = document) ->

It will not find the node if the attribute selector has / or : in it. Example:
toNode("//p[@resource='http://purl.org/pearson/asset/adb19cb3a588ab58cbc0db272048aed6d60a29db5']") and toNode("//p[@resource='asset:adb4afbb2c91461765201bd8eb32f54aad1bb3800']") will not work. However toNode("//p[@resource='adb4afbb2c91461765201bd8eb32f54aad1bb3800']") works.

The problem is with the following code (line 161 in xpath.coffee):

      path = (for segment in path.split '/'
        if segment and segment.indexOf(':') == -1
          segment.replace(/^([a-z]+)/, 'xhtml:$1')
        else segment
      ).join('/')

This will convert the //p[@resource='http://purl.org/pearson/asset/adb19cb3a588ab58cbc0db272048aed6d60a29db5'] into
//xhtml:p[@resource='http://xhtml:purl.org/xhtml:pearson/xhtml:asset/xhtml:adb19cb3a588ab58cbc0db272048aed6d60a29db5'] instead of converting to //xhtml:p[@resource='http://purl.org/pearson/asset/adb19cb3a588ab58cbc0db272048aed6d60a29db5']

I have never worked on coffee script and it took me a while to find the coffee script... by that time I made a code change to the generated code to fix the issue I am facing... Here is the screenshot of diff (which may not be useful):
http://content.screencast.com/users/harinair/folders/Jing/media/fac08329-05bf-4b51-8bba-cea25913dc15/00000064.png

The test suite no longer tests XML since 03205bf made isXML a private utility of the xpath module.

A better option would be to run the entire test suite on an XML document.

Going back to openannotation/annotator#173, there's no support for XPath related operations in any version of IE really, so we'll need a shim here. Wicked-Good-Xpath does in fact solve most of the issues here, so I recommend adding that as part of the main package, since it affects even modern IEs.