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

RESOLVED FIXED in Firefox 19

Status

()

Firefox
Developer Tools: Inspector
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: N Bosma, Assigned: N Bosma)

Tracking

Trunk
Firefox 19
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

532 bytes, text/plain
Details
7.18 KB, patch
dcamp
: review+
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
Created attachment 674052 [details]
Example scenario.
(Assignee)

Comment 2

5 years ago
Created attachment 674053 [details]
Fix
(Assignee)

Comment 3

5 years ago
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++) {
(Assignee)

Updated

5 years ago
Attachment #674053 - Attachment is obsolete: true
Attachment #674053 - Attachment is patch: false
(Assignee)

Comment 4

5 years ago
Created attachment 674058 [details] [diff] [review]
Fix.
(Assignee)

Updated

5 years ago
Attachment #674058 - Flags: review?(dcamp)

Updated

5 years ago
Attachment #674058 - Flags: review?(dcamp) → review+

Comment 5

5 years ago
Looks good, thanks!
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [land-in-fx-team]
No test?
(Assignee)

Comment 7

5 years ago
Created attachment 675686 [details] [diff] [review]
Further fix, and testcases

Tests and fix.
Attachment #674058 - Attachment is obsolete: true
Attachment #675686 - Flags: review?(dcamp)

Comment 8

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

Awesome, thanks!
Attachment #675686 - Flags: review?(dcamp) → review+
https://hg.mozilla.org/integration/fx-team/rev/a31ad3a90e4a
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
Backed out on suspicion of causing mochitest-1 and 3 permaorange:
https://hg.mozilla.org/integration/fx-team/rev/bb37616b7145
Whiteboard: [fixed-in-fx-team]
Relanded:
https://hg.mozilla.org/integration/fx-team/rev/ea6521dc8ee9
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/ea6521dc8ee9
Assignee: nobody → dev.nibo
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 19
You need to log in before you can comment on or make changes to this bug.