diff --git a/lib/Element.js b/lib/Element.js index 92c86e2..097ba75 100644 --- a/lib/Element.js +++ b/lib/Element.js @@ -28,7 +28,8 @@ function Element(doc, localName, namespaceURI, prefix) { this._tagName = undefined; // These properties maintain the set of attributes - this._attrsByQName = Object.create(null); // The qname->Attr map + // Map keeps numeric attribute names out of sparse object-index storage. + this._attrsByQName = new Map(); // The qname->Attr map this._attrsByLName = Object.create(null); // The ns|lname->Attr map this._attrKeys = []; // attr index -> ns|lname } @@ -534,7 +535,7 @@ Element.prototype = Object.create(ContainerNode.prototype, { qname = String(qname); if (/[A-Z]/.test(qname) && this.isHTML) qname = utils.toASCIILowerCase(qname); - var attr = this._attrsByQName[qname]; + var attr = this._attrsByQName.get(qname); if (!attr) return null; if (Array.isArray(attr)) // If there is more than one @@ -554,7 +555,7 @@ Element.prototype = Object.create(ContainerNode.prototype, { qname = String(qname); if (/[A-Z]/.test(qname) && this.isHTML) qname = utils.toASCIILowerCase(qname); - return this._attrsByQName[qname] !== undefined; + return this._attrsByQName.has(qname); }}, hasAttributeNS: { value: function hasAttributeNS(ns, lname) { @@ -573,7 +574,7 @@ Element.prototype = Object.create(ContainerNode.prototype, { if (!xml.isValidName(qname)) utils.InvalidCharacterError(); if (/[A-Z]/.test(qname) && this.isHTML) qname = utils.toASCIILowerCase(qname); - var a = this._attrsByQName[qname]; + var a = this._attrsByQName.get(qname); if (a === undefined) { if (force === undefined || force === true) { this._setAttribute(qname, ''); @@ -594,7 +595,7 @@ Element.prototype = Object.create(ContainerNode.prototype, { // XXX: the spec says that this next search should be done // on the local name, but I think that is an error. // email pending on www-dom about it. - var attr = this._attrsByQName[qname]; + var attr = this._attrsByQName.get(qname); var isnew; if (!attr) { attr = this._newattr(qname); @@ -607,7 +608,7 @@ Element.prototype = Object.create(ContainerNode.prototype, { // Now set the attribute value on the new or existing Attr object. // The Attr.value setter method handles mutation events, etc. attr.value = value; - if (this._attributes) this._attributes[qname] = attr; + setNamedProperty(this._attributes, qname, attr); if (isnew && this._newattrhook) this._newattrhook(qname, value); }}, @@ -695,7 +696,7 @@ Element.prototype = Object.create(ContainerNode.prototype, { utils.InUseAttributeError(); } var result = null; - var oldAttrs = this._attrsByQName[attr.name]; + var oldAttrs = this._attrsByQName.get(attr.name); if (oldAttrs) { if (!Array.isArray(oldAttrs)) { oldAttrs = [ oldAttrs ]; } if (oldAttrs.some(function(a) { return a===attr; })) { @@ -734,7 +735,7 @@ Element.prototype = Object.create(ContainerNode.prototype, { if (/[A-Z]/.test(qname) && this.isHTML) qname = utils.toASCIILowerCase(qname); - var attr = this._attrsByQName[qname]; + var attr = this._attrsByQName.get(qname); if (!attr) return; // If there is more than one match for this qname @@ -745,13 +746,13 @@ Element.prototype = Object.create(ContainerNode.prototype, { attr = attr.shift(); // remove it from the array } else { - this._attrsByQName[qname] = attr[1]; + this._attrsByQName.set(qname, attr[1]); attr = attr[0]; } } else { // only a single match, so remove the qname mapping - this._attrsByQName[qname] = undefined; + this._attrsByQName.delete(qname); } var ns = attr.namespaceURI; @@ -763,7 +764,7 @@ Element.prototype = Object.create(ContainerNode.prototype, { var i = this._attrKeys.indexOf(key); if (this._attributes) { Array.prototype.splice.call(this._attributes, i, 1); - this._attributes[qname] = undefined; + setNamedProperty(this._attributes, qname, undefined); } this._attrKeys.splice(i, 1); @@ -832,20 +833,20 @@ Element.prototype = Object.create(ContainerNode.prototype, { // prefix will never have two matching Attr objects (because // setAttributeNS doesn't allow a non-null namespace with a // null prefix. - var attr = this._attrsByQName[qname]; + var attr = this._attrsByQName.get(qname); return attr ? attr.value : null; }}, // The raw version of setAttribute for reflected idl attributes. _setattr: { value: function _setattr(qname, value) { - var attr = this._attrsByQName[qname]; + var attr = this._attrsByQName.get(qname); var isnew; if (!attr) { attr = this._newattr(qname); isnew = true; } attr.value = String(value); - if (this._attributes) this._attributes[qname] = attr; + setNamedProperty(this._attributes, qname, attr); if (isnew && this._newattrhook) this._newattrhook(qname, value); }}, @@ -854,7 +855,7 @@ Element.prototype = Object.create(ContainerNode.prototype, { _newattr: { value: function _newattr(qname) { var attr = new Attr(this, qname, null, null); var key = '|' + qname; - this._attrsByQName[qname] = attr; + this._attrsByQName.set(qname, attr); this._attrsByLName[key] = attr; if (this._attributes) { this._attributes[this._attrKeys.length] = attr; @@ -863,52 +864,48 @@ Element.prototype = Object.create(ContainerNode.prototype, { return attr; }}, - // Add a qname->Attr mapping to the _attrsByQName object, taking into + // Add a qname->Attr mapping to the _attrsByQName map, taking into // account that there may be more than one attr object with the // same qname _addQName: { value: function(attr) { var qname = attr.name; - var existing = this._attrsByQName[qname]; + var existing = this._attrsByQName.get(qname); if (!existing) { - this._attrsByQName[qname] = attr; + this._attrsByQName.set(qname, attr); } else if (Array.isArray(existing)) { existing.push(attr); } else { - this._attrsByQName[qname] = [existing, attr]; + this._attrsByQName.set(qname, [existing, attr]); } - if (this._attributes) this._attributes[qname] = attr; + setNamedProperty(this._attributes, qname, attr); }}, - // Remove a qname->Attr mapping to the _attrsByQName object, taking into + // Remove a qname->Attr mapping from the _attrsByQName map, taking into // account that there may be more than one attr object with the // same qname _removeQName: { value: function(attr) { var qname = attr.name; - var target = this._attrsByQName[qname]; + var target = this._attrsByQName.get(qname); if (Array.isArray(target)) { var idx = target.indexOf(attr); utils.assert(idx !== -1); // It must be here somewhere if (target.length === 2) { - this._attrsByQName[qname] = target[1-idx]; - if (this._attributes) { - this._attributes[qname] = this._attrsByQName[qname]; - } + this._attrsByQName.set(qname, target[1-idx]); + setNamedProperty(this._attributes, qname, this._attrsByQName.get(qname)); } else { target.splice(idx, 1); if (this._attributes && this._attributes[qname] === attr) { - this._attributes[qname] = target[0]; + setNamedProperty(this._attributes, qname, target[0]); } } } else { utils.assert(target === attr); // If only one, it must match - this._attrsByQName[qname] = undefined; - if (this._attributes) { - this._attributes[qname] = undefined; - } + this._attrsByQName.delete(qname); + setNamedProperty(this._attributes, qname, undefined); } }}, @@ -1090,15 +1087,30 @@ Attr.prototype = Object.create(Object.prototype, { // Sneakily export this class for use by Document.createAttribute() Element._Attr = Attr; +// WebIDL reserves array indices for indexed access, even outside the list. +// Mirroring numeric names would also create sparse object-index storage. +function isArrayIndex(qname) { + if (qname.length > 10) { return false; } + var index = qname >>> 0; + // Reject noncanonical spellings and 2^32-1, which is not an array index. + return index !== 0xFFFFFFFF && String(index) === qname; +} + +// Mirror a qname->Attr mapping onto an already-created NamedNodeMap. +function setNamedProperty(attributes, qname, attr) { + if (attributes && !isArrayIndex(qname)) { attributes[qname] = attr; } +} + // The attributes property of an Element will be an instance of this class. // This class is really just a dummy, though. It only defines a length // property and an item() method. The AttrArrayProxy that // defines the public API just uses the Element object itself. function AttributesArray(elt) { NamedNodeMap.call(this, elt); - for (var name in elt._attrsByQName) { - this[name] = elt._attrsByQName[name]; - } + var self = this; + elt._attrsByQName.forEach(function(attr, qname) { + setNamedProperty(self, qname, attr); + }); for (var i = 0; i < elt._attrKeys.length; i++) { this[i] = elt._attrsByLName[elt._attrKeys[i]]; } diff --git a/test/domino.js b/test/domino.js index bf0b33b..e8d0636 100644 --- a/test/domino.js +++ b/test/domino.js @@ -1,4 +1,5 @@ 'use strict'; +var assert = require('assert'); var domino = require('../lib'); var fs = require('fs'); var html = fs.readFileSync(__dirname + '/fixture/doc.html', 'utf8'); @@ -260,6 +261,224 @@ exports.attributes2 = function() { (div.attributes.onclick === undefined).should.be.true(); }; +var indexNames = ['0', '1', '7', '42', '4294967294']; +var otherNames = [ + '00', '01', '-0', '+1', '-1', '1.0', '1.5', '1e2', '0x10', + '4294967295', '4294967296', '10000000000', 'nan', 'infinity' +]; + +function indexedKeys(object) { + return Object.getOwnPropertyNames(object).filter(function(name) { + return /^(0|[1-9][0-9]*)$/.test(name) && Number(name) < 0xFFFFFFFF; + }); +} + +function checkStorage(element) { + var names = element.getAttributeNames(); + assert.ok(element._attrsByQName instanceof Map); + assert.strictEqual(element._attrsByQName.size, new Set(names).size); + assert.deepStrictEqual(indexedKeys(element._attrsByQName), []); + assert.deepStrictEqual(indexedKeys(element._attrsByLName), []); + var attrs = element.attributes; + assert.strictEqual(attrs, element.attributes); + assert.strictEqual(attrs.length, names.length); + assert.deepStrictEqual(indexedKeys(attrs), names.map(function(_, i) { return String(i); })); + names.forEach(function(name, i) { + assert.strictEqual(attrs[i], attrs.item(i)); + assert.strictEqual(attrs[i].name, name); + assert.strictEqual(attrs[i].ownerElement, element); + assert.ok(element._attrsByQName.has(name)); + }); + assert.strictEqual(attrs[names.length], undefined); + assert.strictEqual(attrs.item(names.length), null); + assert.deepStrictEqual(Array.from(attrs), names.map(function(_, i) { return attrs[i]; })); +} + +function parseElement(html) { + return domino.createDocument(html).body.firstChild; +} + +exports.numericAttributes = { + 'stores parsed numeric names without sparse property keys': function() { + var element = parseElement('
'); + checkStorage(element); + assert.strictEqual(element.getAttribute('999'), 'x'); + assert.strictEqual(element.getAttribute('2539'), 'y'); + assert.strictEqual(element.hasAttribute('2540'), false); + assert.strictEqual(element.attributes.getNamedItem('2539').value, 'y'); + assert.strictEqual(element.outerHTML, ''); + }, + + 'distinguishes array indices from numeric-looking names': function() { + indexNames.concat(otherNames).forEach(function(name) { + var element = parseElement(''); + checkStorage(element); + var attr = element.getAttributeNode(name); + assert.strictEqual(attr.value, 'value'); + assert.strictEqual(element.attributes.getNamedItem(name), attr); + if (indexNames.indexOf(name) !== -1) { + assert.strictEqual(element.attributes[name], element.attributes.item(Number(name)) || undefined); + } else { + assert.strictEqual(element.attributes[name], attr); + } + }); + }, + + 'keeps dense indices when attributes are added or updated after access': function() { + var element = parseElement(''); + var attrs = element.attributes; + element._setAttribute('1', 'second'); + element._setAttribute('42', 'last'); + element._setAttribute('0', 'updated'); + element.id = 'reflected'; + checkStorage(element); + assert.strictEqual(element.attributes, attrs); + assert.deepStrictEqual(element.getAttributeNames(), ['title', '0', '1', '42', 'id']); + assert.strictEqual(attrs[0].name, 'title'); + assert.strictEqual(attrs.getNamedItem('0').value, 'updated'); + assert.strictEqual(attrs.id.value, 'reflected'); + }, + + 'keeps DOM setter validation unchanged': function() { + var element = domino.createDocument().createElement('div'); + assert.throws(function() { element.setAttribute('7', 'value'); }, {name: 'InvalidCharacterError'}); + assert.throws(function() { element.setAttributeNS(null, '7', 'value'); }, {name: 'InvalidCharacterError'}); + assert.throws(function() { element.toggleAttribute('7'); }, {name: 'InvalidCharacterError'}); + checkStorage(element); + }, + + 'preserves clone and import storage before and after attributes access': function() { + [false, true].forEach(function(materialized) { + var element = parseElement('one
two'; + var expected = '
one
' + + 'two
'; + var doc = domino.createDocument(input); + assert.strictEqual(doc.body.innerHTML, expected); + Array.from(doc.querySelectorAll('b')).forEach(checkStorage); + doc.body.innerHTML = doc.body.innerHTML; + assert.strictEqual(doc.body.innerHTML, expected); + Array.from(doc.querySelectorAll('b')).forEach(checkStorage); + var template = doc.createElement('template'); + template.innerHTML = input; + assert.strictEqual(template.innerHTML, expected); + Array.from(template.content.querySelectorAll('b')).forEach(checkStorage); + }, + + 'preserves numeric names across incremental parser chunk boundaries': function() { + var input = ''; + for (var split = 0; split <= input.length; split++) { + var parser = domino.createIncrementalHTMLParser(); + parser.write(input.slice(0, split)); + parser.process(); + parser.end(input.slice(split)); + assert.strictEqual(parser.process(), false); + var element = parser.document().body.firstChild; + assert.strictEqual(element.outerHTML, ''); + checkStorage(element); + } + } +}; + +var removers = { + removeAttribute: function(element, name) { element.removeAttribute(name); }, + removeAttributeNS: function(element, name) { element.removeAttributeNS(null, name); }, + removeAttributeNode: function(element, name) { element.removeAttributeNode(element.getAttributeNode(name)); }, + removeNamedItem: function(element, name) { element.attributes.removeNamedItem(name); }, + removeNamedItemNS: function(element, name) { element.attributes.removeNamedItemNS(null, name); } +}; +Object.keys(removers).forEach(function(method) { + exports.numericAttributes[method + ' releases names and keeps indices dense'] = function() { + [false, true].forEach(function(materialized) { + var element = parseElement(''); + if (materialized) { checkStorage(element); } + ['0', '42', '1', 'title'].forEach(function(name) { + var attr = element.getAttributeNode(name); + removers[method](element, name); + assert.strictEqual(attr.ownerElement, null); + assert.strictEqual(element.hasAttribute(name), false); + assert.strictEqual(element.getAttribute(name), null); + assert.strictEqual(element._attrsByQName.has(name), false); + checkStorage(element); + }); + element._setAttribute('0', 'again'); + checkStorage(element); + assert.strictEqual(element.getAttribute('0'), 'again'); + }); + }; +}); + // exports.jquery1_9 = function() { // var window = createWindow(html); // var f = __dirname + '/fixture/jquery-1.9.1.js';