Web Inspector: [PARITY] Styles Redesign: Ability to modify style attr…

…ibutes 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. Canonical link: https://commits.webkit.org/195173@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@224210 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Oct 30, 2017 · 334e3f1219b5deca9932d69f754288b27f3b7e8b · 334e3f1
1 parent 0faa459
commit 334e3f1219b5deca9932d69f754288b27f3b7e8b
Showing 5 changed files with 126 additions and 1 deletion.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -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
@@ -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
@@ -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>
diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
@@ -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

diff --git a/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js b/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js
@@ -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.");