2011-04-29 Pavel Feldman <pfeldman@google.com>
authorpfeldman@chromium.org <pfeldman@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Apr 2011 15:35:14 +0000 (15:35 +0000)
committerpfeldman@chromium.org <pfeldman@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Apr 2011 15:35:14 +0000 (15:35 +0000)
        Reviewed by Yury Semikhatsky.

        Web Inspector: CSS editing breaks when entering "color: rgb(1"
        https://bugs.webkit.org/show_bug.cgi?id=59789

        * inspector/styles/styles-add-invalid-property-expected.txt: Added.
        * inspector/styles/styles-add-invalid-property.html: Added.
        * inspector/styles/styles-cancel-editing-expected.txt: Added.
        * inspector/styles/styles-cancel-editing.html: Added.
        * inspector/styles/styles-commit-editing-expected.txt: Added.
        * inspector/styles/styles-commit-editing.html: Added.
2011-04-29  Pavel Feldman  <pfeldman@google.com>

        Reviewed by Yury Semikhatsky.

        Web Inspector: CSS editing breaks when entering "color: rgb(1"
        https://bugs.webkit.org/show_bug.cgi?id=59789

        Tests: inspector/styles/styles-add-invalid-property.html
               inspector/styles/styles-cancel-editing.html
               inspector/styles/styles-commit-editing.html

        * inspector/front-end/CSSStyleModel.js:
        (WebInspector.CSSProperty.prototype.setText.callback):
        (WebInspector.CSSProperty.prototype.setText):
        * inspector/front-end/StylesSidebarPane.js:
        (WebInspector.StylePropertyTreeElement.prototype.selectElement):
        (WebInspector.StylePropertyTreeElement.prototype):
        (WebInspector.StylePropertyTreeElement.prototype.styleText.updateInterface.majorChange.isRevert.originalPropertyText):
        * inspector/front-end/inspector.js:
        (WebInspector.startEditing.defaultFinishHandler):
        (WebInspector.startEditing.keyDownEventListener):
        (WebInspector.startEditing):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@85324 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/inspector/styles/styles-add-invalid-property-expected.txt [new file with mode: 0644]
LayoutTests/inspector/styles/styles-add-invalid-property.html [new file with mode: 0644]
LayoutTests/inspector/styles/styles-cancel-editing-expected.txt [new file with mode: 0644]
LayoutTests/inspector/styles/styles-cancel-editing.html [new file with mode: 0644]
LayoutTests/inspector/styles/styles-commit-editing-expected.txt [new file with mode: 0644]
LayoutTests/inspector/styles/styles-commit-editing.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/inspector/front-end/CSSStyleModel.js
Source/WebCore/inspector/front-end/StylesSidebarPane.js
Source/WebCore/inspector/front-end/inspector.js

index 960c25b..0bf06de 100644 (file)
@@ -1,3 +1,17 @@
+2011-04-29  Pavel Feldman  <pfeldman@google.com>
+
+        Reviewed by Yury Semikhatsky.
+
+        Web Inspector: CSS editing breaks when entering "color: rgb(1"
+        https://bugs.webkit.org/show_bug.cgi?id=59789
+
+        * inspector/styles/styles-add-invalid-property-expected.txt: Added.
+        * inspector/styles/styles-add-invalid-property.html: Added.
+        * inspector/styles/styles-cancel-editing-expected.txt: Added.
+        * inspector/styles/styles-cancel-editing.html: Added.
+        * inspector/styles/styles-commit-editing-expected.txt: Added.
+        * inspector/styles/styles-commit-editing.html: Added.
+
 2011-04-29  Pavel Podivilov  <podivilov@chromium.org>
 
         Reviewed by Pavel Feldman.
