Attachment 356890 Details for Bug 192396 – WIP: single agent call

[patch] WIP: single agent call

patch.txt (text/plain), 15.42 KB, created by Nikita Vasilyev on 2018-12-08 15:38:44 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Nikita Vasilyev

Created: 2018-12-08 15:38:44 PST

Size: 15.42 KB

patch

obsolete

>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 565ab81a36f..72e2291d289 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,16 @@
>+2018-12-06  Nikita Vasilyev  <nvasilyev@apple.com>
>+
>+        Web Inspector: Styles: toggling selected properties may cause data corruption
>+        https://bugs.webkit.org/show_bug.cgi?id=192396
>+        <rdar://problem/46478383>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Test commenting out and uncommenting CSS properties.
>+
>+        * inspector/css/modify-css-property-expected.txt:
>+        * inspector/css/modify-css-property.html:
>+
> 2018-12-06  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         [iOS] WKWebView should match UITextView behavior when editing text with an RTL keyboard
>diff --git a/LayoutTests/inspector/css/modify-css-property-expected.txt b/LayoutTests/inspector/css/modify-css-property-expected.txt
>index c8aa3d9cbfc..8dc2debd86b 100644
>--- a/LayoutTests/inspector/css/modify-css-property-expected.txt
>+++ b/LayoutTests/inspector/css/modify-css-property-expected.txt
>@@ -16,3 +16,25 @@ PASS: Style declaration text should stay "width: 64px".
> PASS: "width" property value should update to "200px".
> PASS: Inline style declaration text should update when not locked.
> 
>+-- Running test case: ModifyCSSProperty.CommentOutAndUncommentPropertyLocked
>+PASS: Commented out property should be disabled.
>+PASS: Style declaration text should update immediately with uncommented property.
>+PASS: Uncommented property should be enabled.
>+PASS: Style declaration text should update immediately with commented out property.
>+PASS: Commented out property should be disabled.
>+
>+-- Running test case: ModifyCSSProperty.CommentOutAndUncommentPropertyLocked2
>+PASS: Commented out property should be disabled.
>+PASS: Style declaration text should update immediately with uncommented property.
>+PASS: Uncommented property should be enabled.
>+PASS: Commented out property should be disabled.
>+PASS: Style declaration text should update immediately with commented out property.
>+PASS: Uncommented property should be enabled.
>+
>+-- Running test case: ModifyCSSProperty.CommentOutAndUncommentPropertyUnlocked
>+PASS: Commented out property should be disabled.
>+PASS: Style declaration text should stay unchanged.
>+PASS: Uncommented property should be enabled.
>+PASS: Style declaration text should stay unchanged.
>+PASS: Commented out property should be disabled.
>+
>diff --git a/LayoutTests/inspector/css/modify-css-property.html b/LayoutTests/inspector/css/modify-css-property.html
>index c35b9f03157..ca01f761862 100644
>--- a/LayoutTests/inspector/css/modify-css-property.html
>+++ b/LayoutTests/inspector/css/modify-css-property.html
>@@ -127,6 +127,157 @@ function test() {
>         }
>     });
> 
>+    suite.addTestCase({
>+        name: "ModifyCSSProperty.CommentOutAndUncommentPropertyLocked",
>+        test(resolve, reject) {
>+            let getMatchedStyleDeclaration = () => {
>+                for (let rule of nodeStyles.matchedRules) {
>+                    if (rule.selectorText === ".rule-c")
>+                        return rule.style;
>+                }
>+            };
>+
>+            let getProperty = (propertyName) => {
>+                let styleDeclaration = getMatchedStyleDeclaration();
>+                for (let property of styleDeclaration.allProperties) {
>+                    if (property.name === propertyName)
>+                        return property;
>+                }
>+            };
>+
>+            let styleDeclaration = getMatchedStyleDeclaration();
>+            styleDeclaration.locked = true;
>+
>+            InspectorTest.expectThat(!getProperty("padding-right").enabled, `Commented out property should be disabled.`);
>+
>+            let disabled = false;
>+            getProperty("padding-right").commentOut(disabled);
>+
>+            let expectedStyleText = `
>+        /* padding-left: 2em; */
>+        padding-right: 0px;
>+    `;
>+            InspectorTest.expectEqual(styleDeclaration.text, expectedStyleText, `Style declaration text should update immediately with uncommented property.`);
>+
>+            InspectorTest.expectThat(getProperty("padding-right").enabled, `Uncommented property should be enabled.`);
>+
>+            disabled = true;
>+            getProperty("padding-right").commentOut(disabled);
>+
>+            expectedStyleText = `
>+        /* padding-left: 2em; */
>+        /* padding-right: 0px; */
>+    `;
>+            InspectorTest.expectEqual(styleDeclaration.text, expectedStyleText, `Style declaration text should update immediately with commented out property.`);
>+
>+            InspectorTest.expectThat(!getProperty("padding-right").enabled, `Commented out property should be disabled.`);
>+
>+            resolve();
>+        }
>+    });
>+
>+    suite.addTestCase({
>+        name: "ModifyCSSProperty.CommentOutAndUncommentPropertyLocked2",
>+        test(resolve, reject) {
>+            let getMatchedStyleDeclaration = () => {
>+                for (let rule of nodeStyles.matchedRules) {
>+                    if (rule.selectorText === ".rule-d")
>+                        return rule.style;
>+                }
>+            };
>+
>+            let getProperty = (propertyName) => {
>+                let styleDeclaration = getMatchedStyleDeclaration();
>+                for (let property of styleDeclaration.allProperties) {
>+                    if (property.name === propertyName)
>+                        return property;
>+                }
>+            };
>+
>+            let styleDeclaration = getMatchedStyleDeclaration();
>+            styleDeclaration.locked = true;
>+
>+            InspectorTest.expectThat(!getProperty("font-size").enabled, `Commented out property should be disabled.`);
>+
>+            let disabled = false;
>+            getProperty("font-size").commentOut(disabled);
>+
>+            let expectedStyleText = `font-size: 13px;/*border: 2px solid brown*/`;
>+            InspectorTest.expectEqual(styleDeclaration.text, expectedStyleText, `Style declaration text should update immediately with uncommented property.`);
>+
>+            InspectorTest.expectThat(getProperty("font-size").enabled, `Uncommented property should be enabled.`);
>+            InspectorTest.expectThat(!getProperty("border").enabled, `Commented out property should be disabled.`);
>+
>+            disabled = false;
>+            getProperty("border").commentOut(disabled);
>+
>+            expectedStyleText = `font-size: 13px;border: 2px solid brown`;
>+            InspectorTest.expectEqual(styleDeclaration.text, expectedStyleText, `Style declaration text should update immediately with commented out property.`);
>+
>+            InspectorTest.expectThat(getProperty("border").enabled, `Uncommented property should be enabled.`);
>+
>+            resolve();
>+        }
>+    });
>+
>+    suite.addTestCase({
>+        name: "ModifyCSSProperty.CommentOutAndUncommentPropertyUnlocked",
>+        test(resolve, reject) {
>+            let getMatchedStyleDeclaration = () => {
>+                for (let rule of nodeStyles.matchedRules) {
>+                    if (rule.selectorText === ".rule-c")
>+                        return rule.style;
>+                }
>+            };
>+
>+            let getProperty = (propertyName) => {
>+                let styleDeclaration = getMatchedStyleDeclaration();
>+                for (let property of styleDeclaration.allProperties) {
>+                    if (property.name === propertyName)
>+                        return property;
>+                }
>+
>+
>+]
>+                    InspectorTest.fail("No style declaration found.");
>+                    resolve();
>+                    return;
>+                }
>+            };
>+
>+            let styleDeclaration = getMatchedStyleDeclaration();
>+            styleDeclaration.locked = false;
>+
>+            if (!styleDeclaration) {
>+                InspectorTest.fail("No style declaration found.");
>+                resolve();
>+                return;
>+            }
>+
>+            InspectorTest.expectThat(!getProperty("padding-right").enabled, `Commented out property should be disabled.`);
>+
>+            let disabled = false;
>+            getProperty("padding-right").commentOut(disabled);
>+
>+            const expectedStyleText = `
>+        /* padding-left: 2em; */
>+        /* padding-right: 0px; */
>+    `;
>+            InspectorTest.expectEqual(styleDeclaration.text, expectedStyleText, `Style declaration text should stay unchanged.`);
>+
>+            InspectorTest.expectThat(getProperty("padding-right").enabled, `Uncommented property should be enabled.`);
>+
>+            disabled = true;
>+            getProperty("padding-right").commentOut(disabled);
>+
>+            InspectorTest.expectEqual(styleDeclaration.text, expectedStyleText, `Style declaration text should stay unchanged.`);
>+
>+            InspectorTest.expectThat(!getProperty("padding-right").enabled, `Commented out property should be disabled.`);
>+
>+            resolve();
>+        }
>+    });
>+
>     WI.domManager.requestDocument((documentNode) => {
>         WI.domManager.querySelector(documentNode.id, "#x", (contentNodeId) => {
>             if (contentNodeId) {
>@@ -160,7 +311,12 @@ function test() {
>         margin-top: 1em;
>     }
>     .rule-b {font-size: 12px; color: antiquewhite}
>+    .rule-c {
>+        /* padding-left: 2em; */
>+        /* padding-right: 0px; */
>+    }
>+    .rule-d {/*font-size: 13px;*//*border: 2px solid brown*/}
>     </style>
>-    <div id="x" class="test-node rule-a rule-b" style="width: 100px"></div>
>+    <div id="x" class="test-node rule-a rule-b rule-c rule-d" style="width: 100px"></div>
> </body>
> </html>
>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index 698900f5b78..d5c14fc4359 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,34 @@
>+2018-12-06  Nikita Vasilyev  <nvasilyev@apple.com>
>+
>+        Web Inspector: Styles: toggling selected properties may cause data corruption
>+        https://bugs.webkit.org/show_bug.cgi?id=192396
>+        <rdar://problem/46478383>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Uncommenting a property after a commented out property used to insert an unnecessary semicolon.
>+
>+        For example:
>+
>+            /* color: red; */
>+            /* font-size: 12px */
>+
>+        Uncommenting `font-size` would result in something like this:
>+
>+            /* color: red; */; font-size: 12px
>+                             ^
>+                             unnecessary semicolon
>+
>+        Now the semicolon doesn't get inserted:
>+
>+            /* color: red; */
>+            font-size: 12px
>+
>+        * UserInterface/Models/CSSProperty.js:
>+        (WI.CSSProperty.prototype._updateOwnerStyleText):
>+        (WI.CSSProperty.prototype._appendSemicolonIfNeeded): Removed.
>+        (WI.CSSProperty.prototype._prependSemicolonIfNeeded): Added.
>+
> 2018-12-06  Matt Baker  <mattbaker@apple.com>
> 
>         Web Inspector: REGRESSION(r238602): Elements: collapsing a DOM node with the left arrow doesn't work
>diff --git a/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js b/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js
>index d6e4ad71535..f7dda2220cb 100644
>--- a/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js
>+++ b/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js
>@@ -270,7 +270,16 @@ WI.CSSProperty = class CSSProperty extends WI.Object
>     get inherited() { return this._inherited; }
>     get valid() { return this._valid; }
>     get variable() { return this._variable; }
>-    get styleSheetTextRange() { return this._styleSheetTextRange; }
>+
>+    get styleSheetTextRange()
>+    {
>+        return this._styleSheetTextRange;
>+    }
>+
>+    set styleSheetTextRange(value)
>+    {
>+        this._styleSheetTextRange = value;
>+    }
> 
>     get editable()
>     {
>@@ -313,6 +322,11 @@ WI.CSSProperty = class CSSProperty extends WI.Object
> 
>     get relatedLonghandProperties() { return this._relatedLonghandProperties; }
> 
>+    getTextRangeWithinOwnerStyle()
>+    {
>+        return this._styleSheetTextRange.relativeTo(this._ownerStyle.styleSheetTextRange.startLine, this._ownerStyle.styleSheetTextRange.startColumn);
>+    }
>+
>     addRelatedLonghandProperty(property)
>     {
>         this._relatedLonghandProperties.push(property);
>@@ -360,9 +374,11 @@ WI.CSSProperty = class CSSProperty extends WI.Object
> 
>         let styleText = this._ownerStyle.text || "";
> 
>+        styleText = this._prependSemicolonIfNeeded(styleText);
>+
>         // _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);
>+        let range = this.getTextRangeWithinOwnerStyle();
> 
>         // Append a line break to count the last line of styleText towards endOffset.
>         range.resolveOffsets(styleText + "\n");
>@@ -375,7 +391,7 @@ WI.CSSProperty = class CSSProperty extends WI.Object
>             console.info(`${prefix}%c${oldText}%c${newText}%c${postfix}`, `background: hsl(356, 100%, 90%); color: black`, `background: hsl(100, 100%, 91%); color: black`, `background: transparent`);
>         }
> 
>-        let newStyleText = this._appendSemicolonIfNeeded(styleText.slice(0, range.startOffset)) + newText + styleText.slice(range.endOffset);
>+        let newStyleText = styleText.slice(0, range.startOffset) + newText + styleText.slice(range.endOffset);
> 
>         let lineDelta = newText.lineCount - oldText.lineCount;
>         let columnDelta = newText.lastLine.length - oldText.lastLine.length;
>@@ -387,10 +403,43 @@ WI.CSSProperty = class CSSProperty extends WI.Object
>         this._ownerStyle.shiftPropertiesAfter(this, lineDelta, columnDelta, propertyWasRemoved);
>     }
> 
>-    _appendSemicolonIfNeeded(styleText)
>+    _prependSemicolonIfNeeded(styleText)
>     {
>-        if (/[^;\s]\s*$/.test(styleText))
>-            return styleText.trimRight() + "; ";
>+        for (let i = this.index - 1; i >= 0; --i) {
>+            let property = this._ownerStyle.allProperties[i];
>+            if (!property.enabled)
>+                continue;
>+
>+            let match = property.text.match(/[^;\s](\s*)$/);
>+            if (match) {
>+                let range = property.getTextRangeWithinOwnerStyle();
>+
>+                // Append a line break to count the last line of styleText towards endOffset.
>+                range.resolveOffsets(styleText + "\n");
>+
>+                let oldText = styleText.slice(range.startOffset, range.endOffset);
>+                let newText = oldText.trimRight() + ";" + match[1];
>+
>+                let lineDelta = newText.lineCount - oldText.lineCount;
>+                let columnDelta = newText.lastLine.length - oldText.lastLine.length;
>+                property.styleSheetTextRange = property.styleSheetTextRange.cloneAndModify(0, 0, lineDelta, columnDelta);
>+
>+                let propertyWasRemoved = false;
>+
>+                // Text ranges are temporary inaccurate until property._ownerStyle.text is updated.
>+                property._ownerStyle.shiftPropertiesAfter(property, lineDelta, columnDelta, propertyWasRemoved);
>+
>+                let prefix = styleText.slice(0, range.startOffset);
>+                let postfix = styleText.slice(range.endOffset);
>+
>+                if (WI.settings.enableStyleEditingDebugMode.value)
>+                    console.info(`${prefix}%c${oldText}%c${newText}%c${postfix}`, `background: hsl(356, 100%, 90%); color: black`, `background: hsl(100, 100%, 91%); color: black`, `background: transparent`);
>+
>+                styleText = prefix + newText + postfix;
>+            }
>+
>+            break;
>+        }
> 
>         return styleText;
>     }

Flags:

nvasilyev: review-
nvasilyev: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 192396: 356761 | 356785 | 356790 | 356850 | 356890 | 356891 | 357397