diff --git a/browser/devtools/markupview/MarkupView.jsm b/browser/devtools/markupview/MarkupView.jsm index abd39d409521..ae1ec25858b5 100644 --- a/browser/devtools/markupview/MarkupView.jsm +++ b/browser/devtools/markupview/MarkupView.jsm @@ -1178,28 +1178,21 @@ ElementEditor.prototype = { * @param Element aAttrNode the attribute editor that created this * set of attributes, used to place new attributes where the * user put them. - * @throws SYNTAX_ERR if aValue is not well-formed. */ _applyAttributes: function EE__applyAttributes(aValue, aAttrNode) { - // Create a dummy node for parsing the attribute list. - let dummyNode = this.doc.createElement("div"); - - let parseTag = (this.node.namespaceURI.match(/svg/i) ? "svg" : - (this.node.namespaceURI.match(/mathml/i) ? "math" : "div")); - let parseText = "<" + parseTag + " " + aValue + "/>"; - // Throws exception if parseText is not well-formed. - dummyNode.innerHTML = parseText; - let parsedNode = dummyNode.firstChild; - - let attrs = parsedNode.attributes; + let attrs = escapeAttributeValues(aValue); this.undo.startBatch(); - for (let i = 0; i < attrs.length; i++) { + for (let attr of attrs) { + let attribute = { + name: attr.name, + value: attr.value + }; // Create an attribute editor next to the current attribute if needed. - this._createAttribute(attrs[i], aAttrNode ? aAttrNode.nextSibling : null); - this._setAttribute(this.node, attrs[i].name, attrs[i].value); + this._createAttribute(attribute, aAttrNode ? aAttrNode.nextSibling : null); + this._setAttribute(this.node, attr.name, attr.value); } this.undo.endBatch(); @@ -1424,6 +1417,100 @@ DocumentWalker.prototype = { nextSibling: function DW_nextSibling() this.walker.nextSibling(), // XXX bug 785143: not doing previousNode or nextNode, which would sure be useful. +}; + +/** + * Properly escape attribute values. + * + * @param {String} attr + * The attributes for which the values are to be escaped. + * @return {Array} + * An array of attribute names and their escaped values. + */ +function escapeAttributeValues(attr) { + let name = null; + let value = null; + let result = ""; + let attributes = []; + + while(attr.length > 0) { + let match; + let dirty = false; + + // Trim quotes and spaces from attr start + match = attr.match(/^["\s]+/); + if (match && match.length == 1) { + attr = attr.substr(match[0].length); + } + + // Name + if (!dirty) { + match = attr.match(/^([\w-]+)="/); + if (match && match.length == 2) { + if (name) { + // We had a name without a value e.g. disabled. Let's set the value to ""; + value = ""; + } else { + name = match[1]; + attr = attr.substr(match[0].length); + } + dirty = true; + } + } + + // Value (in the case of multiple attributes) + if (!dirty) { + match = attr.match(/^(.+?)"\s+[\w-]+="/); + if (match && match.length > 1) { + value = typeof match[1] == "undefined" ? match[2] : match[1]; + attr = attr.substr(value.length); + value = simpleEscape(value); + dirty = true; + } + } + + // Final value + if (!dirty && attr.indexOf("=\"") == -1) { + // No more attributes, get the remaining value minus it's ending quote. + if (attr.charAt(attr.length - 1) == '"') { + attr = attr.substr(0, attr.length - 1); + } + + if (!name) { + name = attr; + value = ""; + } else { + value = simpleEscape(attr); + } + attr = ""; + dirty = true; + } + + if (name !== null && value !== null) { + attributes.push({name: name, value: value}); + name = value = null; + } + + if (!dirty) { + // This should never happen but we exit here if it does. + return attributes; + } + } + return attributes; +} + +/** + * Escape basic html entities <, >, " and '. + * @param {String} value + * Value to escape. + * @return {String} + * Escaped value. + */ +function simpleEscape(value) { + return value.replace(//g, ">") + .replace(/"/g, """) + .replace(/'/g, "'"); } /** diff --git a/browser/devtools/markupview/test/browser_inspector_markup_edit.html b/browser/devtools/markupview/test/browser_inspector_markup_edit.html index a199cc485287..b48d3535c188 100644 --- a/browser/devtools/markupview/test/browser_inspector_markup_edit.html +++ b/browser/devtools/markupview/test/browser_inspector_markup_edit.html @@ -39,5 +39,6 @@
+
diff --git a/browser/devtools/markupview/test/browser_inspector_markup_edit.js b/browser/devtools/markupview/test/browser_inspector_markup_edit.js index e8c240b6076f..a5eb0dcf39b9 100644 --- a/browser/devtools/markupview/test/browser_inspector_markup_edit.js +++ b/browser/devtools/markupview/test/browser_inspector_markup_edit.js @@ -42,12 +42,27 @@ function test() { EventUtils.sendKey("return", inspector.panelWin); } + /** + * Check that the appropriate attributes are assigned to a node. + * + * @param {HTMLNode} aElement + * The node to check. + * @param {Object} aAttributes + * An object containing the arguments to check. + * e.g. {id="id1",class="someclass"} + * + * NOTE: When checking attribute values bare in mind that node.getAttribute() + * returns attribute values provided by the HTML parser. The parser only + * provides unescaped entities so & will return &. + */ function assertAttributes(aElement, aAttributes) { let attrs = Object.getOwnPropertyNames(aAttributes); - is(aElement.attributes.length, attrs.length, "Node has the correct number of attributes"); + is(aElement.attributes.length, attrs.length, + "Node has the correct number of attributes"); for (let attr of attrs) { - is(aElement.getAttribute(attr), aAttributes[attr], "Node has the correct " + attr + " attribute."); + is(aElement.getAttribute(attr), aAttributes[attr], + "Node has the correct " + attr + " attribute."); } } @@ -76,7 +91,8 @@ function test() { }, { - desc: "Try change an attribute to a badly formed string", + desc: 'Try changing an attribute to a quote (") - this should result ' + + 'in it being set to an empty string', before: function() { assertAttributes(doc.querySelector("#node22"), { id: "node22", @@ -92,7 +108,7 @@ function test() { after: function() { assertAttributes(doc.querySelector("#node22"), { id: "node22", - class: "unchanged" + class: "" }); } }, @@ -141,7 +157,9 @@ function test() { }, { - desc: "Try add a badly formed attribute by clicking the empty space after a node", + desc: 'Try add an attribute containing a quote (") attribute by ' + + 'clicking the empty space after a node - this should result ' + + 'in it being set to an empty string', before: function() { assertAttributes(doc.querySelector("#node23"), { id: "node23", @@ -156,6 +174,8 @@ function test() { after: function() { assertAttributes(doc.querySelector("#node23"), { id: "node23", + class: "newclass", + style: "" }); } }, @@ -176,6 +196,7 @@ function test() { after: function() { assertAttributes(doc.querySelector("#node24"), { id: "node24", + class: "" }); } }, @@ -198,6 +219,27 @@ function test() { is(node.nodeValue, "New text", "Text should be changed."); }, }, + + { + desc: "Add an attribute value containing < > ü \" & '", + before: function() { + assertAttributes(doc.querySelector("#node25"), { + id: "node25", + }); + }, + execute: function(after) { + inspector.once("markupmutation", after); + let editor = markup.getContainer(doc.querySelector("#node25")).editor; + let attr = editor.newAttr; + editField(attr, 'src="somefile.html?param1=¶m2=ü"bl\'ah"'); + }, + after: function() { + assertAttributes(doc.querySelector("#node25"), { + id: "node25", + src: "somefile.html?param1=<a>¶m2=ü"bl'ah" + }); + } + }, ]; // Create the helper tab for parsing...