Merge remote-tracking branch 'hypothesis/master'

CHANGELOG.md

@@ -4,6 +4,13 @@ Entries in this change log follow the format suggested at http://keepachangelog.
 
 # Change Log
 
+## [1.32.1] - 2017-07-25
+
+### Changed
+
+- Fix IE 10/11 regression in documents without a `<base>` element.
+  ([#507](https://github.com/hypothesis/client/pull/507)).
+
 ## [1.32.0] - 2017-07-20
 
 ### Changed

package-lock.json

 {
   "name": "hypothesis",
-  "version": "1.32.0",
+  "version": "1.32.1",
   "lockfileVersion": 1,
   "dependencies": {
     "@gulp-sourcemaps/map-sources": {

package.json

 {
   "name": "hypothesis",
-  "version": "1.32.0",
+  "version": "1.32.1",
   "description": "Annotate with anyone, anywhere.",
   "license": "BSD-2-Clause",
   "homepage": "https://hypothes.is",

src/annotator/plugin/document.coffee

@@ -21,6 +21,10 @@ module.exports = class Document extends Plugin
     'beforeAnnotationCreated': 'beforeAnnotationCreated'
 
   pluginInit: ->
+    # Test seams.
+    @baseURI = @options.baseURI or baseURI
+    @document = @options.document or document
+
     this.getDocumentMetadata()
 
 # returns the primary URI for the document being annotated
@@ -179,20 +183,27 @@ module.exports = class Document extends Plugin
       if $(link).prop("rel") in ["shortcut icon", "icon"]
         @metadata["favicon"] = this._absoluteUrl(link.href)
 
-# hack to get a absolute url from a possibly relative one
-
+  # Hack to get a absolute url from a possibly relative one
   _absoluteUrl: (url) ->
-    d = document.createElement('a')
+    d = @document.createElement('a')
     d.href = url
     d.href
 
-# get the true URI record when it's masked via a different protocol.
-# this happens when an href is set with a uri using the 'blob:' protocol
-# but the document can set a different uri through a <base> tag.
-
+  # Get the true URI record when it's masked via a different protocol.
+  # This happens when an href is set with a uri using the 'blob:' protocol
+  # but the document can set a different uri through a <base> tag.
   _getDocumentHref: ->
-    href = document.location.href
-    if new URL(href).protocol != new URL(baseURI).protocol
-      # use the baseURI instead since it's likely what's intended
-      href = baseURI
+    href = @document.location.href
+    allowedSchemes = ['http:', 'https:', 'file:']
+
+    # Use the current document location if it has a recognized scheme.
+    if new URL(href).protocol in allowedSchemes
+      return href
+
+    # Otherwise, try using the location specified by the <base> element.
+    if @baseURI and (new URL(@baseURI).protocol in allowedSchemes)
+      return @baseURI
+
+    # Fall back to returning the document URI, even though the scheme is not
+    # in the allowed list.
     return href

src/annotator/plugin/test/document-test.coffee

 $ = require('jquery')
-proxyquire = require('proxyquire')
-Document = null
+Document = require('../document')
 
 ###
 ** Adapted from:
@@ -17,20 +16,15 @@ Document = null
 describe 'Document', ->
   testDocument = null
 
-# stub the baseURI for the following testing setup
-  docBaseUri = 'https://example.com/a_document.html'
-
   beforeEach ->
-    Document = proxyquire('../document', {
-      'document-base-uri': docBaseUri
-    })
     testDocument = new Document($('<div></div>')[0], {})
+    testDocument.pluginInit()
 
   afterEach ->
     $(document).unbind()
 
   describe 'annotation should have some metadata', ->
-# add some metadata to the page
+    # Add some metadata to the page
     head = $("head")
     head.append('<link rel="alternate" href="foo.pdf" type="application/pdf"></link>')
     head.append('<link rel="alternate" href="foo.doc" type="application/msword"></link>')
@@ -55,7 +49,6 @@ describe 'Document', ->
     metadata = null
 
     beforeEach ->
-      testDocument.pluginInit()
       metadata = testDocument.metadata
 
     it 'should have metadata', ->
@@ -67,7 +60,6 @@ describe 'Document', ->
     it 'should have links with absolute hrefs and types', ->
       assert.ok(metadata.link)
       assert.equal(metadata.link.length, 10)
-      assert.match(metadata.link[0].href, docBaseUri)
       assert.equal(metadata.link[1].rel, "alternate")
       assert.match(metadata.link[1].href, /^.+foo\.pdf$/)
       assert.equal(metadata.link[1].type, "application/pdf")
@@ -172,14 +164,62 @@ describe 'Document', ->
       )
       assert.equal(result, expected)
 
-  describe '#_getDocumentHref', ->
-
-    it 'should use the baseURI for the current testing setup', ->
-      assert.equal(testDocument._getDocumentHref(), docBaseUri)
+  describe '#uri', ->
 
-    it 'should use the test page href in a different case', ->
-      Document = proxyquire('../document', {
-        'document-base-uri': document.location.href
+    beforeEach ->
+      # Remove any existing canonical links which would otherwise override the
+      # document's own location.
+      canonicalLink = document.querySelector('link[rel="canonical"]')
+      if canonicalLink
+        canonicalLink.remove()
+
+    createDoc = (href, baseURI) ->
+      fakeDocument =
+        createElement: document.createElement.bind(document),
+        location:
+          href: href
+      doc = new Document($('<div></div>')[0], {
+        document: fakeDocument,
+        baseURI: baseURI,
       })
-      altDocument = new Document($('<div></div>')[0], {})
-      assert.equal(altDocument._getDocumentHref(), document.location.href)
+      doc.pluginInit()
+      doc
+
+    [
+      'http://publisher.org/book',
+      'https://publisher.org/book',
+      'file:///Users/jim/book',
+    ].forEach (href) ->
+      it "should return the document's URL if it has an allowed scheme", ->
+        baseURI = 'https://publisher.org/'
+        doc = createDoc(href, baseURI)
+        assert.equal(doc.uri(), href)
+
+    it "should return the baseURI if the document's URL does not have an allowed scheme", ->
+      href = 'blob:1234-5678'
+      baseURI = 'https://publisher.org/book'
+      doc = createDoc(href, baseURI)
+      assert.equal(doc.uri(), baseURI)
+
+    [
+      # The base URI is not available in IE if the document has no `<base>`
+      # tags. This is a limitation of `document-base-uri`.
+      ['https://publisher.org/article', undefined],
+      # Ignore base URIs with non-HTTP/HTTPS/file protocols, which can be
+      # created by a `<base>` tag.
+      ['blob:1234', 'doi:foo'],
+      ['chrome://foo', 'chrome://blah'],
+    ].forEach ([href, baseURI]) ->
+      it "should return the document's URL if it and the baseURI do not have an allowed scheme", ->
+        doc = createDoc(href, baseURI)
+        assert.equal(doc.uri(), href)
+
+    it 'returns the canonical URI if present', ->
+      canonicalLink = document.createElement('link')
+      canonicalLink.rel = 'canonical'
+      canonicalLink.href = 'https://publisher.org/canonical'
+      document.head.appendChild(canonicalLink)
+
+      doc = createDoc('https://publisher.org/not-canonical', null)
+
+      assert.equal doc.uri(), canonicalLink.href