Skip to content

Commit

Permalink
Avoid document plugin problem on XHTML documents
Browse files Browse the repository at this point in the history 
jQuery will choke if the img tag isn't closed.
  • Loading branch information
@tilgovi
tilgovi committed Oct 10, 2013
1 parent 2fee8b7 commit d2f8c43
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/plugin/document.coffee
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ class Annotator.Plugin.Document extends Annotator.Plugin
# hack to get a absolute url from a possibly relative one

_absoluteUrl: (url) ->
img = $("<img src='#{ url }'>")
img = $("<img src='#{ url }'></img>")
url = img.prop('src')
img.prop('src', null)
return url

6 comments on commit d2f8c43

@edsu
Copy link
Contributor

@edsu edsu commented on d2f8c43 Oct 10, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eek, really? jQuery throws an error if you add an element to the DOM for an XHTML document?

@tilgovi
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. When running on an XHTML document (maybe only strict mode), the tag must be closed or creating the fragment is an error. That's in chrome, at least.

@csillag
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@edsu, I have already pinged you about this problem. See original report, and my fix here:
https://github.com/hypothesis/annotator/commit/423e6d14d1e72de7050fa1d7b3442d1a0213014f

@nickstenning
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be a self-closing tag (<img ... />) not a tag pair?

@csillag
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my implementation (referenced above), it was, but I guess this works, too.

@tilgovi
Copy link
Member Author

@tilgovi tilgovi commented on d2f8c43 Oct 24, 2013 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.