Attachment 371536 Details for Bug 198629 – Patch

[patch] Patch

patch.txt (text/plain), 12.67 KB, created by Nikita Vasilyev on 2019-06-06 16:38:12 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Nikita Vasilyev

Created: 2019-06-06 16:38:12 PDT

Size: 12.67 KB

patch

obsolete

>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index d6e8c8af05a..eb8ffa009dc 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-06-06  Nikita Vasilyev  <nvasilyev@apple.com>
>+
>+        Web Inspector: longhand CSS properties overridden by shorthands miss strikethrough
>+        https://bugs.webkit.org/show_bug.cgi?id=198629
>+        <rdar://problem/51504160>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * inspector/css/overridden-property-expected.txt:
>+        * inspector/css/overridden-property.html:
>+
> 2019-05-23  Saam barati  <sbarati@apple.com>
> 
>         [WHLSL] Property resolver needs to recurse to handle the base when simplifying rvalues
>diff --git a/LayoutTests/inspector/css/overridden-property-expected.txt b/LayoutTests/inspector/css/overridden-property-expected.txt
>index 6c762cbe2b1..834308b78bd 100644
>--- a/LayoutTests/inspector/css/overridden-property-expected.txt
>+++ b/LayoutTests/inspector/css/overridden-property-expected.txt
>@@ -1,5 +1,3 @@
>-Test that CSSProperty.prototype.overridden is cleared when the overriding (effective) property is removed.
>-
> 
> == Running test suite: OverriddenProperty
> -- Running test case: OverriddenProperty.effectivePropertyRemoved
>@@ -11,3 +9,15 @@ REMOVED
> color: red;
> 
> 
>+-- Running test case: OverriddenByShorthand
>+border-top-color: red; overridden
>+border-color: green;
>+
>+-- Running test case: OverriddenByShorthandImportant
>+border-color: green !important;
>+border-top-color: red; overridden
>+
>+-- Running test case: NotOverriddenByLonghandImportant
>+border-color: green;
>+border-top-color: red !important;
>+
>diff --git a/LayoutTests/inspector/css/overridden-property.html b/LayoutTests/inspector/css/overridden-property.html
>index f19ea1dec3c..4d865ff86b2 100644
>--- a/LayoutTests/inspector/css/overridden-property.html
>+++ b/LayoutTests/inspector/css/overridden-property.html
>@@ -4,76 +4,146 @@
> <script src="../../http/tests/inspector/resources/inspector-test.js"></script>
> <script>
> function test() {
>-    let nodeStyles = null;
>-
>     let suite = InspectorTest.createAsyncSuite("OverriddenProperty");
> 
>     function logProperty(property) {
>+        if (property.implicit)
>+            return;
>+
>         let text = property.formattedText || "REMOVED";
>+
>         if (property.overridden)
>             text += " overridden";
>+
>         InspectorTest.log(text);
>     }
> 
>+    function getNodeStyles(selector, callback) {
>+        WI.domManager.requestDocument((documentNode) => {
>+            WI.domManager.querySelector(documentNode.id, selector, (contentNodeId) => {
>+                if (!contentNodeId) {
>+                    InspectorTest.fail("DOM node not found.");
>+                    InspectorTest.completeTest();
>+                    return;
>+                }
>+
>+                let domNode = WI.domManager.nodeForId(contentNodeId);
>+                let nodeStyles = WI.cssManager.stylesForNode(domNode);
>+
>+                if (nodeStyles.needsRefresh) {
>+                    nodeStyles.singleFireEventListener(WI.DOMNodeStyles.Event.Refreshed, (event) => {
>+                        callback(nodeStyles);
>+                    });
>+                } else
>+                    callback(nodeStyles);
>+            });
>+        });
>+    }
>+
>+    function logMatchingRule(selector, resolve) {
>+        getNodeStyles(selector, (nodeStyles) => {
>+            let matchedRule = null;
>+            for (let rule of nodeStyles.matchedRules) {
>+                if (rule.selectorText === selector) {
>+                    matchedRule = rule;
>+                    break;
>+                }
>+            }
>+
>+            if (!matchedRule) {
>+                InspectorTest.log(`Couldn't find ${selector}`);
>+                return;
>+            }
>+
>+            for (let property of matchedRule.style.enabledProperties)
>+                logProperty(property);
>+
>+            resolve();
>+        });
>+    }
>+
>     suite.addTestCase({
>         name: "OverriddenProperty.effectivePropertyRemoved",
>+        description: "Test that CSSProperty.prototype.overridden is cleared when the overriding (effective) property is removed.",
>         test(resolve, reject) {
>-            let inlineStyle = nodeStyles.inlineStyle;
>-            let styleRule = nodeStyles.matchedRules[0];
>-            let inlineStyleProperty = inlineStyle.enabledProperties[0];
>-            let styleRuleProperty = styleRule.style.enabledProperties[0];
>-
>-            function log() {
>-                logProperty(inlineStyleProperty);
>-                logProperty(styleRuleProperty);
>-                InspectorTest.log("");
>-            }
>+            getNodeStyles("#x", (nodeStyles) => {
>+                let inlineStyle = nodeStyles.inlineStyle;
>+                let styleRule = nodeStyles.matchedRules[0];
>+                let inlineStyleProperty = inlineStyle.enabledProperties[0];
>+                let styleRuleProperty = styleRule.style.enabledProperties[0];
> 
>-            log();
>+                function log() {
>+                    logProperty(inlineStyleProperty);
>+                    logProperty(styleRuleProperty);
>+                    InspectorTest.log("");
>+                }
> 
>-            inlineStyleProperty.remove();
>+                log();
> 
>-            styleRuleProperty.addEventListener(WI.CSSProperty.Event.OverriddenStatusChanged, (event) => {
>-                // FIXME: <https://webkit.org/b/195651> OverriddenStatusChanged may fire more than once.
>-                if (styleRuleProperty.overridden)
>-                    return;
>+                inlineStyleProperty.remove();
> 
>-                InspectorTest.log("OverriddenStatusChanged event fired.");
>-                log();
>-                resolve();
>+                styleRuleProperty.addEventListener(WI.CSSProperty.Event.OverriddenStatusChanged, (event) => {
>+                    // FIXME: <https://webkit.org/b/195651> OverriddenStatusChanged may fire more than once.
>+                    if (styleRuleProperty.overridden)
>+                        return;
>+
>+                    InspectorTest.log("OverriddenStatusChanged event fired.");
>+                    log();
>+                    resolve();
>+                });
>             });
>         }
>     });
> 
>-    WI.domManager.requestDocument((documentNode) => {
>-        WI.domManager.querySelector(documentNode.id, "div#x", (contentNodeId) => {
>-            if (!contentNodeId) {
>-                InspectorTest.fail("DOM node not found.");
>-                InspectorTest.completeTest();
>-                return;
>-            }
>+    suite.addTestCase({
>+        name: "OverriddenByShorthand",
>+        test(resolve, reject) {
>+            logMatchingRule(".shorthand", resolve);
>+        }
>+    });
> 
>-            let domNode = WI.domManager.nodeForId(contentNodeId);
>-            nodeStyles = WI.cssManager.stylesForNode(domNode);
>+    suite.addTestCase({
>+        name: "OverriddenByShorthandImportant",
>+        test(resolve, reject) {
>+            logMatchingRule(".shorthand-important", resolve);
>+        }
>+    });
> 
>-            if (nodeStyles.needsRefresh) {
>-                nodeStyles.singleFireEventListener(WI.DOMNodeStyles.Event.Refreshed, (event) => {
>-                    suite.runTestCasesAndFinish();
>-                });
>-            } else
>-                suite.runTestCasesAndFinish();
>-        });
>+    suite.addTestCase({
>+        name: "NotOverriddenByLonghandImportant",
>+        test(resolve, reject) {
>+            logMatchingRule(".longhand-important", resolve);
>+        }
>     });
>+
>+    suite.runTestCasesAndFinish();
> }
> </script>
> </head>
> <body onload="runTest()">
>-    <p>Test that CSSProperty.prototype.overridden is cleared when the overriding (effective) property is removed.</p>
>     <style>
>     #x {
>         color: red;
>     }
>+
>+    .shorthand {
>+        border-top-color: red;
>+        border-color: green;
>+    }
>+
>+    .shorthand-important {
>+        border-color: green !important;
>+        border-top-color: red;
>+    }
>+
>+    .longhand-important {
>+        border-color: green;
>+        border-top-color: red !important;
>+    }
>     </style>
>     <div id="x" style="color: green"></div>
>+    <div class="shorthand"></div>
>+    <div class="shorthand-important"></div>
>+    <div class="longhand-important"></div>
> </body>
> </html>
>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index 8205b91b694..10f28bc101b 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,26 @@
>+2019-06-06  Nikita Vasilyev  <nvasilyev@apple.com>
>+
>+        Web Inspector: longhand CSS properties overridden by shorthands miss strikethrough
>+        https://bugs.webkit.org/show_bug.cgi?id=198629
>+        <rdar://problem/51504160>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Longhand CSS properties (e.g. "font-size") overriden by shorthands (e.g. "font") now have strikethroughs.
>+
>+        * UserInterface/Models/CSSProperty.js:
>+        (WI.CSSProperty.prototype.set overridingProperty):
>+        (WI.CSSProperty):
>+
>+        * UserInterface/Models/DOMNodeStyles.js:
>+        (WI.DOMNodeStyles.prototype._updateStyleCascade):
>+        Call _associateRelatedProperties before _markOverriddenProperties because
>+        _associateRelatedProperties sets relatedShorthandProperty property, which
>+        is now used by _markOverriddenProperties.
>+
>+        (WI.DOMNodeStyles.prototype._markOverriddenProperties.isOverriddenBy):
>+        (WI.DOMNodeStyles.prototype._markOverriddenProperties):
>+
> 2019-05-23  Matt Baker  <mattbaker@apple.com>
> 
>         Web Inspector: Remove unused CSS class "offset-sections"
>diff --git a/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js b/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js
>index 2c10895cc78..59a02ecfa24 100644
>--- a/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js
>+++ b/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js
>@@ -299,6 +299,7 @@ WI.CSSProperty = class CSSProperty extends WI.Object
>         if (!WI.settings.experimentalEnableStylesJumpToEffective.value)
>             return;
> 
>+        console.assert(this !== effectiveProperty, `Property "${this.formattedText}" can't override itself.`);
>         this._overridingProperty = effectiveProperty || null;
>     }
> 
>diff --git a/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js b/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js
>index a57b5201e35..6ef1490356c 100644
>--- a/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js
>+++ b/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js
>@@ -795,13 +795,13 @@ WI.DOMNodeStyles = class DOMNodeStyles extends WI.Object
> 
>         this._propertyNameToEffectivePropertyMap = {};
> 
>-        this._markOverriddenProperties(cascadeOrderedStyleDeclarations, this._propertyNameToEffectivePropertyMap);
>         this._associateRelatedProperties(cascadeOrderedStyleDeclarations, this._propertyNameToEffectivePropertyMap);
>+        this._markOverriddenProperties(cascadeOrderedStyleDeclarations, this._propertyNameToEffectivePropertyMap);
> 
>         for (let pseudoElementInfo of this._pseudoElements.values()) {
>             pseudoElementInfo.orderedStyles = this._collectStylesInCascadeOrder(pseudoElementInfo.matchedRules, null, null);
>-            this._markOverriddenProperties(pseudoElementInfo.orderedStyles);
>             this._associateRelatedProperties(pseudoElementInfo.orderedStyles);
>+            this._markOverriddenProperties(pseudoElementInfo.orderedStyles);
>         }
>     }
> 
>@@ -847,6 +847,25 @@ WI.DOMNodeStyles = class DOMNodeStyles extends WI.Object
>     {
>         propertyNameToEffectiveProperty = propertyNameToEffectiveProperty || {};
> 
>+        function isOverriddenBy(a, b) {
>+            if (!b)
>+                return false;
>+
>+            if (a.important && !b.important)
>+                return false;
>+
>+            if (!a.important && b.important)
>+                return true;
>+
>+            if (a.ownerStyle === b.ownerStyle)
>+                return b.index > a.index;
>+            else {
>+                let styleIndexA = styles.indexOf(a.ownerStyle);
>+                let styleIndexB = styles.indexOf(b.ownerStyle);
>+                return styleIndexB > styleIndexA;
>+            }
>+        }
>+
>         for (var i = 0; i < styles.length; ++i) {
>             var style = styles[i];
>             var properties = style.enabledProperties;
>@@ -885,7 +904,11 @@ WI.DOMNodeStyles = class DOMNodeStyles extends WI.Object
>                     }
>                 }
> 
>-                property.overridden = false;
>+                if (isOverriddenBy(property, property.relatedShorthandProperty)) {
>+                    property.overridden = true;
>+                    property.overridingProperty = property.relatedShorthandProperty;
>+                } else
>+                    property.overridden = false;
> 
>                 propertyNameToEffectiveProperty[canonicalName] = property;
>             }

Actions: View | Formatted Diff | Diff

Attachments on bug 198629: 371536 | 371613 | 371618