804400 - Inspector stops updating when badly formed attribute input is given in the markup panel.

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[N Bosma]

Uncaught exception (SYNTAX_ERR) in MarkupView.jsm:_applyAttributes (innerHTML). Problem occurs when badly formed attribute string is supplied. One of the consequences of this is that the inspector view stops updating.

Comment 1

[N Bosma]

Attachment: Example scenario.

Comment 2

[N Bosma]

Attachment: Fix

Comment 3

[N Bosma]

Comment on attachment 674053 [details]
Fix

># HG changeset patch
># User N Bosma <[e-mail]>
># Parent  034634c4125ae8bfd14f9aa16b5df813c16a136b
>Bug 804400 - Inspector stops updating when badly formed attribute input is given in the markup panel.
>
>diff --git a/browser/devtools/markupview/MarkupView.jsm b/browser/devtools/markupview/MarkupView.jsm
>--- a/browser/devtools/markupview/MarkupView.jsm
>+++ b/browser/devtools/markupview/MarkupView.jsm
>@@ -957,21 +957,26 @@ ElementEditor.prototype = {
>         done: function EE_editAttribute_done(aVal, aCommit) {
>           if (!aCommit) {
>             return;
>           }
> 
>           this.undo.startBatch();
> 
>           // Remove the attribute stored in this editor and re-add any attributes
>-          // parsed out of the input element.
>-          this._removeAttribute(this.node, aAttr.name)
>-          this._applyAttributes(aVal, attr);
>-
>-          this.undo.endBatch();
>+          // parsed out of the input element. Restore original attribute if
>+          // parsing fails.
>+          this._removeAttribute(this.node, aAttr.name);
>+          try {
>+            this._applyAttributes(aVal, attr);
>+            this.undo.endBatch();
>+          } catch (e) {
>+            this.undo.endBatch();
>+            this.undo.undo();
>+          }
>         }.bind(this)
>       });
> 
>       this.attrs[aAttr.name] = attr;
>     }
> 
>     name.textContent = aAttr.name;
>     val.textContent = aAttr.value;
>@@ -982,25 +987,27 @@ ElementEditor.prototype = {
>   /**
>    * Parse a user-entered attribute string and apply the resulting
>    * attributes to the node.  This operation is undoable.
>    *
>    * @param string aValue the user-entered value.
>    * @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;
> 
>     this.undo.startBatch();
> 
>     for (let i = 0; i < attrs.length; i++) {

Comment 4

[N Bosma]

Attachment: Fix.

Comment 5

[Dave Camp (:dcamp)]

Looks good, thanks!

Comment 6

[Mihai Sucan [:msucan]]

No test?

Comment 7

[N Bosma]

Attachment: Further fix, and testcases

Tests and fix.

Comment 8

[Dave Camp (:dcamp)]

Comment on attachment 675686 [details] [diff] [review]
Further fix, and testcases

Awesome, thanks!

Comment 9

[Panos Astithas (he/him) [:past] (please ni?)]

https://hg.mozilla.org/integration/fx-team/rev/a31ad3a90e4a

Comment 10

[Panos Astithas (he/him) [:past] (please ni?)]

Backed out on suspicion of causing mochitest-1 and 3 permaorange:
https://hg.mozilla.org/integration/fx-team/rev/bb37616b7145

Comment 11

[Panos Astithas (he/him) [:past] (please ni?)]

Relanded:
https://hg.mozilla.org/integration/fx-team/rev/ea6521dc8ee9

Comment 12

[Panos Astithas (he/him) [:past] (please ni?)]

https://hg.mozilla.org/mozilla-central/rev/ea6521dc8ee9