From aec2c04645542868764dbef981b7570e4c72f3be Mon Sep 17 00:00:00 2001 From: Brian Grinstead Date: Thu, 15 Aug 2013 09:40:44 -0500 Subject: [PATCH] Bug 904049 - Inspector doesn't handle quotes in attributes and/or their escaping correctly. r=mratcliffe --- browser/devtools/markupview/markup-view.js | 161 ++++++++++-------- .../test/browser_inspector_markup_edit.html | 2 + .../test/browser_inspector_markup_edit.js | 73 ++++++++ 3 files changed, 166 insertions(+), 70 deletions(-) diff --git a/browser/devtools/markupview/markup-view.js b/browser/devtools/markupview/markup-view.js index beb45793ec8..b03aefdf69b 100644 --- a/browser/devtools/markupview/markup-view.js +++ b/browser/devtools/markupview/markup-view.js @@ -1219,81 +1219,102 @@ ElementEditor.prototype = { return this.node.startModifyingAttributes(); }, - _createAttribute: function EE_createAttribute(aAttr, aBefore) + _createAttribute: function EE_createAttribute(aAttr, aBefore = null) { - if (this.attrs.hasOwnProperty(aAttr.name)) { - var attr = this.attrs[aAttr.name]; - var name = attr.querySelector(".attrname"); - var val = attr.querySelector(".attrvalue"); - } else { - // Create the template editor, which will save some variables here. - let data = { - attrName: aAttr.name, - }; - this.template("attribute", data); - var {attr, inner, name, val} = data; + // Create the template editor, which will save some variables here. + let data = { + attrName: aAttr.name, + }; + this.template("attribute", data); + var {attr, inner, name, val} = data; - // Figure out where we should place the attribute. - let before = aBefore || null; - if (aAttr.name == "id") { - before = this.attrList.firstChild; - } else if (aAttr.name == "class") { - let idNode = this.attrs["id"]; - before = idNode ? idNode.nextSibling : this.attrList.firstChild; - } - this.attrList.insertBefore(attr, before); + // Double quotes need to be handled specially to prevent DOMParser failing. + // name="v"a"l"u"e" when editing -> name='v"a"l"u"e"' + // name="v'a"l'u"e" when editing -> name="v'a"l'u"e" + let editValueDisplayed = aAttr.value; + let hasDoubleQuote = editValueDisplayed.contains('"'); + let hasSingleQuote = editValueDisplayed.contains("'"); + let initial = aAttr.name + '="' + editValueDisplayed + '"'; - // Make the attribute editable. - editableField({ - element: inner, - trigger: "dblclick", - stopOnReturn: true, - selectAll: false, - contentType: InplaceEditor.CONTENT_TYPES.CSS_MIXED, - popup: this.markup.popup, - start: (aEditor, aEvent) => { - // If the editing was started inside the name or value areas, - // select accordingly. - if (aEvent && aEvent.target === name) { - aEditor.input.setSelectionRange(0, name.textContent.length); - } else if (aEvent && aEvent.target === val) { - let length = val.textContent.length; - let editorLength = aEditor.input.value.length; - let start = editorLength - (length + 1); - aEditor.input.setSelectionRange(start, start + length); - } else { - aEditor.input.select(); - } - }, - done: (aVal, aCommit) => { - if (!aCommit) { - return; - } - - let doMods = this._startModifyingAttributes(); - let undoMods = this._startModifyingAttributes(); - - // Remove the attribute stored in this editor and re-add any attributes - // parsed out of the input element. Restore original attribute if - // parsing fails. - try { - this._saveAttribute(aAttr.name, undoMods); - doMods.removeAttribute(aAttr.name); - this._applyAttributes(aVal, attr, doMods, undoMods); - this.undo.do(() => { - doMods.apply(); - }, () => { - undoMods.apply(); - }) - } catch(ex) { - console.error(ex); - } - } - }); - - this.attrs[aAttr.name] = attr; + // Can't just wrap value with ' since the value contains both " and '. + if (hasDoubleQuote && hasSingleQuote) { + editValueDisplayed = editValueDisplayed.replace(/\"/g, """); + initial = aAttr.name + '="' + editValueDisplayed + '"'; } + // Wrap with ' since there are no single quotes in the attribute value. + if (hasDoubleQuote && !hasSingleQuote) { + initial = aAttr.name + "='" + editValueDisplayed + "'"; + } + + // Make the attribute editable. + editableField({ + element: inner, + trigger: "dblclick", + stopOnReturn: true, + selectAll: false, + initial: initial, + contentType: InplaceEditor.CONTENT_TYPES.CSS_MIXED, + popup: this.markup.popup, + start: (aEditor, aEvent) => { + // If the editing was started inside the name or value areas, + // select accordingly. + if (aEvent && aEvent.target === name) { + aEditor.input.setSelectionRange(0, name.textContent.length); + } else if (aEvent && aEvent.target === val) { + let length = editValueDisplayed.length; + let editorLength = aEditor.input.value.length; + let start = editorLength - (length + 1); + aEditor.input.setSelectionRange(start, start + length); + } else { + aEditor.input.select(); + } + }, + done: (aVal, aCommit) => { + if (!aCommit) { + return; + } + + let doMods = this._startModifyingAttributes(); + let undoMods = this._startModifyingAttributes(); + + // Remove the attribute stored in this editor and re-add any attributes + // parsed out of the input element. Restore original attribute if + // parsing fails. + try { + this._saveAttribute(aAttr.name, undoMods); + doMods.removeAttribute(aAttr.name); + this._applyAttributes(aVal, attr, doMods, undoMods); + this.undo.do(() => { + doMods.apply(); + }, () => { + undoMods.apply(); + }) + } catch(ex) { + console.error(ex); + } + } + }); + + + // Figure out where we should place the attribute. + let before = aBefore; + if (aAttr.name == "id") { + before = this.attrList.firstChild; + } else if (aAttr.name == "class") { + let idNode = this.attrs["id"]; + before = idNode ? idNode.nextSibling : this.attrList.firstChild; + } + this.attrList.insertBefore(attr, before); + + // Remove the old version of this attribute from the DOM. + let oldAttr = this.attrs[aAttr.name]; + if (oldAttr && oldAttr.parentNode) { + oldAttr.parentNode.removeChild(oldAttr); + } + + this.attrs[aAttr.name] = attr; + name.textContent = aAttr.name; val.textContent = aAttr.value; diff --git a/browser/devtools/markupview/test/browser_inspector_markup_edit.html b/browser/devtools/markupview/test/browser_inspector_markup_edit.html index b48d3535c18..d49b4372488 100644 --- a/browser/devtools/markupview/test/browser_inspector_markup_edit.html +++ b/browser/devtools/markupview/test/browser_inspector_markup_edit.html @@ -40,5 +40,7 @@
+
+
diff --git a/browser/devtools/markupview/test/browser_inspector_markup_edit.js b/browser/devtools/markupview/test/browser_inspector_markup_edit.js index 8ac46389818..23760b80a39 100644 --- a/browser/devtools/markupview/test/browser_inspector_markup_edit.js +++ b/browser/devtools/markupview/test/browser_inspector_markup_edit.js @@ -239,6 +239,79 @@ function test() { }); } }, + + { + desc: "Modify inline style containing \"", + before: function() { + assertAttributes(doc.querySelector("#node26"), { + id: "node26", + style: 'background-image: url("moz-page-thumb://thumbnail?url=http%3A%2F%2Fwww.mozilla.org%2F");' + }); + }, + execute: function(after) { + inspector.once("markupmutation", after); + let editor = getContainerForRawNode(markup, doc.querySelector("#node26")).editor; + let attr = editor.attrs["style"].querySelector(".editable"); + + + attr.focus(); + EventUtils.sendKey("return", inspector.panelWin); + + let input = inplaceEditor(attr).input; + let value = input.value; + + is (value, + "style='background-image: url(\"moz-page-thumb://thumbnail?url=http%3A%2F%2Fwww.mozilla.org%2F\");'", + "Value contains actual double quotes" + ); + + value = value.replace(/mozilla\.org/, "mozilla.com"); + input.value = value; + + EventUtils.sendKey("return", inspector.panelWin); + }, + after: function() { + assertAttributes(doc.querySelector("#node26"), { + id: "node26", + style: 'background-image: url("moz-page-thumb://thumbnail?url=http%3A%2F%2Fwww.mozilla.com%2F");' + }); + } + }, + + { + desc: "Modify inline style containing \" and \'", + before: function() { + assertAttributes(doc.querySelector("#node27"), { + id: "node27", + class: 'Double " and single \'' + }); + }, + execute: function(after) { + inspector.once("markupmutation", after); + let editor = getContainerForRawNode(markup, doc.querySelector("#node27")).editor; + let attr = editor.attrs["class"].querySelector(".editable"); + + attr.focus(); + EventUtils.sendKey("return", inspector.panelWin); + + let input = inplaceEditor(attr).input; + let value = input.value; + + is (value, "class=\"Double " and single '\"", "Value contains ""); + + value = value.replace(/Double/, """).replace(/single/, "'"); + input.value = value; + + EventUtils.sendKey("return", inspector.panelWin); + }, + after: function() { + assertAttributes(doc.querySelector("#node27"), { + id: "node27", + class: '" " and \' \'' + }); + } + }, + { desc: "Add an attribute value without closing \"", enteredText: 'style="display: block;',