Web Inspector: tabbing in Styles sidebar is broken when additional ":" and ";" are...
authorwebkit@devinrousso.com <webkit@devinrousso.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Apr 2017 05:26:44 +0000 (05:26 +0000)
committerwebkit@devinrousso.com <webkit@devinrousso.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Apr 2017 05:26:44 +0000 (05:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=170184

Reviewed by Matt Baker.

Source/WebInspectorUI:

New tests:
- inspector/unit-tests/text-utilities.html

* UserInterface/Base/TextUtilities.js: Added.
(WebInspector.rangeForNextCSSNameOrValue):
Consolidate logic for finding the next range to highlight in a CSS string given a starting index.

* UserInterface/Main.html:
* UserInterface/Test.html:
Include TextUtilities.

* UserInterface/Views/CSSStyleDeclarationTextEditor.js:
(WebInspector.CSSStyleDeclarationTextEditor.prototype._highlightNextNameOrValue):
(WebInspector.CSSStyleDeclarationTextEditor.prototype._rangeForNextNameOrValue):
(WebInspector.CSSStyleDeclarationTextEditor.prototype._handleMouseUp):
Utilize TextUtilities for range-finding logic.

LayoutTests:

* inspector/unit-tests/text-utilities-expected.txt: Added.
* inspector/unit-tests/text-utilities.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/inspector/unit-tests/text-utilities-expected.txt [new file with mode: 0644]
LayoutTests/inspector/unit-tests/text-utilities.html [new file with mode: 0644]
Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Base/TextUtilities.js [new file with mode: 0644]
Source/WebInspectorUI/UserInterface/Main.html
Source/WebInspectorUI/UserInterface/Test.html
Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js

index 0546e99..fab92f0 100644 (file)
@@ -1,3 +1,13 @@
+2017-04-09  Devin Rousso  <webkit@devinrousso.com>
+
+        Web Inspector: tabbing in Styles sidebar is broken when additional ":" and ";" are in the property value
+        https://bugs.webkit.org/show_bug.cgi?id=170184
+
+        Reviewed by Matt Baker.
+
+        * inspector/unit-tests/text-utilities-expected.txt: Added.
+        * inspector/unit-tests/text-utilities.html: Added.
+
 2017-04-09  Youenn Fablet  <youenn@apple.com>
 
         Resync WPT tests up to 23cd07d4685d81024b7440e042f8bbdb33e7ddec
diff --git a/LayoutTests/inspector/unit-tests/text-utilities-expected.txt b/LayoutTests/inspector/unit-tests/text-utilities-expected.txt
new file mode 100644 (file)
index 0000000..4df880c
--- /dev/null
@@ -0,0 +1,22 @@
+Testing basic functionality of functions defined in TextUtilities.js.
+
+
+== Running test suite: TextUtilities
+-- Running test case: WebInspector.rangeForNextCSSNameOrValue
+PASS: Next name/value token in "" starting at index 0 is "" [0, 0]
+PASS: Next name/value token in "" starting at index 2 is "" [0, 0]
+PASS: Next name/value token in "foo" starting at index 0 is "foo" [0, 3]
+PASS: Next name/value token in "foo" starting at index 2 is "foo" [0, 3]
+PASS: Next name/value token in "foo:bar" starting at index 0 is "foo" [0, 3]
+PASS: Next name/value token in "foo:bar" starting at index 2 is "foo" [0, 3]
+PASS: Next name/value token in "foo:bar" starting at index 3 is "bar" [4, 7]
+PASS: Next name/value token in "foo:bar" starting at index 5 is "bar" [4, 7]
+PASS: Next name/value token in "foo:  bar  ;" starting at index 0 is "foo" [0, 3]
+PASS: Next name/value token in "foo:  bar  ;" starting at index 2 is "foo" [0, 3]
+PASS: Next name/value token in "foo:  bar  ;" starting at index 3 is "bar" [6, 9]
+PASS: Next name/value token in "foo:  bar  ;" starting at index 5 is "bar" [6, 9]
+PASS: Next name/value token in "foo: url(http://baz);" starting at index 0 is "foo" [0, 3]
+PASS: Next name/value token in "foo: url(http://baz);" starting at index 2 is "foo" [0, 3]
+PASS: Next name/value token in "foo: url(http://baz);" starting at index 3 is "url(http://baz)" [5, 20]
+PASS: Next name/value token in "foo: url(http://baz);" starting at index 5 is "url(http://baz)" [5, 20]
+
diff --git a/LayoutTests/inspector/unit-tests/text-utilities.html b/LayoutTests/inspector/unit-tests/text-utilities.html
new file mode 100644 (file)
index 0000000..1bf753a
--- /dev/null
@@ -0,0 +1,55 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../http/tests/inspector/resources/inspector-test.js"></script>
+<script>
+function test()
+{
+    let suite = InspectorTest.createSyncSuite("TextUtilities");
+
+    suite.addTestCase({
+        name: "WebInspector.rangeForNextCSSNameOrValue",
+        test() {
+            function testValid(text, index, expected) {
+                let actual = WebInspector.rangeForNextCSSNameOrValue(text, index);
+                InspectorTest.expectShallowEqual(actual, expected, `Next name/value token in "${text}" starting at index ${index} is "${text.substring(actual.from, actual.to)}" [${actual.from}, ${actual.to}]`);
+            }
+
+            // Test empty string and out-of-bounds index
+            testValid("", 0, {from: 0, to: 0});
+            testValid("", 2, {from: 0, to: 0});
+
+            // Test basic string
+            testValid("foo", 0, {from: 0, to: 3});
+            testValid("foo", 2, {from: 0, to: 3});
+
+            // Test string with single colon
+            testValid("foo:bar", 0, {from: 0, to: 3});
+            testValid("foo:bar", 2, {from: 0, to: 3});
+            testValid("foo:bar", 3, {from: 4, to: 7});
+            testValid("foo:bar", 5, {from: 4, to: 7});
+
+            // Test string with extra whitespace
+            testValid("foo:  bar  ;", 0, {from: 0, to: 3});
+            testValid("foo:  bar  ;", 2, {from: 0, to: 3});
+            testValid("foo:  bar  ;", 3, {from: 6, to: 9});
+            testValid("foo:  bar  ;", 5, {from: 6, to: 9});
+
+            // Test string with multiple colons
+            testValid("foo: url(http://baz);", 0, {from: 0, to: 3});
+            testValid("foo: url(http://baz);", 2, {from: 0, to: 3});
+            testValid("foo: url(http://baz);", 3, {from: 5, to: 20});
+            testValid("foo: url(http://baz);", 5, {from: 5, to: 20});
+
+            return true;
+        }
+    });
+
+    suite.runTestCasesAndFinish();
+}
+</script>
+</head>
+<body onLoad="runTest()">
+    <p>Testing basic functionality of functions defined in TextUtilities.js.</p>
+</body>
+</html>
index 11fb414..a85bea3 100644 (file)
@@ -1,3 +1,27 @@
+2017-04-09  Devin Rousso  <webkit@devinrousso.com>
+
+        Web Inspector: tabbing in Styles sidebar is broken when additional ":" and ";" are in the property value
+        https://bugs.webkit.org/show_bug.cgi?id=170184
+
+        Reviewed by Matt Baker.
+
+        New tests:
+        - inspector/unit-tests/text-utilities.html
+
+        * UserInterface/Base/TextUtilities.js: Added.
+        (WebInspector.rangeForNextCSSNameOrValue):
+        Consolidate logic for finding the next range to highlight in a CSS string given a starting index.
+
+        * UserInterface/Main.html:
+        * UserInterface/Test.html:
+        Include TextUtilities.
+
+        * UserInterface/Views/CSSStyleDeclarationTextEditor.js:
+        (WebInspector.CSSStyleDeclarationTextEditor.prototype._highlightNextNameOrValue):
+        (WebInspector.CSSStyleDeclarationTextEditor.prototype._rangeForNextNameOrValue):
+        (WebInspector.CSSStyleDeclarationTextEditor.prototype._handleMouseUp):
+        Utilize TextUtilities for range-finding logic.
+
 2017-04-08  Simon Fraser  <simon.fraser@apple.com>
 
         Unprefix CSS cursor values grab and grabbing
diff --git a/Source/WebInspectorUI/UserInterface/Base/TextUtilities.js b/Source/WebInspectorUI/UserInterface/Base/TextUtilities.js
new file mode 100644 (file)
index 0000000..403154a
--- /dev/null
@@ -0,0 +1,46 @@
+/*
+ * Copyright (C) 2017 Devin Rousso <webkit@devinrousso.com>. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+WebInspector.rangeForNextCSSNameOrValue = function(text, index = 0) {
+    let from = 0;
+    let to = 0;
+
+    let colonIndex = text.indexOf(":");
+    if (index < colonIndex) {
+        from = 0;
+        to = colonIndex;
+    } else {
+        from = colonIndex + 1;
+        to = text.length;
+    }
+
+    let substring = text.substring(from, to);
+
+    // Don't select leading/trailing whitespace.
+    from += substring.match(/^\s*/)[0].length;
+    to -= substring.match(/[\s\;]*$/)[0].length;
+
+    return {from, to};
+};
index 1c77d6e..fd803e2 100644 (file)
     <script src="Base/ImageUtilities.js"></script>
     <script src="Base/LoadLocalizedStrings.js"></script>
     <script src="Base/MIMETypeUtilities.js"></script>
