Last Comment Bug 804400 - Inspector stops updating when badly formed attribute input is given in the markup panel.
: Inspector stops updating when badly formed attribute input is given in the ma...
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Developer Tools: Inspector (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: Firefox 19
Assigned To: N Bosma
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-22 16:45 PDT by N Bosma
Modified: 2012-11-01 00:44 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Example scenario. (532 bytes, text/plain)
2012-10-22 16:50 PDT, N Bosma
no flags Details
Fix (2.48 KB, text/plain)
2012-10-22 16:51 PDT, N Bosma
no flags Details
Fix. (2.48 KB, patch)
2012-10-22 17:04 PDT, N Bosma
dcamp: review+
Details | Diff | Splinter Review
Further fix, and testcases (7.18 KB, patch)
2012-10-26 14:07 PDT, N Bosma
dcamp: review+
Details | Diff | Splinter Review

Description N Bosma 2012-10-22 16:45:31 PDT
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 2012-10-22 16:50:29 PDT
Created attachment 674052 [details]
Example scenario.
Comment 2 N Bosma 2012-10-22 16:51:27 PDT
Created attachment 674053 [details]
Fix
Comment 3 N Bosma 2012-10-22 16:58:00 PDT
Comment on attachment 674053 [details]
Fix

># HG changeset patch
># User N Bosma <dev.nibo@gmail.com>
># 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 2012-10-22 17:04:37 PDT
Created attachment 674058 [details] [diff] [review]
Fix.
Comment 5 Dave Camp (:dcamp) 2012-10-24 08:13:01 PDT
Looks good, thanks!
Comment 6 Mihai Sucan [:msucan] 2012-10-25 04:58:51 PDT
No test?
Comment 7 N Bosma 2012-10-26 14:07:30 PDT
Created attachment 675686 [details] [diff] [review]
Further fix, and testcases

Tests and fix.
Comment 8 Dave Camp (:dcamp) 2012-10-26 14:09:38 PDT
Comment on attachment 675686 [details] [diff] [review]
Further fix, and testcases

Awesome, thanks!
Comment 9 Panos Astithas [:past] 2012-10-31 01:33:39 PDT
https://hg.mozilla.org/integration/fx-team/rev/a31ad3a90e4a
Comment 10 Panos Astithas [:past] 2012-10-31 04:45:50 PDT
Backed out on suspicion of causing mochitest-1 and 3 permaorange:
https://hg.mozilla.org/integration/fx-team/rev/bb37616b7145
Comment 11 Panos Astithas [:past] 2012-10-31 09:39:21 PDT
Relanded:
https://hg.mozilla.org/integration/fx-team/rev/ea6521dc8ee9
Comment 12 Panos Astithas [:past] 2012-11-01 00:44:21 PDT
https://hg.mozilla.org/mozilla-central/rev/ea6521dc8ee9

Note You need to log in before you can comment on or make changes to this bug.