Changeset 246223 in webkit


Ignore:
Timestamp:
Jun 7, 2019 4:18:39 PM (5 years ago)
Author:
Nikita Vasilyev
Message:

Web Inspector: longhand CSS properties overridden by shorthands miss strikethrough
https://bugs.webkit.org/show_bug.cgi?id=198629
<rdar://problem/51504160>

Reviewed by Devin Rousso.

Source/WebInspectorUI:

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):

LayoutTests:

  • inspector/css/overridden-property-expected.txt:
  • inspector/css/overridden-property.html:
Location:
trunk
Files:
6 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r246217 r246223  
     12019-06-07  Nikita Vasilyev  <nvasilyev@apple.com>
     2
     3        Web Inspector: longhand CSS properties overridden by shorthands miss strikethrough
     4        https://bugs.webkit.org/show_bug.cgi?id=198629
     5        <rdar://problem/51504160>
     6
     7        Reviewed by Devin Rousso.
     8
     9        * inspector/css/overridden-property-expected.txt:
     10        * inspector/css/overridden-property.html:
     11
    1122019-06-07  Justin Fan  <justin_fan@apple.com>
    213
  • trunk/LayoutTests/inspector/css/overridden-property-expected.txt

    r242914 r246223  
    1 Test that CSSProperty.prototype.overridden is cleared when the overriding (effective) property is removed.
     1Test that CSSProperty.prototype.overridden is set correctly.
    22
    33
     
    1212
    1313
     14-- Running test case: OverriddenProperty.OverriddenByShorthand
     15PASS: border-top-color is overridden.
     16PASS: border-color is NOT overridden.
     17
     18-- Running test case: OverriddenProperty.OverriddenByImportantShorthand
     19PASS: border-color is NOT overridden.
     20PASS: border-top-color is overridden.
     21
     22-- Running test case: OverriddenProperty.NotOverriddenByImportantLonghand
     23PASS: border-top-color is NOT overridden.
     24PASS: border-color is NOT overridden.
     25
     26-- Running test case: OverriddenProperty.NotOverriddenByLonghand
     27PASS: border-color is NOT overridden.
     28PASS: border-top-color is NOT overridden.
     29
  • trunk/LayoutTests/inspector/css/overridden-property.html

    r242914 r246223  
    55<script>
    66function test() {
    7     let nodeStyles = null;
    8 
    97    let suite = InspectorTest.createAsyncSuite("OverriddenProperty");
    108
     
    1614    }
    1715
     16    function getNodeStyles(selector, callback) {
     17        WI.domManager.requestDocument((documentNode) => {
     18            WI.domManager.querySelector(documentNode.id, selector, (contentNodeId) => {
     19                if (!contentNodeId) {
     20                    InspectorTest.fail("DOM node not found.");
     21                    InspectorTest.completeTest();
     22                    return;
     23                }
     24
     25                let domNode = WI.domManager.nodeForId(contentNodeId);
     26                let nodeStyles = WI.cssManager.stylesForNode(domNode);
     27
     28                if (nodeStyles.needsRefresh) {
     29                    nodeStyles.singleFireEventListener(WI.DOMNodeStyles.Event.Refreshed, (event) => {
     30                        callback(nodeStyles);
     31                    });
     32                } else
     33                    callback(nodeStyles);
     34            });
     35        });
     36    }
     37
     38    function getStyleDeclaration(selector, callback, resolve) {
     39        getNodeStyles(selector, (nodeStyles) => {
     40            let matchedRule = null;
     41            for (let rule of nodeStyles.matchedRules) {
     42                if (rule.selectorText === selector) {
     43                    matchedRule = rule;
     44                    break;
     45                }
     46            }
     47
     48            if (!matchedRule) {
     49                InspectorTest.fail(`Couldn't find ${selector}`);
     50                resolve();
     51                return;
     52            }
     53
     54            callback(matchedRule.style)
     55        });
     56    }
     57
    1858    suite.addTestCase({
    1959        name: "OverriddenProperty.effectivePropertyRemoved",
     60        description: "Test that CSSProperty.prototype.overridden is cleared when the overriding (effective) property is removed.",
    2061        test(resolve, reject) {
    21             let inlineStyle = nodeStyles.inlineStyle;
    22             let styleRule = nodeStyles.matchedRules[0];
    23             let inlineStyleProperty = inlineStyle.enabledProperties[0];
    24             let styleRuleProperty = styleRule.style.enabledProperties[0];
     62            getNodeStyles("#x", (nodeStyles) => {
     63                let inlineStyle = nodeStyles.inlineStyle;
     64                let styleRule = nodeStyles.matchedRules[0];
     65                let inlineStyleProperty = inlineStyle.enabledProperties[0];
     66                let styleRuleProperty = styleRule.style.enabledProperties[0];
    2567
    26             function log() {
    27                 logProperty(inlineStyleProperty);
    28                 logProperty(styleRuleProperty);
    29                 InspectorTest.log("");
    30             }
     68                function log() {
     69                    logProperty(inlineStyleProperty);
     70                    logProperty(styleRuleProperty);
     71                    InspectorTest.log("");
     72                }
    3173
    32             log();
     74                log();
    3375
    34             inlineStyleProperty.remove();
     76                inlineStyleProperty.remove();
    3577
    36             styleRuleProperty.addEventListener(WI.CSSProperty.Event.OverriddenStatusChanged, (event) => {
    37                 // FIXME: <https://webkit.org/b/195651> OverriddenStatusChanged may fire more than once.
    38                 if (styleRuleProperty.overridden)
    39                     return;
     78                styleRuleProperty.addEventListener(WI.CSSProperty.Event.OverriddenStatusChanged, (event) => {
     79                    // FIXME: <https://webkit.org/b/195651> OverriddenStatusChanged may fire more than once.
     80                    if (styleRuleProperty.overridden)
     81                        return;
    4082
    41                 InspectorTest.log("OverriddenStatusChanged event fired.");
    42                 log();
    43                 resolve();
     83                    InspectorTest.log("OverriddenStatusChanged event fired.");
     84                    log();
     85                    resolve();
     86                });
    4487            });
    4588        }
    4689    });
    4790
    48     WI.domManager.requestDocument((documentNode) => {
    49         WI.domManager.querySelector(documentNode.id, "div#x", (contentNodeId) => {
    50             if (!contentNodeId) {
    51                 InspectorTest.fail("DOM node not found.");
    52                 InspectorTest.completeTest();
    53                 return;
    54             }
     91    suite.addTestCase({
     92        name: "OverriddenProperty.OverriddenByShorthand",
     93        test(resolve, reject) {
     94            getStyleDeclaration(".longhand-overridden-by-shorthand", (style) => {
     95                const dontCreateIfMissing = true;
     96                let borderTopColorProperty = style.propertyForName("border-top-color", dontCreateIfMissing);
     97                InspectorTest.expectTrue(borderTopColorProperty.overridden, "border-top-color is overridden.");
    5598
    56             let domNode = WI.domManager.nodeForId(contentNodeId);
    57             nodeStyles = WI.cssManager.stylesForNode(domNode);
     99                let borderColorProperty = style.propertyForName("border-color", dontCreateIfMissing);
     100                InspectorTest.expectFalse(borderColorProperty.overridden, "border-color is NOT overridden.");
    58101
    59             if (nodeStyles.needsRefresh) {
    60                 nodeStyles.singleFireEventListener(WI.DOMNodeStyles.Event.Refreshed, (event) => {
    61                     suite.runTestCasesAndFinish();
    62                 });
    63             } else
    64                 suite.runTestCasesAndFinish();
    65         });
     102                resolve();
     103            }, reject);
     104        }
    66105    });
     106
     107    suite.addTestCase({
     108        name: "OverriddenProperty.OverriddenByImportantShorthand",
     109        test(resolve, reject) {
     110            getStyleDeclaration(".longhand-overridden-by-important-shorthand", (style) => {
     111                const dontCreateIfMissing = true;
     112                let borderColorProperty = style.propertyForName("border-color", dontCreateIfMissing);
     113                InspectorTest.expectFalse(borderColorProperty.overridden, "border-color is NOT overridden.");
     114
     115                let borderTopColorProperty = style.propertyForName("border-top-color", dontCreateIfMissing);
     116                InspectorTest.expectTrue(borderTopColorProperty.overridden, "border-top-color is overridden.");
     117
     118                resolve();
     119            }, reject);
     120        }
     121    });
     122
     123    suite.addTestCase({
     124        name: "OverriddenProperty.NotOverriddenByImportantLonghand",
     125        test(resolve, reject) {
     126            getStyleDeclaration(".shorthand-overridden-by-important-longhand", (style) => {
     127                const dontCreateIfMissing = true;
     128                let borderTopColorProperty = style.propertyForName("border-top-color", dontCreateIfMissing);
     129                InspectorTest.expectFalse(borderTopColorProperty.overridden, "border-top-color is NOT overridden.");
     130
     131                let borderColorProperty = style.propertyForName("border-color", dontCreateIfMissing);
     132                InspectorTest.expectFalse(borderColorProperty.overridden, "border-color is NOT overridden.");
     133
     134                resolve();
     135            }, reject);
     136        }
     137    });
     138
     139    suite.addTestCase({
     140        name: "OverriddenProperty.NotOverriddenByLonghand",
     141        test(resolve, reject) {
     142            getStyleDeclaration(".shorthand-not-overridden-by-longhand", (style) => {
     143                const dontCreateIfMissing = true;
     144                let borderColorProperty = style.propertyForName("border-color", dontCreateIfMissing);
     145                InspectorTest.expectFalse(borderColorProperty.overridden, "border-color is NOT overridden.");
     146
     147                let borderTopColorProperty = style.propertyForName("border-top-color", dontCreateIfMissing);
     148                InspectorTest.expectFalse(borderTopColorProperty.overridden, "border-top-color is NOT overridden.");
     149
     150                resolve();
     151            }, reject);
     152        }
     153    });
     154
     155    suite.runTestCasesAndFinish();
    67156}
    68157</script>
    69158</head>
    70159<body onload="runTest()">
    71     <p>Test that CSSProperty.prototype.overridden is cleared when the overriding (effective) property is removed.</p>
     160    <p>Test that CSSProperty.prototype.overridden is set correctly.</p>
    72161    <style>
    73162    #x {
    74163        color: red;
    75164    }
     165
     166    .longhand-overridden-by-shorthand {
     167        border-top-color: red;
     168        border-color: green;
     169    }
     170
     171    .longhand-overridden-by-important-shorthand {
     172        border-color: green !important;
     173        border-top-color: red;
     174    }
     175
     176    .shorthand-overridden-by-important-longhand {
     177        border-top-color: red !important;
     178        border-color: green;
     179    }
     180
     181    .shorthand-not-overridden-by-longhand {
     182        border-color: green;
     183        border-top-color: red;
     184    }
    76185    </style>
    77186    <div id="x" style="color: green"></div>
     187    <div class="longhand-overridden-by-shorthand"></div>
     188    <div class="longhand-overridden-by-important-shorthand"></div>
     189    <div class="shorthand-overridden-by-important-longhand"></div>
     190    <div class="shorthand-not-overridden-by-longhand"></div>
    78191</body>
    79192</html>
  • trunk/Source/WebInspectorUI/ChangeLog

    r246180 r246223  
     12019-06-07  Nikita Vasilyev  <nvasilyev@apple.com>
     2
     3        Web Inspector: longhand CSS properties overridden by shorthands miss strikethrough
     4        https://bugs.webkit.org/show_bug.cgi?id=198629
     5        <rdar://problem/51504160>
     6
     7        Reviewed by Devin Rousso.
     8
     9        Longhand CSS properties (e.g. "font-size") overriden by shorthands (e.g. "font") now have strikethroughs.
     10
     11        * UserInterface/Models/CSSProperty.js:
     12        (WI.CSSProperty.prototype.set overridingProperty):
     13        (WI.CSSProperty):
     14
     15        * UserInterface/Models/DOMNodeStyles.js:
     16        (WI.DOMNodeStyles.prototype._updateStyleCascade):
     17        Call _associateRelatedProperties before _markOverriddenProperties because
     18        _associateRelatedProperties sets relatedShorthandProperty property, which
     19        is now used by _markOverriddenProperties.
     20
     21        (WI.DOMNodeStyles.prototype._markOverriddenProperties.isOverriddenBy):
     22        (WI.DOMNodeStyles.prototype._markOverriddenProperties):
     23
    1242019-06-06  Devin Rousso  <drousso@apple.com>
    225
  • trunk/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js

    r245991 r246223  
    310310            return;
    311311
     312        console.assert(this !== effectiveProperty, `Property "${this.formattedText}" can't override itself.`, this);
    312313        this._overridingProperty = effectiveProperty || null;
    313314    }
  • trunk/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js

    r243264 r246223  
    796796        this._propertyNameToEffectivePropertyMap = {};
    797797
     798        this._associateRelatedProperties(cascadeOrderedStyleDeclarations, this._propertyNameToEffectivePropertyMap);
    798799        this._markOverriddenProperties(cascadeOrderedStyleDeclarations, this._propertyNameToEffectivePropertyMap);
    799         this._associateRelatedProperties(cascadeOrderedStyleDeclarations, this._propertyNameToEffectivePropertyMap);
    800800
    801801        for (let pseudoElementInfo of this._pseudoElements.values()) {
    802802            pseudoElementInfo.orderedStyles = this._collectStylesInCascadeOrder(pseudoElementInfo.matchedRules, null, null);
     803            this._associateRelatedProperties(pseudoElementInfo.orderedStyles);
    803804            this._markOverriddenProperties(pseudoElementInfo.orderedStyles);
    804             this._associateRelatedProperties(pseudoElementInfo.orderedStyles);
    805805        }
    806806    }
     
    847847    {
    848848        propertyNameToEffectiveProperty = propertyNameToEffectiveProperty || {};
     849
     850        function isOverriddenByRelatedShorthand(property) {
     851            let shorthand = property.relatedShorthandProperty;
     852            if (!shorthand)
     853                return false;
     854
     855            if (property.important && !shorthand.important)
     856                return false;
     857
     858            if (!property.important && shorthand.important)
     859                return true;
     860
     861            if (property.ownerStyle === shorthand.ownerStyle)
     862                return shorthand.index > property.index;
     863
     864            let propertyStyleIndex = styles.indexOf(property.ownerStyle);
     865            let shorthandStyleIndex = styles.indexOf(shorthand.ownerStyle);
     866            return shorthandStyleIndex > propertyStyleIndex;
     867        }
    849868
    850869        for (var i = 0; i < styles.length; ++i) {
     
    886905                }
    887906
    888                 property.overridden = false;
     907                if (isOverriddenByRelatedShorthand(property)) {
     908                    property.overridden = true;
     909                    property.overridingProperty = property.relatedShorthandProperty;
     910                } else
     911                    property.overridden = false;
    889912
    890913                propertyNameToEffectiveProperty[canonicalName] = property;
Note: See TracChangeset for help on using the changeset viewer.