The default bug view has changed. See this FAQ.

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.