Title: [224210] trunk
Revision
224210
Author
nvasil...@apple.com
Date
2017-10-30 16:11:06 -0700 (Mon, 30 Oct 2017)

Log Message

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.

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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (224209 => 224210)


--- trunk/LayoutTests/ChangeLog	2017-10-30 23:08:29 UTC (rev 224209)
+++ trunk/LayoutTests/ChangeLog	2017-10-30 23:11:06 UTC (rev 224210)
@@ -1,3 +1,16 @@
+2017-10-30  Nikita Vasilyev  <nvasil...@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  <jlew...@apple.com>
 
         Followup rebaseline to r224204.

Added: trunk/LayoutTests/inspector/unit-tests/text-range-expected.txt (0 => 224210)


--- trunk/LayoutTests/inspector/unit-tests/text-range-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/inspector/unit-tests/text-range-expected.txt	2017-10-30 23:11:06 UTC (rev 224210)
@@ -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.
+

Added: trunk/LayoutTests/inspector/unit-tests/text-range.html (0 => 224210)


--- trunk/LayoutTests/inspector/unit-tests/text-range.html	                        (rev 0)
+++ trunk/LayoutTests/inspector/unit-tests/text-range.html	2017-10-30 23:11:06 UTC (rev 224210)
@@ -0,0 +1,76 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<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>

Modified: trunk/Source/WebInspectorUI/ChangeLog (224209 => 224210)


--- trunk/Source/WebInspectorUI/ChangeLog	2017-10-30 23:08:29 UTC (rev 224209)
+++ trunk/Source/WebInspectorUI/ChangeLog	2017-10-30 23:11:06 UTC (rev 224210)
@@ -1,3 +1,21 @@
+2017-10-30  Nikita Vasilyev  <nvasil...@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.kirsl...@sony.com>
 
         Web Inspector: Clicking filler in data grid should clear selection

Modified: trunk/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js (224209 => 224210)


--- trunk/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js	2017-10-30 23:08:29 UTC (rev 224209)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js	2017-10-30 23:11:06 UTC (rev 224210)
@@ -355,8 +355,10 @@
         // _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.");
 
         let newStyleText = styleText.slice(0, range.startOffset) + newText + styleText.slice(range.endOffset);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to