Feature: Allow for additional attributes

[terence1990]

I needed a data attribute on the link in my project so thought this would be helpful for others

[ljosa]

What quoting is necessary here? I know we don't have any quoting for target, but that one has a much more limited use.

Can you please add unit tests? See test_urlize.js.

The last line should end with a newline.

[terence1990]

@ljosa i'll do this on Friday, what do you mean by quoting? - do you mean code commenting?

[ljosa]

Perhaps I should have written "escaping" instead of "quoting." The code builds HTML. As you know, many characters have special meaning in HTML. If these characters occur in the value of your data attribute, then the result can be invalid HTML (or even a security problem, such as an XSS vulnerability, depending on where the value comes from). I believe that &, <, >, `, and " need to be escaped (but please give that some thought and make sure it's correct).

Perhaps it would be a good idea to validate the names of the data attributes also, since controls, U+0020 SPACE, U+0022 ("), U+0027 ('), U+003E (>), U+002F (/), U+003D (=), and noncharacters are not allowed in HTML attribute names (source).

[terence1990]

Hey @ljosa - have escaped values and removed illegal characters from keys and added tests - all asserting true

[ljosa]

Sorry, that's not right kind of escaping. If you have an HTML document with

<a id="foo" href="http://www.example.com/" position="left" open-in-app="%22%2Ctrue">http://www.example.com/</a>

then
document.getElementById('foo').getAttribute('open-in-app')
returns "%22%2Ctrue",
which is not what you want. You'll have to replace the illegal characters with HTML entities. For instance:

<a id="foo" href="http://www.example.com/" position="left" open-in-app="&quot;,true">http://www.example.com/</a>

Then, document.getElementById('foo').getAttribute('open-in-app') returns "\",true" as it should.

[terence1990]

@ljosa just made another commit fixes this to ensure HTMLEntities are returned - tests updated and asserting true

[ljosa]

A few problems still:

• In the value, ampersand (&) needs to be escaped as &.
• In the value, backtick (`) needs to be escaped as `.
• The correct escape for single quote (') is ', not ‘.

Question: why are you escaping space, slash, and equal sign in the values? I don't think that is necessary—do you have reason to believe otherwise?

I believe the unit test for the values should be:

	it('add additional attributes with illegal values', function () {
	    equal(urlize('http://www.example.com/', {attrs: {position: 'left', 'open-in-app': '&<>`"\n\'/true'}}),
	          '<a href="http://www.example.com/" position="left" open-in-app="&amp;&lt;&gt;&#96;&quot;&#13;&#39;/true">http://www.example.com/</a>');
	});

Do you agree?

Finally, can you please document the new functionality in the README.md?

Thanks!