Changeset 283723 in webkit


Ignore:
Timestamp:
Oct 7, 2021 11:15:14 AM (10 months ago)
Author:
Nikita Vasilyev
Message:

Web Inspector: Styles: format style declarations after editing
​https://bugs.webkit.org/show_bug.cgi?id=178835
<rdar://problem/35185060>

Reviewed by Devin Rousso.

Source/WebInspectorUI:

Indent CSS properties with spaces/tabs set in Web Inspector settings. Increse indentation level when CSS rules are
inside of at-rules (e.g. @media, @keyframes, @supports).

Don't indent CSS properties in style attributes. Keep them on the single line, separated by a space character:

style="font-size: 12px; color: black;"

  • UserInterface/Models/CSSProperty.js:

(WI.CSSProperty.prototype.set text):
Introduce _isTextPendingSave flag. It's needed when saving pasted text, and saving commented out or uncommented CSS properties.

(WI.CSSProperty.prototype.get formattedText):

(WI.CSSProperty.prototype.replaceWithText): Deleted.
This is redundant - setting text works the same.

(WI.CSSProperty.prototype._updateStyleText):
(WI.CSSProperty.prototype._updateOwnerStyleText):
(WI.CSSProperty.prototype._prependSemicolonIfNeeded): Deleted.
Greatly simplify the logic now that we save formatted text and don't modify styleText.

  • UserInterface/Models/CSSStyleDeclaration.js:

(WI.CSSStyleDeclaration.prototype.removeProperty):
Remode unnecessary code that modifies _styleSheetTextRange. The backend sends new _styleSheetTextRange data
upon a change.

(WI.CSSStyleDeclaration.prototype.generateFormattedText): Renamed from 'generateCSSRuleString'.

  • UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:

(WI.SpreadsheetCSSStyleDeclarationSection.prototype._populateIconElementContextMenu):

  • UserInterface/Views/SpreadsheetStyleProperty.js:

(WI.SpreadsheetStyleProperty.prototype.remove):

LayoutTests:

  • Indent text of expected CSS properties.
  • Test generateFormattedText with all permutations of its options.
  • Add a basic test ensuring styleSheetTextRange updates correctly from the backend.
  • inspector/css/add-css-property.html:
  • inspector/css/generateCSSRuleString-expected.txt: Removed.
  • inspector/css/generateCSSRuleString.html: Removed.
  • inspector/css/generateFormattedText-expected.txt: Added.
  • inspector/css/generateFormattedText.html: Added.
  • inspector/css/modify-css-property-expected.txt:
  • inspector/css/modify-css-property.html:
  • inspector/css/modify-inline-style-expected.txt:
  • inspector/css/resources/modify-css-property.css: Added.

(.rule-a):
(.rule-b):
(.rule-c):
(.rule-d):