+    <script src="Base/TextUtilities.js"></script>
     <script src="Base/URLUtilities.js"></script>
     <script src="Base/Utilities.js"></script>
     <script src="Base/Setting.js"></script>
index d8e8ddb..ab3b304 100644 (file)
@@ -51,6 +51,7 @@
     <script src="Base/DOMUtilities.js"></script>
     <script src="Base/EventListener.js"></script>
     <script src="Base/EventListenerSet.js"></script>
+    <script src="Base/TextUtilities.js"></script>
     <script src="Base/URLUtilities.js"></script>
     <script src="Base/Utilities.js"></script>
     <script src="Base/Setting.js"></script>
index 85e845c..7023233 100644 (file)
@@ -401,25 +401,27 @@ WebInspector.CSSStyleDeclarationTextEditor = class CSSStyleDeclarationTextEditor
 
     _highlightNextNameOrValue(codeMirror, cursor, text)
     {
-        var nextAnchor;
-        var nextHead;
-
-        if (this._textAtCursorIsComment(codeMirror, cursor)) {
-            nextAnchor = 0;
-            nextHead = text.length;
-        } else {
-            var colonIndex = text.indexOf(":");
-            var substringIndex = colonIndex >= 0 && cursor.ch >= colonIndex ? colonIndex : 0;
+        let range = this._rangeForNextNameOrValue(codeMirror, cursor, text);
+        codeMirror.setSelection(range.from, range.to);
+    }
 
-            var regExp = /(?:[^:;\s]\s*)+/g;
-            regExp.lastIndex = substringIndex;
-            var match = regExp.exec(text);
+    _rangeForNextNameOrValue(codeMirror, cursor, text)
+    {
+        let nextAnchor = 0;
+        let nextHead = 0;
 
-            nextAnchor = match.index;
-            nextHead = nextAnchor + match[0].length;
+        if (this._textAtCursorIsComment(codeMirror, cursor))
+            nextHead = text.length;
+        else {
+            let range = WebInspector.rangeForNextCSSNameOrValue(text, cursor.ch);
+            nextAnchor = range.from;
+            nextHead = range.to;
         }
 
-        codeMirror.setSelection({line: cursor.line, ch: nextAnchor}, {line: cursor.line, ch: nextHead});
+        return {
+            from: {line: cursor.line, ch: nextAnchor},
+            to: {line: cursor.line, ch: nextHead},
+        };
     }
 
     _handleMouseDown(event)
@@ -457,41 +459,17 @@ WebInspector.CSSStyleDeclarationTextEditor = class CSSStyleDeclarationTextEditor
                     this._codeMirror.replaceRange(replacement, cursor);
                 }
             } else if (WebInspector.settings.stylesSelectOnFirstClick.value && this._mouseDownCursorPosition.previousRange) {
-                let from = {line: cursor.line, ch: 0};
-                let to = {line: cursor.line, ch: 0};
-
-                let colonIndex = line.indexOf(":");
-                if (colonIndex === -1) // Select entire line if unable to find colon, such as for a comment.
-                    colonIndex = line.length;
-
-                let text = line;
-
-                if (cursor.ch <= colonIndex) {
-                    text = text.substring(0, colonIndex);
-
-                    to.ch += colonIndex;
-                } else {
-                    text = text.substring(colonIndex + 1);
-
-                    from.ch += colonIndex + 1;
-                    to.ch += line.length;
-                }
-
-                let leadingSpacesCount = text.match(/^\s*/)[0].length;
-                let trailingNonWordCount = text.match(/[\s\;]*$/)[0].length;
-
-                from.ch += leadingSpacesCount;
-                to.ch -= trailingNonWordCount;
+                let range = this._rangeForNextNameOrValue(this._codeMirror, cursor, line);
 
                 let clickedDifferentLine = this._mouseDownCursorPosition.previousRange.from.line !== cursor.line || this._mouseDownCursorPosition.previousRange.to.line !== cursor.line;
                 let cursorInPreviousRange = cursor.ch >= this._mouseDownCursorPosition.previousRange.from.ch && cursor.ch <= this._mouseDownCursorPosition.previousRange.to.ch;
-                let previousInNewRange = this._mouseDownCursorPosition.previousRange.from.ch >= from.ch && this._mouseDownCursorPosition.previousRange.to.ch <= to.ch;
+                let previousInNewRange = this._mouseDownCursorPosition.previousRange.from.ch >= range.from.ch && this._mouseDownCursorPosition.previousRange.to.ch <= range.to.ch;
 
                 // Only select the new range if the editor is not focused, a new line is being clicked,
                 // or the new cursor position is outside of the previous range and the previous range is
                 // outside of the new range (meaning you're not clicking in the same area twice).
                 if (!this._codeMirror.hasFocus() || clickedDifferentLine || (!cursorInPreviousRange && !previousInNewRange))
-                    this._codeMirror.setSelection(from, to);
+                    this._codeMirror.setSelection(range.from, range.to);
             }
         }