diff --git a/LayoutTests/inspector/styles/styles-add-invalid-property-expected.txt b/LayoutTests/inspector/styles/styles-add-invalid-property-expected.txt
new file mode 100644 (file)
index 0000000..1fcc3b6
--- /dev/null
@@ -0,0 +1,22 @@
+Tests that adding an invalid property retains its syntax.
+
+Text
+Before append:
+[expanded] element.style { ()
+font-size: 12px;
+
+======== Matched CSS Rules ========
+[expanded] div { (user agent stylesheet)
+display: block;
+
+
+After append:
+[expanded] element.style { ()
+font-size: 12px;
+
+======== Matched CSS Rules ========
+[expanded] div { (user agent stylesheet)
+display: block;
+
+
+
diff --git a/LayoutTests/inspector/styles/styles-add-invalid-property.html b/LayoutTests/inspector/styles/styles-add-invalid-property.html
new file mode 100644 (file)
index 0000000..8af4ffe
--- /dev/null
@@ -0,0 +1,76 @@
+<html>
+<head>
+<script src="../../http/tests/inspector/inspector-test.js"></script>
+<script src="../../http/tests/inspector/elements-test.js"></script>
+<script>
+
+function test()
+{
+    WebInspector.showPanel("elements");
+    InspectorTest.selectNodeWithId("inspected", step0);
+
+    var treeElement;
+    var section;
+
+    function step0()
+    {
+        InspectorTest.addResult("Before append:");
+        InspectorTest.dumpSelectedElementStyles(true);
+        section = WebInspector.panels.elements.sidebarPanes.styles.sections[0][1];
+        section.expand();
+        InspectorTest.runAfterPendingDispatches(step1);
+    }
+
+    function step1()
+    {
+        // Create and increment.
+        treeElement = section.addNewBlankProperty();
+        treeElement.startEditing();
+        treeElement.nameElement.textContent = "color";
+        treeElement.nameElement.dispatchEvent(InspectorTest.createKeyEvent("Enter"));
+
+        // Update incrementally to a valid value. 
+        treeElement.valueElement.textContent = "rgb(";
+        treeElement.kickFreeFlowStyleEditForTest();
+        InspectorTest.runAfterPendingDispatches(step2);
+    }
+
+    function step2()
+    {
+        // Commit invalid value. 
+        treeElement.valueElement.textContent = "rgb(1";
+        treeElement.valueElement.dispatchEvent(InspectorTest.createKeyEvent("Enter"));
+        InspectorTest.runAfterPendingDispatches(step3);
+    }
+
+    function step3()
+    {
+        InspectorTest.selectNodeWithId("other", step4);
+    }
+
+    function step4()
+    {
+        InspectorTest.selectNodeWithId("inspected", step5);
+    }
+
+    function step5()
+    {
+        InspectorTest.addResult("After append:");
+        InspectorTest.dumpSelectedElementStyles(true);
+        InspectorTest.completeTest();
+    }
+}
+
+</script>
+</head>
+
+<body onload="runTest()">
+<p>
+Tests that adding an invalid property retains its syntax.
+</p>
+
+<div id="inspected" style="font-size: 12px">Text</div>
+<div id="other"></div>
+
+</body>
+</html>
diff --git a/LayoutTests/inspector/styles/styles-cancel-editing-expected.txt b/LayoutTests/inspector/styles/styles-cancel-editing-expected.txt
new file mode 100644 (file)
index 0000000..13749de
--- /dev/null
@@ -0,0 +1,21 @@
+Tests that editing is canceled properly after incremental editing.
+
+Text
+[expanded] element.style { ()
+color: red;
+
+======== Matched CSS Rules ========
+[expanded] div { (user agent stylesheet)
+display: block;
+
+
+After append:
+[expanded] element.style { ()
+color: red;
+
+======== Matched CSS Rules ========
+[expanded] div { (user agent stylesheet)
+display: block;
+
+
+
diff --git a/LayoutTests/inspector/styles/styles-cancel-editing.html b/LayoutTests/inspector/styles/styles-cancel-editing.html
new file mode 100644 (file)
index 0000000..c99fa31
--- /dev/null
@@ -0,0 +1,64 @@
+<html>
+<head>
+<script src="../../http/tests/inspector/inspector-test.js"></script>
+<script src="../../http/tests/inspector/elements-test.js"></script>
+<script>
+
+function test()
+{
+    WebInspector.showPanel("elements");
+    InspectorTest.selectNodeWithId("inspected", step1);
+
+    var treeElement;
+    var section;
+
+    function step1()
+    {
+        InspectorTest.dumpSelectedElementStyles(true);
+        treeElement = InspectorTest.getElementStylePropertyTreeItem("color");
+
+        treeElement.startEditing();
+        treeElement.nameElement.textContent = "color";
+        treeElement.nameElement.dispatchEvent(InspectorTest.createKeyEvent("Enter"));
+
+        // Update incrementally, do not commit.
+        treeElement.valueElement.textContent = "green";
+        treeElement.kickFreeFlowStyleEditForTest();
+
+        // Cancel editing.
+        treeElement.valueElement.firstChild.select();
+        treeElement.valueElement.dispatchEvent(InspectorTest.createKeyEvent("U+001B")); // Escape
+        InspectorTest.runAfterPendingDispatches(step2);
+    }
+
+    function step2()
+    {
+        InspectorTest.selectNodeWithId("other", step3);
+    }
+
+    function step3()
+    {
+        InspectorTest.selectNodeWithId("inspected", step4);
+    }
+
+    function step4()
+    {
+        InspectorTest.addResult("After append:");
+        InspectorTest.dumpSelectedElementStyles(true);
+        InspectorTest.completeTest();
+    }
+}
+
+</script>
+</head>
+
+<body onload="runTest()">
+<p>
+Tests that editing is canceled properly after incremental editing.
+</p>
+
+<div id="inspected" style="color: red">Text</div>
+<div id="other"></div>
+
+</body>
+</html>
diff --git a/LayoutTests/inspector/styles/styles-commit-editing-expected.txt b/LayoutTests/inspector/styles/styles-commit-editing-expected.txt
new file mode 100644 (file)
index 0000000..0c98ff3
--- /dev/null
@@ -0,0 +1,21 @@
+Tests that editing is canceled properly after incremental editing.
+
+Text
+[expanded] element.style { ()
+color: red;
+
+======== Matched CSS Rules ========
+[expanded] div { (user agent stylesheet)
+display: block;
+
+
+After append:
+[expanded] element.style { ()
+color: green;
+
+======== Matched CSS Rules ========
+[expanded] div { (user agent stylesheet)
+display: block;
+
+
+
diff --git a/LayoutTests/inspector/styles/styles-commit-editing.html b/LayoutTests/inspector/styles/styles-commit-editing.html
new file mode 100644 (file)
index 0000000..414ea22
--- /dev/null
@@ -0,0 +1,65 @@
+<html>
+<head>
+<script src="../../http/tests/inspector/inspector-test.js"></script>
+<script src="../../http/tests/inspector/elements-test.js"></script>
+<script>
+
+function test()
+{
+    WebInspector.showPanel("elements");
+    InspectorTest.selectNodeWithId("inspected", step1);
+
+    var treeElement;
+    var section;
+
+    function step1()
+    {
+        InspectorTest.dumpSelectedElementStyles(true);
+        treeElement = InspectorTest.getElementStylePropertyTreeItem("color");
+
+        treeElement.startEditing();
+        treeElement.nameElement.textContent = "color";
+        treeElement.nameElement.dispatchEvent(InspectorTest.createKeyEvent("Enter"));
+
+        // Update incrementally, do not commit.
+        treeElement.valueElement.textContent = "rgb(/*";
+        treeElement.kickFreeFlowStyleEditForTest();
+
+        // Commit editing.
+        treeElement.valueElement.textContent = "green";
+        treeElement.valueElement.firstChild.select();
+        treeElement.valueElement.dispatchEvent(InspectorTest.createKeyEvent("Enter"));
+        InspectorTest.runAfterPendingDispatches(step2);
+    }
+
+    function step2()
+    {
+        InspectorTest.selectNodeWithId("other", step3);
+    }
+
+    function step3()
+    {
+        InspectorTest.selectNodeWithId("inspected", step4);
+    }
+
+    function step4()
+    {
+        InspectorTest.addResult("After append:");
+        InspectorTest.dumpSelectedElementStyles(true);
+        InspectorTest.completeTest();
+    }
+}
+
+</script>
+</head>
+
+<body onload="runTest()">
+<p>
+Tests that editing is canceled properly after incremental editing.
+</p>
+
+<div id="inspected" style="color: red">Text</div>
+<div id="other"></div>
+
+</body>
+</html>
index 574c295..b9efdda 100644 (file)
@@ -1,3 +1,26 @@
+2011-04-29  Pavel Feldman  <pfeldman@google.com>
+
+        Reviewed by Yury Semikhatsky.
+
+        Web Inspector: CSS editing breaks when entering "color: rgb(1"
+        https://bugs.webkit.org/show_bug.cgi?id=59789
+
+        Tests: inspector/styles/styles-add-invalid-property.html
+               inspector/styles/styles-cancel-editing.html
+               inspector/styles/styles-commit-editing.html
+
+        * inspector/front-end/CSSStyleModel.js:
+        (WebInspector.CSSProperty.prototype.setText.callback):
+        (WebInspector.CSSProperty.prototype.setText):
+        * inspector/front-end/StylesSidebarPane.js:
+        (WebInspector.StylePropertyTreeElement.prototype.selectElement):
+        (WebInspector.StylePropertyTreeElement.prototype):
+        (WebInspector.StylePropertyTreeElement.prototype.styleText.updateInterface.majorChange.isRevert.originalPropertyText):
+        * inspector/front-end/inspector.js:
+        (WebInspector.startEditing.defaultFinishHandler):
+        (WebInspector.startEditing.keyDownEventListener):
+        (WebInspector.startEditing):
+
 2011-04-29  Tor Arne Vestbø  <tor.arne.vestbo@nokia.com>
 
         Reviewed by Simon Hausmann.
index e85fb6d..26771b7 100644 (file)
@@ -532,7 +532,6 @@ WebInspector.CSSProperty.prototype = {
 
                 WebInspector.cssModel._fireStyleSheetChanged(style.id.styleSheetId, majorChange, userCallback ? userCallback.bind(this, style) : null);
             } else {
-                console.error(JSON.stringify(error));
                 if (userCallback)
                     userCallback(null);
             }
index 8888d2c..27b13d6 100644 (file)
@@ -1641,7 +1641,7 @@ WebInspector.StylePropertyTreeElement.prototype = {
                 // Enter or colon (for name)/semicolon outside of string (for value).
                 event.preventDefault();
                 return "move-forward";
-            } else if (event.keyCode === WebInspector.KeyboardShortcut.Keys.Esc.code)
+            } else if (event.keyCode === WebInspector.KeyboardShortcut.Keys.Esc.code || event.keyIdentifier === "U+001B")
                 return "cancel";
             else if (!isEditingName && this._newProperty && event.keyCode === WebInspector.KeyboardShortcut.Keys.Backspace.code) {
                 // For a new property, when Backspace is pressed at the beginning of new property value, move back to the property name.
@@ -1679,6 +1679,7 @@ WebInspector.StylePropertyTreeElement.prototype = {
             return "move-forward";
         }
 
+        delete this.originalPropertyText;
         WebInspector.panels.elements.startEditingStyle();
         WebInspector.startEditing(selectElement, {
             context: context,
@@ -1702,7 +1703,7 @@ WebInspector.StylePropertyTreeElement.prototype = {
         this._applyFreeFlowStyleTextEdit();
     },
 
-    _applyFreeFlowStyleTextEdit: function()
+    _applyFreeFlowStyleTextEdit: function(now)
     {
         if (this._applyFreeFlowStyleTextEditTimer)
             clearTimeout(this._applyFreeFlowStyleTextEditTimer);
@@ -1711,7 +1712,15 @@ WebInspector.StylePropertyTreeElement.prototype = {
         {
             this.applyStyleText(this.nameElement.textContent + ": " + this.valueElement.textContent);
         }
-        this._applyFreeFlowStyleTextEditTimer = setTimeout(apply.bind(this), 100);
+        if (now)
+            apply.call(this);
+        else
+            this._applyFreeFlowStyleTextEditTimer = setTimeout(apply.bind(this), 100);
+    },
+
+    kickFreeFlowStyleEditForTest: function()
+    {
+        this._applyFreeFlowStyleTextEdit(true);
     },
 
     _handleUpOrDownKeyPressed: function(event)
@@ -1769,12 +1778,6 @@ WebInspector.StylePropertyTreeElement.prototype = {
             event.handled = true;
             event.preventDefault();
 
-            if (!("originalPropertyText" in this)) {
-                // Remember the rule's original CSS text on [Page](Up|Down), so it can be restored
-                // if the editing is canceled.
-                this.originalPropertyText = this.property.propertyText;
-            }
-
             // Synthesize property text disregarding any comments, custom whitespace etc.
             this.applyStyleText(this.nameElement.textContent + ": " + this.valueElement.textContent);
         }
@@ -1795,25 +1798,29 @@ WebInspector.StylePropertyTreeElement.prototype = {
         if (editedElement.parentElement)
             editedElement.parentElement.removeStyleClass("child-editing");
 
-        delete this.originalPropertyText;
         WebInspector.panels.elements.endEditingStyle();
     },
 
     editingCancelled: function(element, context)
     {
         this._removePrompt();
-        if ("originalPropertyText" in this)
-            this.applyStyleText(this.originalPropertyText, true);
-        else {
+        this._revertStyleUponEditingCanceled(this.originalPropertyText);
+        // This should happen last, as it clears the info necessary to restore the property value after [Page]Up/Down changes.
+        this.editingEnded(context);
+    },
+
+    _revertStyleUponEditingCanceled: function(originalPropertyText)
+    {
+        if (typeof originalPropertyText === "string") {
+            delete this.originalPropertyText;
+            this.applyStyleText(originalPropertyText, true, false, true);
+        } else {
             if (this._newProperty)
                 this.treeOutline.removeChild(this);
             else
                 this.updateTitle();
         }
-
-        // This should happen last, as it clears the info necessary to restore the property value after [Page]Up/Down changes.
-        this.editingEnded(context);
-    },
+    }, 
 
     editingCommitted: function(element, userInput, previousContent, context, moveDirection)
     {
@@ -1856,7 +1863,7 @@ WebInspector.StylePropertyTreeElement.prototype = {
                 else
                     propertyText = this.nameElement.textContent + ": " + userInput;
             }
-            this.applyStyleText(propertyText, true);
+            this.applyStyleText(propertyText, true, true);
         } else {
             if (!isDataPasted && !this._newProperty)
                 this.updateTitle();
@@ -1927,38 +1934,40 @@ WebInspector.StylePropertyTreeElement.prototype = {
         }
     },
 
-    _hasBeenAppliedToPageViaUpDown: function()
+    _hasBeenModifiedIncrementally: function()
     {
         // New properties applied via up/down have an originalPropertyText and will be deleted later
         // on, if cancelled, when the empty string gets applied as their style text.
-        return ("originalPropertyText" in this);
+        return typeof this.originalPropertyText === "string";
     },
 
-    applyStyleText: function(styleText, updateInterface)
+    applyStyleText: function(styleText, updateInterface, majorChange, isRevert)
     {
+        // Leave a way to cancel editing after incremental changes.
+        if (!isRevert && !updateInterface && !this._hasBeenModifiedIncrementally()) {
+            // Remember the rule's original CSS text on [Page](Up|Down), so it can be restored
+            // if the editing is canceled.
+            this.originalPropertyText = this.property.propertyText;
+        }
+
         var section = this.treeOutline.section;
         var elementsPanel = WebInspector.panels.elements;
         styleText = styleText.replace(/\s/g, " ").trim(); // Replace &nbsp; with whitespace.
         var styleTextLength = styleText.length;
-        if (!styleTextLength && updateInterface && this._newProperty && !this._hasBeenAppliedToPageViaUpDown()) {
+        if (!styleTextLength && updateInterface && !isRevert && this._newProperty && !this._hasBeenModifiedIncrementally()) {
             // The user deleted everything and never applied a new property value via Up/Down scrolling, so remove the tree element and update.
             this.parent.removeChild(this);
             section.afterUpdate();
             return;
         }
 
-        function callback(newStyle)
+        function callback(originalPropertyText, newStyle)
         {
             if (!newStyle) {
-                // The user typed something, but it didn't parse. Just abort and restore
-                // the original title for this property.  If this was a new attribute and
-                // we couldn't parse, then just remove it.
-                if (this._newProperty) {
-                    this.parent.removeChild(this);
-                    return;
+                if (updateInterface) {
+                    // It did not apply, cancel editing.
+                    this._revertStyleUponEditingCanceled(originalPropertyText);
                 }
-                if (updateInterface)
-                    this.updateTitle();
                 return;
             }
 
@@ -1977,7 +1986,7 @@ WebInspector.StylePropertyTreeElement.prototype = {
         // FIXME: this does not handle trailing comments.
         if (styleText.length && !/;\s*$/.test(styleText))
             styleText += ";";
-        this.property.setText(styleText, updateInterface, callback.bind(this));
+        this.property.setText(styleText, majorChange, callback.bind(this, this.originalPropertyText));
     }
 }
 
index c37c28b..7d1eaf5 100644 (file)
@@ -1519,7 +1519,7 @@ WebInspector.startEditing = function(element, config)
             event.ctrlKey && !event.shiftKey && !event.metaKey && !event.altKey;
         if (isEnterKey(event) && (event.isMetaOrCtrlForTest || !config.multiline || isMetaOrCtrl))
             return "commit";
-        else if (event.keyCode === WebInspector.KeyboardShortcut.Keys.Esc.code)
+        else if (event.keyCode === WebInspector.KeyboardShortcut.Keys.Esc.code || event.keyIdentifier === "U+001B")
             return "cancel";
         else if (event.keyIdentifier === "U+0009") // Tab key
             return "move-" + (event.shiftKey ? "backward" : "forward");