Attachment 362101 Details for Bug 194619 – Patch

[patch] Patch

patch.txt (text/plain), 7.93 KB, created by Nikita Vasilyev on 2019-02-14 23:25:08 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Nikita Vasilyev

Created: 2019-02-14 23:25:08 PST

Size: 7.93 KB

patch

obsolete

>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 1af7989c41f..c7986ea0fc9 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-02-14  Nikita Vasilyev  <nvasilyev@apple.com>
>+
>+        Web Inspector: Styles: valid values in style attributes are reported as unsupported property values
>+        https://bugs.webkit.org/show_bug.cgi?id=194619
>+        <rdar://problem/47917373>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * inspector/css/modify-inline-style-expected.txt: Added.
>+        * inspector/css/modify-inline-style.html: Added.
>+
> 2019-02-14  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r241497.
>diff --git a/LayoutTests/inspector/css/modify-inline-style-expected.txt b/LayoutTests/inspector/css/modify-inline-style-expected.txt
>new file mode 100644
>index 00000000000..397ca66305f
>--- /dev/null
>+++ b/LayoutTests/inspector/css/modify-inline-style-expected.txt
>@@ -0,0 +1,18 @@
>+Testing that adding and editing CSS properties of inline styles works.
>+
>+
>+== Running test suite: ModifyInlineStyle
>+-- Running test case: ModifyInlineStyle.AddPropertyAndEdit
>+CSSProperty changed to "font: 12px normal sans-serif!important;".
>+font: 12px normal sans-serif!important;
>+
>+CSSProperty changed to "color: red;".
>+font: 12px normal sans-serif!important;color: red;
>+
>+CSSProperty changed to "font: 12px sans-serif;".
>+font: 12px sans-serif;color: red;
>+
>+CSSProperty changed to "color: invalid_c010r;".
>+font: 12px sans-serif;color: invalid_c010r;
>+
>+
>diff --git a/LayoutTests/inspector/css/modify-inline-style.html b/LayoutTests/inspector/css/modify-inline-style.html
>new file mode 100644
>index 00000000000..bb8214435a3
>--- /dev/null
>+++ b/LayoutTests/inspector/css/modify-inline-style.html
>@@ -0,0 +1,99 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<script src="../../http/tests/inspector/resources/inspector-test.js"></script>
>+<script>
>+function test() {
>+    let nodeStyles = null;
>+
>+    let suite = InspectorTest.createAsyncSuite("ModifyInlineStyle");
>+
>+    suite.addTestCase({
>+        name: "ModifyInlineStyle.AddPropertyAndEdit",
>+        test(resolve, reject) {
>+            let getInlineStyleDeclaration = () => {
>+                for (let styleDeclaration of nodeStyles.orderedStyles) {
>+                    if (styleDeclaration.type === styleDeclaration.constructor.Type.Inline)
>+                        return styleDeclaration;
>+                }
>+                InspectorTest.fail("No declaration found.");
>+                resolve();
>+            };
>+
>+            let style = getInlineStyleDeclaration();
>+
>+            const fontPropertyIndex = 0;
>+            let fontProperty = style.newBlankProperty(fontPropertyIndex);
>+            fontProperty.name = "font";
>+            fontProperty.rawValue = "12px normal sans-serif!important";
>+
>+            let colorProperty = null;
>+
>+            let cssPropertyChanged = (event) => {
>+                if (event.target.ownerStyle && event.target.ownerStyle.type === WI.CSSStyleDeclaration.Type.Computed)
>+                    return;
>+
>+                if (!event.target.text)
>+                    return;
>+
>+                InspectorTest.log(`CSSProperty changed to "${event.target.text}".`);
>+                InspectorTest.log(style.text + "\n");
>+            };
>+
>+            WI.CSSProperty.addEventListener(WI.CSSProperty.Event.Changed, cssPropertyChanged);
>+
>+            fontProperty.awaitEvent(WI.CSSProperty.Event.Changed).then((event) => {
>+                const colorPropertyIndex = 1;
>+                colorProperty = style.newBlankProperty(colorPropertyIndex);
>+                let promise = colorProperty.awaitEvent(WI.CSSProperty.Event.Changed);
>+                colorProperty.name = "color";
>+                colorProperty.rawValue = "red";
>+                return promise;
>+            }).then(() => {
>+                let promise = fontProperty.awaitEvent(WI.CSSProperty.Event.Changed);
>+                fontProperty.rawValue = "12px sans-serif";
>+                return promise;
>+            }).then(() => {
>+                let promise = colorProperty.awaitEvent(WI.CSSProperty.Event.Changed);
>+                colorProperty.rawValue = "invalid_c010r";
>+                return promise;
>+            }).then(() => {
>+                WI.CSSProperty.removeEventListener(WI.CSSProperty.Event.Changed, cssPropertyChanged);
>+                resolve();
>+                return true;
>+            });
>+        }
>+    });
>+
>+    WI.domManager.requestDocument((documentNode) => {
>+        WI.domManager.querySelector(documentNode.id, "#x", (contentNodeId) => {
>+            if (contentNodeId) {
>+                let domNode = WI.domManager.nodeForId(contentNodeId);
>+                nodeStyles = WI.cssManager.stylesForNode(domNode);
>+
>+                if (nodeStyles.needsRefresh) {
>+                    nodeStyles.singleFireEventListener(WI.DOMNodeStyles.Event.Refreshed, (event) => {
>+                        suite.runTestCasesAndFinish()
>+                    });
>+                } else
>+                    suite.runTestCasesAndFinish();
>+
>+                nodeStyles.addEventListener(WI.DOMNodeStyles.Event.NeedsRefresh, function(event) {
>+                    // Normally, this would get called from views if the styles sidebar is visible.
>+                    // This doesn't work in tests.
>+                    event.target.refresh();
>+                });
>+            } else {
>+                InspectorTest.fail("DOM node not found.");
>+                InspectorTest.completeTest();
>+            }
>+        });
>+    });
>+}
>+</script>
>+</head>
>+<body onload="runTest()">
>+    <p>Testing that adding and editing CSS properties of inline styles works.</p>
>+    <div id="x"></div>
>+</body>
>+</html>
>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index b83b1ec2c2a..8695fc5801f 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,18 @@
>+2019-02-14  Nikita Vasilyev  <nvasilyev@apple.com>
>+
>+        Web Inspector: Styles: valid values in style attributes are reported as unsupported property values
>+        https://bugs.webkit.org/show_bug.cgi?id=194619
>+        <rdar://problem/47917373>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Payload of inline styles may contain `range` that doesn't match
>+        the actual text of the payload - it has an extra empty line at the end.
>+        Mismatching ranges caused data corruption.
>+
>+        * UserInterface/Models/DOMNodeStyles.js:
>+        (WI.DOMNodeStyles.prototype._parseStylePropertyPayload):
>+
> 2019-02-14  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r241497.
>diff --git a/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js b/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js
>index c7845078fbc..a199be0310b 100644
>--- a/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js
>+++ b/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js
>@@ -512,6 +512,7 @@ WI.DOMNodeStyles = class DOMNodeStyles extends WI.Object
>         var name = payload.name;
>         var value = payload.value || "";
>         var priority = payload.priority || "";
>+        let range = payload.range || null;
> 
>         var enabled = true;
>         var overridden = false;
>@@ -536,6 +537,19 @@ WI.DOMNodeStyles = class DOMNodeStyles extends WI.Object
>             break;
>         }
> 
>+        if (range) {
>+            // Last property of inline style has mismatching range.
>+            // The actual text has one line, but the range spans two lines.
>+            let rangeLineCount = 1 + range.endLine - range.startLine;
>+            if (rangeLineCount > 1) {
>+                let textLineCount = text.lineCount;
>+                if (textLineCount === rangeLineCount - 1) {
>+                    range.endLine = range.startLine + (textLineCount - 1);
>+                    range.endColumn = range.startColumn + text.lastLine.length;
>+                }
>+            }
>+        }
>+
>         var styleSheetTextRange = this._parseSourceRangePayload(payload.range);
> 
>         if (styleDeclaration) {

Flags:

joepeck: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 194619: 361965 | 361971 | 361972 | 361984 | 362101 | 362157