Web Inspector: [PARITY] Styles Redesign: Ability to modify style attributes
authornvasilyev@apple.com <nvasilyev@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Oct 2017 23:11:06 +0000 (23:11 +0000)
committernvasilyev@apple.com <nvasilyev@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Oct 2017 23:11:06 +0000 (23:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=178328
<rdar://problem/35000990>

Reviewed by Joseph Pecoraro.

Source/WebInspectorUI:

Before this patch, modifying a property of a style attribute duplicated the old property
and appended the new one. This happened because WI.TextRange.prototype.resolveOffsets
didn't resolve offsets correctly when text didn't end with a trailing new line.

Since WI.TextRange.prototype.resolveOffsets is used elsewhere and this could be expected
behavior, append a line break before resolving a range.

* UserInterface/Models/CSSProperty.js:
(WI.CSSProperty.prototype._updateOwnerStyleText):

LayoutTests:

Add tests for WI.TextRange.protopyte.resolveOffsets.

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

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

LayoutTests/ChangeLog
LayoutTests/inspector/unit-tests/text-range-expected.txt [new file with mode: 0644]
LayoutTests/inspector/unit-tests/text-range.html [new file with mode: 0644]
Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Models/CSSProperty.js

index 5e4c9bf..8769f1b 100644 (file)
@@ -1,3 +1,16 @@
+2017-10-30  Nikita Vasilyev  <nvasilyev@apple.com>
+
+        Web Inspector: [PARITY] Styles Redesign: Ability to modify style attributes
+        https://bugs.webkit.org/show_bug.cgi?id=178328
+        <rdar://problem/35000990>
+
+        Reviewed by Joseph Pecoraro.
+
+        Add tests for WI.TextRange.protopyte.resolveOffsets.
+
+        * inspector/unit-tests/text-range-expected.txt: Added.
+        * inspector/unit-tests/text-range.html: Added.
+
 2017-10-30  Matt Lewis  <jlewis3@apple.com>
 
         Followup rebaseline to r224204.
diff --git a/LayoutTests/inspector/unit-tests/text-range-expected.txt b/LayoutTests/inspector/unit-tests/text-range-expected.txt
new file mode 100644 (file)
index 0000000..8340c57
--- /dev/null
@@ -0,0 +1,16 @@
+Testing that WI.TextRange works.
+
+
+== Running test suite: WI.TextRange
+-- Running test case: resolveOffsetsNoTrailingLineBreak
+PASS: startOffset should be 1.
+PASS: endOffset should be 0.
+
+-- Running test case: resolveOffsetsTrailingLineBreak
+PASS: startOffset should be 1.
+PASS: endOffset should be 14.
+
+-- Running test case: resolveOffsetsTwoTrailingLineBreaks
+PASS: startOffset should be 1.
+PASS: endOffset should be 14.
+
diff --git a/LayoutTests/inspector/unit-tests/text-range.html b/LayoutTests/inspector/unit-tests/text-range.html
new file mode 100644 (file)
index 0000000..a23cab7
--- /dev/null
@@ -0,0 +1,76 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../http/tests/inspector/resources/inspector-test.js"></script>
+<script>
+function test() {
+    let suite = InspectorTest.createSyncSuite("WI.TextRange");
+
+    suite.addTestCase({
+        name: "resolveOffsetsNoTrailingLineBreak",
+        description: "Ensure endOffset is zero when there's no trailing line break.",
+        test() {
+            let startLine = 1;
+            let startColumn = 0;
+            let endLine = 2;
+            let endColumn = 0;
+            let range = new WI.TextRange(startLine, startColumn, endLine, endColumn);
+
+            let text = "\ncolor: blue;";
+            range.resolveOffsets(text);
+
+            InspectorTest.expectEqual(range.startOffset, "\n".length, "startOffset should be 1.");
+            InspectorTest.expectEqual(range.endOffset, 0, "endOffset should be 0.");
+
+            return true;
+        }
+    });
+
+    suite.addTestCase({
+        name: "resolveOffsetsTrailingLineBreak",
+        description: "Ensure endOffset is not zero when there's a trailing line break.",
+        test() {
+            let startLine = 1;
+            let startColumn = 0;
+            let endLine = 2;
+            let endColumn = 0;
+            let range = new WI.TextRange(startLine, startColumn, endLine, endColumn);
+
+            let text = "\ncolor: blue;\n";
+            range.resolveOffsets(text);
+
+            InspectorTest.expectEqual(range.startOffset, "\n".length, "startOffset should be 1.");
+            InspectorTest.expectEqual(range.endOffset, text.length, "endOffset should be 14.");
+
+            return true;
+        }
+    });
+
+    suite.addTestCase({
+        name: "resolveOffsetsTwoTrailingLineBreaks",
+        description: "Ensure endOffset is not zero when there're two trailing line breaks.",
+        test() {
+            let startLine = 1;
+            let startColumn = 0;
+            let endLine = 2;
+            let endColumn = 0;
+            let range = new WI.TextRange(startLine, startColumn, endLine, endColumn);
+
+            let text = "\ncolor: blue;\n\n";
+            range.resolveOffsets(text);
+
+            InspectorTest.expectEqual(range.startOffset, "\n".length, "startOffset should be 1.");
+            InspectorTest.expectEqual(range.endOffset, 14, "endOffset should be 14.");
+
+            return true;
+        }
+    });
+
+    suite.runTestCasesAndFinish();
+}
+</script>
+</head>
+<body onload="runTest()">
+    <p>Testing that WI.TextRange works.</p>
+</body>
+</html>
index be137e6..2b00fc8 100644 (file)
@@ -1,3 +1,21 @@
+2017-10-30  Nikita Vasilyev  <nvasilyev@apple.com>
+
+        Web Inspector: [PARITY] Styles Redesign: Ability to modify style attributes
+        https://bugs.webkit.org/show_bug.cgi?id=178328
+        <rdar://problem/35000990>
+
+        Reviewed by Joseph Pecoraro.
+
+        Before this patch, modifying a property of a style attribute duplicated the old property
+        and appended the new one. This happened because WI.TextRange.prototype.resolveOffsets
+        didn't resolve offsets correctly when text didn't end with a trailing new line.
+
+        Since WI.TextRange.prototype.resolveOffsets is used elsewhere and this could be expected
+        behavior, append a line break before resolving a range.
+
+        * UserInterface/Models/CSSProperty.js:
+        (WI.CSSProperty.prototype._updateOwnerStyleText):
+
 2017-10-30  Ross Kirsling  <ross.kirsling@sony.com>
 
         Web Inspector: Clicking filler in data grid should clear selection
index c2671d5..8706494 100644 (file)
@@ -355,7 +355,9 @@ WI.CSSProperty = class CSSProperty extends WI.Object
         // _styleSheetTextRange is the position of the property within the stylesheet.
         // range is the position of the property within the rule.
         let range = this._styleSheetTextRange.relativeTo(this._ownerStyle.styleSheetTextRange.startLine, this._ownerStyle.styleSheetTextRange.startColumn);
-        range.resolveOffsets(styleText);
+
+        // Append a line break to count the last line of styleText towards endOffset.
+        range.resolveOffsets(styleText + "\n");
 
         console.assert(oldText === styleText.slice(range.startOffset, range.endOffset), "_styleSheetTextRange data is invalid.");