Linkify element fixes (#148)

* Small refactoring and optimizations for linkify-element
* Added test for empty text node on linkify-element
* Additional small tweaks and enhancements
  * For jquery test and linkify-string

Fixes #145

Author: Nick Frasser

src/linkify-element.js

@@ -3,8 +3,9 @@
 */
 
 import * as linkify from './linkify';
-var tokenize = linkify.tokenize;
-var options = linkify.options;
+let tokenize = linkify.tokenize;
+let options = linkify.options;
+let TEXT_TOKEN = linkify.parser.TOKENS.TEXT;
 
 const HTML_NODE = 1, TXT_NODE = 3;
 
@@ -34,53 +35,54 @@ function tokensToNodes(tokens, opts, doc) {
 
 	for (let i = 0; i < tokens.length; i++) {
 		let token = tokens[i];
-		let validated = token.isLink && options.resolve(opts.validate, token.toString(), token.type);
-
-		if (token.isLink && validated) {
-
-			let
-			href			= token.toHref(opts.defaultProtocol),
-			formatted		= options.resolve(opts.format, token.toString(), token.type),
-			formattedHref	= options.resolve(opts.formatHref, href, token.type),
-			attributesHash	= options.resolve(opts.attributes, href, token.type),
-			tagName			= options.resolve(opts.tagName, href, token.type),
-			linkClass		= options.resolve(opts.linkClass, href, token.type),
-			target			= options.resolve(opts.target, href, token.type),
-			events			= options.resolve(opts.events, href, token.type);
-
-			// Build the link
-			let link = doc.createElement(tagName);
-			link.setAttribute('href', formattedHref);
-			link.setAttribute('class', linkClass);
-			if (target) {
-				link.setAttribute('target', target);
-			}
 
-			// Build up additional attributes
-			if (attributesHash) {
-				for (var attr in attributesHash) {
-					link.setAttribute(attr, attributesHash[attr]);
-				}
+		if (token.type === 'nl' && opts.nl2br) {
+			result.push(doc.createElement('br'));
+			continue;
+		} else if (
+			!token.isLink ||
+			!options.resolve(opts.validate, token.toString(), token.type)
+		) {
+			result.push(doc.createTextNode(token.toString()));
+			continue;
+		}
+
+		let href = token.toHref(opts.defaultProtocol);
+		let formatted = options.resolve(opts.format, token.toString(), token.type);
+		let formattedHref = options.resolve(opts.formatHref, href, token.type);
+		let attributesHash = options.resolve(opts.attributes, href, token.type);
+		let tagName = options.resolve(opts.tagName, href, token.type);
+		let linkClass = options.resolve(opts.linkClass, href, token.type);
+		let target = options.resolve(opts.target, href, token.type);
+		let events = options.resolve(opts.events, href, token.type);
+
+		// Build the link
+		let link = doc.createElement(tagName);
+		link.setAttribute('href', formattedHref);
+		link.setAttribute('class', linkClass);
+		if (target) {
+			link.setAttribute('target', target);
+		}
+
+		// Build up additional attributes
+		if (attributesHash) {
+			for (var attr in attributesHash) {
+				link.setAttribute(attr, attributesHash[attr]);
 			}
+		}
 
-			if (events) {
-				for (var event in events) {
-					if (link.addEventListener) {
-						link.addEventListener(event, events[event]);
-					} else if (link.attachEvent)  {
-						link.attachEvent('on' + event, events[event]);
-					}
+		if (events) {
+			for (var event in events) {
+				if (link.addEventListener) {
+					link.addEventListener(event, events[event]);
+				} else if (link.attachEvent)  {
+					link.attachEvent('on' + event, events[event]);
 				}
 			}
-
-			link.appendChild(doc.createTextNode(formatted));
-			result.push(link);
-
-		} else if (token.type === 'nl' && opts.nl2br) {
-			result.push(doc.createElement('br'));
-		} else {
-			result.push(doc.createTextNode(token.toString()));
 		}
+
+		link.appendChild(doc.createTextNode(formatted));
+		result.push(link);
 	}
 
 	return result;
@@ -112,15 +114,20 @@ function linkifyElementHelper(element, opts, doc) {
 			break;
 		case TXT_NODE:
 
-			let
-			str = childElement.nodeValue,
-			tokens = tokenize(str),
-			nodes = tokensToNodes(tokens, opts, doc);
+			let str = childElement.nodeValue;
+			let tokens = tokenize(str);
+
+			if (tokens.length === 0 || tokens.length === 1 && tokens[0] instanceof TEXT_TOKEN) {
+				// No node replacement required
+				break;
+			}
+
+			let nodes = tokensToNodes(tokens, opts, doc);
 
 			// Swap out the current child for the set of nodes
 			replaceChildWithChildren(element, childElement, nodes);
 
-			// so that the correct sibling is selected
+			// so that the correct sibling is selected next
 			childElement = nodes[nodes.length - 1];
 
 			break;

src/linkify-string.js

@@ -24,7 +24,7 @@ function attributesToString(attributes) {
 	let result = [];
 
 	for (let attr in attributes) {
-		let val = (attributes[attr] + '').replace(/"/g, '"');
+		let val = attributes[attr] + '';
 		result.push(`${attr}="${escapeAttr(val)}"`);
 	}
 	return result.join(' ');

test/spec/linkify-element-test.js

@@ -54,16 +54,12 @@ describe('linkify-element', function () {
 	});
 
 	it('Works with default options', function () {
-		expect(testContainer).to.be.ok;
-		expect(testContainer).to.be.a('object');
 		var result = linkifyElement(testContainer, null, doc);
 		expect(result).to.equal(testContainer); // should return the same element
 		expect(htmlOptions.linkified).to.contain(testContainer.innerHTML);
 	});
 
 	it('Works with overriden options (general)', function () {
-		expect(testContainer).to.be.ok;
-		expect(testContainer).to.be.a('object');
 		var result = linkifyElement(testContainer, htmlOptions.altOptions, doc);
 		expect(result).to.equal(testContainer); // should return the same element
 		expect(htmlOptions.linkifiedAlt).to.contain(testContainer.innerHTML);
@@ -81,10 +77,15 @@ describe('linkify-element', function () {
 	});
 
 	it('Works with overriden options (validate)', function () {
-		expect(testContainer).to.be.ok;
-		expect(testContainer).to.be.a('object');
 		var result = linkifyElement(testContainer, htmlOptions.validateOptions, doc);
 		expect(result).to.equal(testContainer); // should return the same element
 		expect(htmlOptions.linkifiedValidate).to.contain(testContainer.innerHTML);
 	});
+
+	it('Works when there is an empty text nodes', function () {
+		testContainer.appendChild(doc.createTextNode(''));
+		var result = linkifyElement(testContainer, null, doc);
+		expect(result).to.equal(testContainer); // should return the same element
+		expect(htmlOptions.linkified).to.contain(testContainer.innerHTML);
+	});
 });

test/spec/linkify-jquery-test.js

@@ -4,50 +4,6 @@ $, doc, testContainer, jsdom,
 applyLinkify = require('../../lib/linkify-jquery').default,
 htmlOptions = require('./html/options');
 
-if (typeof Object.create != 'function') {
-  // Production steps of ECMA-262, Edition 5, 15.2.3.5
-  // Reference: http://es5.github.io/#x15.2.3.5
-  Object.create = (function() {
-    // To save on memory, use a shared constructor
-    function Temp() {}
-
-    // make a safe reference to Object.prototype.hasOwnProperty
-    var hasOwn = Object.prototype.hasOwnProperty;
-
-    return function (O) {
-      // 1. If Type(O) is not Object or Null throw a TypeError exception.
-      if (typeof O != 'object') {
-        throw TypeError('Object prototype may only be an Object or null');
-      }
-
-      // 2. Let obj be the result of creating a new object as if by the
-      //    expression new Object() where Object is the standard built-in
-      //    constructor with that name
-      // 3. Set the [[Prototype]] internal property of obj to O.
-      Temp.prototype = O;
-      var obj = new Temp();
-      Temp.prototype = null; // Let's not keep a stray reference to O...
-
-      // 4. If the argument Properties is present and not undefined, add
-      //    own properties to obj as if by calling the standard built-in
-      //    function Object.defineProperties with arguments obj and
-      //    Properties.
-      if (arguments.length > 1) {
-        // Object.defineProperties does ToObject on its first argument.
-        var Properties = Object(arguments[1]);
-        for (var prop in Properties) {
-          if (hasOwn.call(Properties, prop)) {
-            obj[prop] = Properties[prop];
-          }
-        }
-      }
-
-      // 5. Return obj
-      return obj;
-    };
-  })();
-}
-
 try {
 	doc = document;
 	$ = require('jquery'); // should be available through Browserify
@@ -104,7 +60,7 @@ describe('linkify-jquery', function () {
 	});
 
 	// This works but is inconsistent across browsers
-	xit('Works with the DOM Data API', function () {
+	it('Works with the DOM Data API', function () {
 		expect($('header').first().html()).to.be.eql(
 			'Have a link to:<br><a href="https://github.com" class="linkified" target="_blank">github.com</a>!'
 		);