Location:
trunk
Files:
3 added
2 deleted
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r283719 r283723  
     12021-10-07  Nikita Vasilyev  <nvasilyev@apple.com>
     2
     3        Web Inspector: Styles: format style declarations after editing
     4        https://bugs.webkit.org/show_bug.cgi?id=178835
     5        <rdar://problem/35185060>
     6
     7        Reviewed by Devin Rousso.
     8
     9        - Indent text of expected CSS properties.
     10        - Test generateFormattedText with all permutations of its options.
     11        - Add a basic test ensuring styleSheetTextRange updates correctly from the backend.
     12
     13        * inspector/css/add-css-property.html:
     14        * inspector/css/generateCSSRuleString-expected.txt: Removed.
     15        * inspector/css/generateCSSRuleString.html: Removed.
     16        * inspector/css/generateFormattedText-expected.txt: Added.
     17        * inspector/css/generateFormattedText.html: Added.
     18        * inspector/css/modify-css-property-expected.txt:
     19        * inspector/css/modify-css-property.html:
     20        * inspector/css/modify-inline-style-expected.txt:
     21        * inspector/css/resources/modify-css-property.css: Added.
     22        (.rule-a):
     23        (.rule-b):
     24        (.rule-c):
     25        (.rule-d):
     26
    1272021-10-07  Ayumi Kojima  <ayumi_kojima@apple.com>
    228

  • trunk/LayoutTests/inspector/css/add-css-property.html

    r253242 r283723  
    2828            newProperty.rawValue = "green";
    2929
    30             InspectorTest.expectEqual(styleDeclaration.text, `font-size: 14px;color: green;`, "`color: green` property should be appended.");
     30            InspectorTest.expectEqual(styleDeclaration.text, `\n    font-size: 14px;\n    color: green;\n`, "`color: green` property should be appended.");
    3131
    3232            styleDeclaration.locked = false;
    … …  
    4444            newProperty.rawValue = "green";
    4545
    46             let expected = `color: green;\n    outline: 2px solid brown;\n`;
     46            let expected = `\n    color: green;\n    outline: 2px solid brown;\n`;
    4747            InspectorTest.expectEqual(styleDeclaration.text, expected, "`color: green` property should be inserted before the first property.");
    4848
    … …  
    5252            newProperty.rawValue = "block";
    5353
    54             expected = `color: green;\n    outline: 2px solid brown;display: block;\n`;
     54            expected = `\n    color: green;\n    outline: 2px solid brown;\n    display: block;\n`;
    5555            InspectorTest.expectEqual(styleDeclaration.text, expected, "`display: block` property should be appended at the end.");
    5656
    … …  
    6969            newProperty.rawValue = "fantasy";
    7070
    71             let expected = `\n    background-color: peachpuff;font-family: fantasy;\n    color: burlywood\n`;
     71            let expected = `\n    background-color: peachpuff;\n    font-family: fantasy;\n    color: burlywood;\n`;
    7272            InspectorTest.expectEqual(styleDeclaration.text, expected, "`font-family: fantasy` property should be inserted after the first property.");
    7373
    … …  
    8686            newProperty.rawValue = "inline";
    8787
    88             const expected = `\n    font-size: 14px;\n    /* color: red; */display: inline;\n`;
     88            const expected = `\n    font-size: 14px;\n    /* color: red; */\n    display: inline;\n`;
    8989            InspectorTest.expectEqual(styleDeclaration.text, expected, "`display: inline` property should be appended.");
    9090
    … …  
    103103            newProperty.rawValue = "inline";
    104104
    105             const expected = `\n    font-size: 14px;\n    /* color: red */display: inline;\n`;
     105            const expected = `\n    font-size: 14px;\n    /* color: red; */\n    display: inline;\n`;
    106106            InspectorTest.expectEqual(styleDeclaration.text, expected, "`display: inline` property should be appended.");
    107107

  • trunk/LayoutTests/inspector/css/modify-css-property-expected.txt

    r240946 r283723  
    66PASS: "font-size" property value should update immediately.
    77PASS: Style declaration text should update immediately.
     8PASS: styleSheetTextRange should correspond to "font-size: 10px;"
    89
    910-- Running test case: Update value when CSSStyleDeclaration is locked

  • trunk/LayoutTests/inspector/css/modify-css-property.html

    r253242 r283723  
    22<html>
    33<head>
     4<link rel="stylesheet" href="resources/modify-css-property.css">
    45<script src="../../http/tests/inspector/resources/inspector-test.js"></script>
    56<script>
    … …  
    4041            let styleDeclaration = getMatchedStyleDeclaration();
    4142            styleDeclaration.locked = false;
     43
     44            styleDeclaration.singleFireEventListener(WI.CSSStyleDeclaration.Event.PropertiesChanged, function(event) {
     45                let fontProperty = getProperty("font-size");
     46                let relativeRange = fontProperty.styleSheetTextRange.relativeTo(fontProperty.ownerStyle.styleSheetTextRange.startLine, fontProperty.ownerStyle.styleSheetTextRange.startOffset);
     47                relativeRange.resolveOffsets(fontProperty.ownerStyle.text);
     48                let propertyText = fontProperty.ownerStyle.text.slice(relativeRange.startOffset, relativeRange.endOffset);
     49                InspectorTest.expectEqual(propertyText, "font-size: 10px;", `styleSheetTextRange should correspond to "font-size: 10px;"`);
     50                resolve();
     51            });
     52
    4253            getProperty("font-size").rawValue = "11px";
    4354            getProperty("font-size").rawValue = "10px";
    … …  
    4556            InspectorTest.expectEqual(getProperty("font-size").rawValue, "10px", `"font-size" property value should update immediately.`);
    4657
    47             InspectorTest.expectEqual(styleDeclaration.text, `font-size: 10px; color: antiquewhite`, `Style declaration text should update immediately.`);
    48 
    49             resolve();
     58            InspectorTest.expectEqual(styleDeclaration.text, `\n    font-size: 10px;\n    color: antiquewhite;\n`, `Style declaration text should update immediately.`);
    5059        }
    5160    });
    … …  
    8190
    8291            InspectorTest.expectEqual(styleDeclaration.text, `
    83         font-size: 16px;
    84         color: #333;
    85 
    86         margin-left: 0;
    87         margin-top: 1em;
    88     `, `Style declaration text should update immediately.`);
     92    font-size: 16px;
     93    color: #333;
     94    margin-left: 0;
     95    margin-top: 1em;
     96`, `Style declaration text should update immediately.`);
    8997
    9098            styleDeclaration.locked = false;
    … …  
    244252            getProperty("padding-right").commentOut(disabled);
    245253
    246             let expectedStyleText = `\n        /* padding-left: 2em; */\n        padding-right: 0px;\n    `;
     254            let expectedStyleText = `\n    /* padding-left: 2em; */\n    padding-right: 0px;\n`;
    247255            InspectorTest.expectEqual(styleDeclaration.text, expectedStyleText, `Style declaration text should update immediately with uncommented property.`);
    248256
    … …  
    252260            getProperty("padding-right").commentOut(disabled);
    253261
    254             expectedStyleText = `\n        /* padding-left: 2em; */\n        /* padding-right: 0px; */\n    `;
     262            expectedStyleText = `\n    /* padding-left: 2em; */\n    /* padding-right: 0px; */\n`;
    255263            InspectorTest.expectEqual(styleDeclaration.text, expectedStyleText, `Style declaration text should update immediately with commented out property.`);
    256264
    … …  
    291299            getProperty("font-size").commentOut(disabled);
    292300
    293             let expectedStyleText = `font-size: 13px;/*border: 2px solid brown*/`;
     301            let expectedStyleText = `\n    font-size: 13px;\n    /* border: 2px solid brown; */\n`;
    294302            InspectorTest.expectEqual(styleDeclaration.text, expectedStyleText, `Style declaration text should update immediately with uncommented property.`);
    295303
    … …  
    300308            getProperty("border").commentOut(disabled);
    301309
    302             expectedStyleText = `font-size: 13px;border: 2px solid brown`;
     310            expectedStyleText = `\n    font-size: 13px;\n    border: 2px solid brown\n`;
    303311            InspectorTest.expectEqual(styleDeclaration.text, expectedStyleText, `Style declaration text should update immediately with commented out property.`);
    304312
    … …  
    332340<body onload="runTest()">
    333341    <p>Testing that CSSStyleDeclaration update immediately after modifying its properties when it is not locked.</p>
    334 
    335     <style>
    336     .rule-a {
    337         font-size: 14px;
    338         color: #333;
    339 
    340         margin-left: 0;
    341         margin-top: 1em;
    342     }
    343     .rule-b {font-size: 12px; color: antiquewhite}
    344     .rule-c {
    345         /* padding-left: 2em; */
    346         /* padding-right: 0px; */
    347     }
    348     .rule-d {/*font-size: 13px;*//*border: 2px solid brown*/}
    349     </style>
    350342    <div id="x" class="test-node rule-a rule-b rule-c rule-d" style="width: 100px"></div>
    351343</body>

  • trunk/LayoutTests/inspector/css/modify-inline-style-expected.txt

    r241623 r283723  
    88
    99CSSProperty changed to "color: red;".
    10 font: 12px normal sans-serif!important;color: red;
     10font: 12px normal sans-serif!important; color: red;
    1111
    1212CSSProperty changed to "font: 12px sans-serif;".
    13 font: 12px sans-serif;color: red;
     13font: 12px sans-serif; color: red;
    1414
    1515CSSProperty changed to "color: invalid_c010r;".
    16 font: 12px sans-serif;color: invalid_c010r;
     16font: 12px sans-serif; color: invalid_c010r;
    1717
    1818

  • trunk/Source/WebInspectorUI/ChangeLog

    r283572 r283723  
     12021-10-07  Nikita Vasilyev  <nvasilyev@apple.com>
     2
     3        Web Inspector: Styles: format style declarations after editing
     4        https://bugs.webkit.org/show_bug.cgi?id=178835
     5        <rdar://problem/35185060>
     6
     7        Reviewed by Devin Rousso.
     8
     9        Indent CSS properties with spaces/tabs set in Web Inspector settings. Increse indentation level when CSS rules are
     10        inside of at-rules (e.g. @media, @keyframes, @supports).
     11
     12        Don't indent CSS properties in style attributes. Keep them on the single line, separated by a space character:
     13
     14            style="font-size: 12px; color: black;"
     15
     16        * UserInterface/Models/CSSProperty.js:
     17        (WI.CSSProperty.prototype.set text):
     18        Introduce `_isTextPendingSave` flag. It's needed when saving pasted text, and saving commented out or uncommented CSS properties.
     19
     20        (WI.CSSProperty.prototype.get formattedText):
     21
     22        (WI.CSSProperty.prototype.replaceWithText): Deleted.
     23        This is redundant - setting `text` works the same.
     24
     25        (WI.CSSProperty.prototype._updateStyleText):
     26        (WI.CSSProperty.prototype._updateOwnerStyleText):
     27        (WI.CSSProperty.prototype._prependSemicolonIfNeeded): Deleted.
     28        Greatly simplify the logic now that we save formatted text and don't modify styleText.
     29
     30        * UserInterface/Models/CSSStyleDeclaration.js:
     31        (WI.CSSStyleDeclaration.prototype.removeProperty):
     32        Remode unnecessary code that modifies `_styleSheetTextRange`. The backend sends new `_styleSheetTextRange` data
     33        upon a change.
     34
     35        (WI.CSSStyleDeclaration.prototype.generateFormattedText): Renamed from 'generateCSSRuleString'.
     36
     37        * UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:
     38        (WI.SpreadsheetCSSStyleDeclarationSection.prototype._populateIconElementContextMenu):
     39        * UserInterface/Views/SpreadsheetStyleProperty.js:
     40        (WI.SpreadsheetStyleProperty.prototype.remove):
     41
    1422021-10-05  Patrick Angle  <pangle@apple.com>
    243

  • trunk/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js

    r278607 r283723  
    3535        this._initialState = null;
    3636        this._modified = false;
     37        this._isUpdatingText = false;
    3738
    3839        this.update(text, name, value, priority, enabled, overridden, implicit, anonymous, valid, styleSheetTextRange, true);
    … …  
    185186    }
    186187
    187     replaceWithText(text)
    188     {
    189         this._markModified();
    190 
    191         this._updateOwnerStyleText(this._text, text, true);
    192     }
    193 
    194188    commentOut(disabled)
    195189    {
    … …  
    214208    set text(newText)
    215209    {
     210        newText = newText.trim();
    216211        if (this._text === newText)
    217212            return;
    218213
    219214        this._markModified();
    220         this._updateOwnerStyleText(this._text, newText);
    221215        this._text = newText;
     216        this._isUpdatingText = true;
     217        this._updateOwnerStyleText();
     218        this._isUpdatingText = false;
    222219    }
    223220
    224221    get formattedText()
    225222    {
     223        if (this._isUpdatingText)
     224            return this._text;
     225
    226226        if (!this._name)
    227227            return "";
    … …  
    502502        }
    503503
    504         let oldText = this._text;
    505504        this._text = text;
    506         this._updateOwnerStyleText(oldText, this._text, forceRemove);
    507     }
    508 
    509     _updateOwnerStyleText(oldText, newText, forceRemove = false)
    510     {
    511         if (oldText === newText) {
    512             if (forceRemove) {
    513                 const lineDelta = 0;
    514                 const columnDelta = 0;
    515                 this._ownerStyle.shiftPropertiesAfter(this, lineDelta, columnDelta, forceRemove);
    516                 this._ownerStyle.updatePropertiesModifiedState();
    517             }
    518             return;
    519         }
    520 
     505
     506        if (forceRemove)
     507            this._ownerStyle.removeProperty(this);
     508
     509        this._updateOwnerStyleText();
     510    }
     511
     512    _updateOwnerStyleText()
     513    {
    521514        console.assert(this._ownerStyle);
    522515        if (!this._ownerStyle)
    523516            return;
    524517
    525         this._prependSemicolonIfNeeded();
    526 
    527         let styleText = this._ownerStyle.text || "";
    528 
    529         // _styleSheetTextRange is the position of the property within the stylesheet.
    530         // range is the position of the property within the rule.
    531         let range = this._styleSheetTextRange.relativeTo(this._ownerStyle.styleSheetTextRange.startLine, this._ownerStyle.styleSheetTextRange.startColumn);
    532 
    533         // Append a line break to count the last line of styleText towards endOffset.
    534         range.resolveOffsets(styleText + "\n");
    535 
    536         console.assert(oldText === styleText.slice(range.startOffset, range.endOffset), "_styleSheetTextRange data is invalid.");
    537 
    538         if (WI.settings.debugEnableStyleEditingDebugMode.value) {
    539             let prefix = styleText.slice(0, range.startOffset);
    540             let postfix = styleText.slice(range.endOffset);
    541             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`);
    542         }
    543 
    544         let newStyleText = styleText.slice(0, range.startOffset) + newText + styleText.slice(range.endOffset);
    545 
    546         let lineDelta = newText.lineCount - oldText.lineCount;
    547         let columnDelta = newText.lastLine.length - oldText.lastLine.length;
    548         this._styleSheetTextRange = this._styleSheetTextRange.cloneAndModify(0, 0, lineDelta, columnDelta);
    549 
    550         this._ownerStyle.text = newStyleText;
    551 
    552         let propertyWasRemoved = !newText;
    553         this._ownerStyle.shiftPropertiesAfter(this, lineDelta, columnDelta, propertyWasRemoved);
     518        this._ownerStyle.text = this._ownerStyle.generateFormattedText({multiline: this._ownerStyle.type === WI.CSSStyleDeclaration.Type.Rule});
    554519        this._ownerStyle.updatePropertiesModifiedState();
    555     }
    556 
    557     _prependSemicolonIfNeeded()
    558     {
    559         for (let i = this.index - 1; i >= 0; --i) {
    560             let property = this._ownerStyle.properties[i];
    561             if (!property.enabled)
    562                 continue;
    563 
    564             let match = property.text.match(/[^;\s](\s*)$/);
    565             if (match)
    566                 property.text = property.text.trimRight() + ";" + match[1];
    567 
    568             break;
    569         }
    570520    }
    571521

  • trunk/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js

    r262302 r283723  
    446446    }
    447447
    448     shiftPropertiesAfter(cssProperty, lineDelta, columnDelta, propertyWasRemoved)
     448    removeProperty(cssProperty)
    449449    {
    450450        // cssProperty.index could be set to NaN by WI.CSSStyleDeclaration.prototype.update.
    … …  
    453453            return;
    454454
    455         let endLine = cssProperty.styleSheetTextRange.endLine;
    456 
    457         for (let i = realIndex + 1; i < this._properties.length; i++) {
    458             let property = this._properties[i];
    459 
    460             if (property._styleSheetTextRange) {
    461                 if (property.styleSheetTextRange.startLine === endLine) {
    462                     // Only update column data if it's on the same line.
    463                     property._styleSheetTextRange = property._styleSheetTextRange.cloneAndModify(lineDelta, columnDelta, lineDelta, columnDelta);
    464                 } else
    465                     property._styleSheetTextRange = property._styleSheetTextRange.cloneAndModify(lineDelta, 0, lineDelta, 0);
    466             }
    467 
    468             if (propertyWasRemoved && !isNaN(property._index))
    469                 property._index--;
    470         }
    471 
    472         if (propertyWasRemoved)
    473             this._properties.splice(realIndex, 1);
     455        this._properties.splice(realIndex, 1);
    474456
    475457        // Invalidate cached properties.
    … …  
    508490    }
    509491
    510     generateCSSRuleString()
     492    generateFormattedText(options = {})
    511493    {
    512494        let indentString = WI.indentString();
    … …  
    514496        let groupings = this.groupings.filter((grouping) => grouping.text !== "all");
    515497        let groupingsCount = groupings.length;
    516         for (let i = groupingsCount - 1; i >= 0; --i)
    517             styleText += indentString.repeat(groupingsCount - i - 1) + groupings[i].prefix + " " + groupings[i].text + " {\n";
    518 
    519         styleText += indentString.repeat(groupingsCount) + this.selectorText + " {\n";
    520 
    521         for (let property of (this._styleSheetTextRange ? this.visibleProperties : this._properties))
    522             styleText += indentString.repeat(groupingsCount + 1) + property.formattedText + "\n";
    523 
    524         for (let i = groupingsCount; i > 0; --i)
    525             styleText += indentString.repeat(i) + "}\n";
    526 
    527         styleText += "}";
     498
     499        if (options.includeGroupingsAndSelectors) {
     500            for (let i = groupingsCount - 1; i >= 0; --i) {
     501                if (options.multiline)
     502                    styleText += indentString.repeat(groupingsCount - i - 1);
     503
     504                styleText += groupings[i].prefix + " " + groupings[i].text + " {";
     505
     506                if (options.multiline)
     507                    styleText += "\n";
     508            }
     509
     510            if (options.multiline)
     511                styleText += indentString.repeat(groupingsCount);
     512
     513            styleText += this.selectorText + " {";
     514        }
     515
     516        let properties = this._styleSheetTextRange ? this.visibleProperties : this._properties;
     517        if (properties.length) {
     518            if (options.multiline) {
     519                let propertyIndent = indentString.repeat(groupingsCount + 1);
     520                for (let property of properties)
     521                    styleText += "\n" + propertyIndent + property.formattedText;
     522
     523                styleText += "\n";
     524                if (!options.includeGroupingsAndSelectors) {
     525                    // Indent the closing "}" for nested rules.
     526                    styleText += indentString.repeat(groupingsCount);
     527                }
     528            } else
     529                styleText += properties.map((property) => property.formattedText).join(" ");
     530        }
     531
     532        if (options.includeGroupingsAndSelectors) {
     533            for (let i = groupingsCount; i > 0; --i) {
     534                if (options.multiline)
     535                    styleText += indentString.repeat(i);
     536
     537                styleText += "}";
     538
     539                if (options.multiline)
     540                    styleText += "\n";
     541            }
     542
     543            styleText += "}";
     544        }
    528545
    529546        return styleText;

  • trunk/Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js

    r278607 r283723  
    482482    {
    483483        contextMenu.appendItem(WI.UIString("Copy Rule"), () => {
    484             InspectorFrontendHost.copyText(this._style.generateCSSRuleString());
     484            InspectorFrontendHost.copyText(this._style.generateFormattedText({includeGroupingsAndSelectors: true, multiline: true}));
    485485        });
    486486

  • trunk/Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js

    r281663 r283723  
    145145
    146146        if (replacement)
    147             this._property.replaceWithText(replacement);
     147            this._property.text = replacement;
    148148        else
    149149            this._property.remove();
Note: See TracChangeset for help on using the changeset viewer.