Fix annotations made on PDFs in non-Safari browsers not appearing in Safari (#3494)

In Safari a trailing '#' is appended to URIs extracted from web
documents and PDFs. For URLs this makes no difference as the Hypothesis
search API ignores the fragment identifier. However a URN with a
trailing '#' is considered different from a URN without a trailing '#'.
Since the URI for PDFs is now a URN of the form 'urn:x-pdf:...', when
viewing a PDF in Safari annotations made in other browsers were not
fetched and vice-versa.

The trailing '#' is caused by a bug in the `URL.hash` setter in Safari.

According to section 6.3 of the URL spec [1], setting URL.hash to an empty
string should _remove_ the fragment identifier. In Safari however, it
sets the fragment identifier to an empty string. The result is that when
the URL is serialized, the output is eg. 'urn:x-pdf:aabb#' instead of
'urn:x-pdf:aabb'.

Upstream bug: https://bugs.webkit.org/show_bug.cgi?id=158869
Fixes #3471

[1] https://url.spec.whatwg.org/#urlutils-members

h/static/scripts/annotator/guest.coffee

@@ -19,6 +19,17 @@ animationPromise = (fn) ->
       catch error
         reject(error)
 
+# Normalize the URI for an annotation. This makes it absolute and strips
+# the fragment identifier.
+normalizeURI = (uri, baseURI) ->
+  # Convert to absolute URL
+  url = new URL(uri, baseURI)
+  # Remove the fragment identifier.
+  # This is done on the serialized URL rather than modifying `url.hash` due
+  # to a bug in Safari.
+  # See https://github.com/hypothesis/h/issues/3471#issuecomment-226713750
+  return url.toString().replace(/#.*/, '');
+
 module.exports = class Guest extends Annotator
   SHOW_HIGHLIGHTS_CLASS = 'annotator-highlights-always-on'
 
@@ -95,12 +106,8 @@ module.exports = class Guest extends Annotator
       link: [{href: decodeURIComponent(window.location.href)}]
     })
 
-    return metadataPromise.then (metadata) =>
-      return uriPromise.then (href) =>
-        uri = new URL(href, baseURI)
-        uri.hash = ''
-        uri = uri.toString()
-        return {uri, metadata}
+    return Promise.all([metadataPromise, uriPromise]).then ([metadata, href]) ->
+      return {uri: normalizeURI(href, baseURI), metadata}
 
   _connectAnnotationSync: (crossframe) ->
     this.subscribe 'annotationDeleted', (annotation) =>

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

@@ -206,6 +206,28 @@ describe 'Guest', ->
 
         emitGuestEvent('getDocumentInfo', assertComplete)
 
+  describe 'getDocumentInfo()', ->
+    guest = null
+
+    beforeEach ->
+      guest = createGuest()
+      guest.plugins.PDF =
+        uri: -> 'urn:x-pdf:001122'
+        getMetadata: sandbox.stub()
+
+    it 'preserves the components of the URI other than the fragment', ->
+      guest.plugins.PDF = null
+      guest.plugins.Document =
+        uri: -> 'http://foobar.com/things?id=42'
+        metadata: {}
+      return guest.getDocumentInfo().then ({uri}) ->
+        assert.equal uri, 'http://foobar.com/things?id=42'
+
+    it 'removes the fragment identifier from URIs', () ->
+      guest.plugins.PDF.uri = -> 'urn:x-pdf:aabbcc#'
+      return guest.getDocumentInfo().then ({uri}) ->
+        assert.equal uri, 'urn:x-pdf:aabbcc'
+
   describe 'onAdderMouseUp', ->
     it 'it prevents the default browser action when triggered', () ->
       event = jQuery.Event('mouseup')