Changeset 128291 in webkit


Ignore:
Timestamp:
Sep 12, 2012 4:36:15 AM (12 years ago)
Author:
apavlov@chromium.org
Message:

Web Inspector: [Elements] Sidebar panes not updated on style changes due to "class" attribute modifications
​https://bugs.webkit.org/show_bug.cgi?id=95722

Reviewed by Vsevolod Vlasov.

Source/WebCore:

The DOMAgent StyleInvalidated event has been removed in favor of the StylesSidebarPane explicitly listening on the
AttrModified/AttrRemoved events that result in those same updates.

  • inspector/front-end/DOMAgent.js:

(WebInspector.DOMAgent.prototype._attributeModified):
(WebInspector.DOMAgent.prototype._loadNodeAttributes):
(WebInspector.DOMAgent.prototype._childNodeRemoved):

  • inspector/front-end/ElementsTreeOutline.js:

(WebInspector.ElementsTreeElement.prototype.updateSelection): Drive-by: avoid a costly synchronous layout during DOM tree updates.

  • inspector/front-end/StylesSidebarPane.js:

(WebInspector.StylesSidebarPane):
(WebInspector.StylesSidebarPane.prototype._attributeChanged):
(WebInspector.StylesSidebarPane.prototype._canAffectCurrentStyles):

LayoutTests:

  • inspector/elements/edit-style-attribute.html: Renamed events on which to listen.
  • inspector/styles/override-screen-size.html: Drive-by: fixed race condition that was resulting in this test's failures.
  • inspector/styles/styles-update-from-js-expected.txt:
  • inspector/styles/styles-update-from-js.html: Added test cases for style updates on ancestor and sibling attributes' changes.
Location:
trunk
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r128288 r128291  
     12012-09-12  Alexander Pavlov  <apavlov@chromium.org>
     2
     3        Web Inspector: [Elements] Sidebar panes not updated on style changes due to "class" attribute modifications
     4        https://bugs.webkit.org/show_bug.cgi?id=95722
     5
     6        Reviewed by Vsevolod Vlasov.
     7
     8        * inspector/elements/edit-style-attribute.html: Renamed events on which to listen.
     9        * inspector/styles/override-screen-size.html: Drive-by: fixed race condition that was resulting in this test's failures.
     10        * inspector/styles/styles-update-from-js-expected.txt:
     11        * inspector/styles/styles-update-from-js.html: Added test cases for style updates on ancestor and sibling attributes' changes.
     12
    1132012-09-12  Csaba Osztrogonác  <ossy@webkit.org>
    214

  • trunk/LayoutTests/inspector/elements/edit-style-attribute-expected.txt

    r112668 r128291  
    55
    66Running: testSetNewValue
    7 WebInspector.DOMAgent.Events.StyleInvalidated should be issued
     7WebInspector.DOMAgent.Events.AttrModified should be issued
    88
    99Running: testSetSameValue

  • trunk/LayoutTests/inspector/elements/edit-style-attribute.html

    r126589 r128291  
    3232            InspectorTest.evaluateInPage("testSetNewValue()");
    3333
    34             WebInspector.domAgent.addEventListener(WebInspector.DOMAgent.Events.StyleInvalidated, listener);
     34            WebInspector.domAgent.addEventListener(WebInspector.DOMAgent.Events.AttrModified, listener);
    3535            function listener(event)
    3636            {
    37                 InspectorTest.addResult("WebInspector.DOMAgent.Events.StyleInvalidated should be issued");
    38                 WebInspector.domAgent.removeEventListener(WebInspector.DOMAgent.Events.StyleInvalidated, listener);
     37                InspectorTest.addResult("WebInspector.DOMAgent.Events.AttrModified should be issued");
     38                WebInspector.domAgent.removeEventListener(WebInspector.DOMAgent.Events.AttrModified, listener);
    3939                next();
    4040            }
    … …  
    4545            InspectorTest.evaluateInPage("testSetSameValue()");
    4646
    47             WebInspector.domAgent.addEventListener(WebInspector.DOMAgent.Events.StyleInvalidated, listener);
     47            WebInspector.domAgent.addEventListener(WebInspector.DOMAgent.Events.AttrModified, listener);
    4848            function listener(event)
    4949            {
    50                 InspectorTest.addResult("WebInspector.DOMAgent.Events.StyleInvalidated should not be issued");
    51                 WebInspector.domAgent.removeEventListener(WebInspector.DOMAgent.Events.StyleInvalidated, listener);
     50                InspectorTest.addResult("WebInspector.DOMAgent.Events.AttrModified should not be issued");
     51                WebInspector.domAgent.removeEventListener(WebInspector.DOMAgent.Events.AttrModified, listener);
    5252            }
    5353
    … …  
    5656            {
    5757                InspectorTest.addResult("WebInspector.DOMNode.prototype._setAttributesPayload should be called");
    58                 WebInspector.domAgent.removeEventListener(WebInspector.DOMAgent.Events.StyleInvalidated, listener);
     58                WebInspector.domAgent.removeEventListener(WebInspector.DOMAgent.Events.AttrModified, listener);
    5959                next();
    6060            }

  • trunk/LayoutTests/inspector/styles/override-screen-size.html

    r113709 r128291  
    159159    function overrideAndDumpData(width, height, callback)
    160160    {
    161         function selectCallback()
     161        function finalCallback()
    162162        {
    163163            InspectorTest.addResult("Main style:");
    … …  
    166166        }
    167167
     168        var gotSizes;
     169        var gotStyles;
     170        function stylesCallback()
     171        {
     172            if (gotSizes)
     173                return finalCallback();
     174            gotStyles = true;
     175        }
     176
     177        function sizesCallback()
     178        {
     179            gotSizes = true;
     180            if (gotStyles)
     181                finalCallback();
     182        }
     183
    168184        function applyCallback()
    169185        {
    170             getAndDumpSizes(selectCallback);
     186            getAndDumpSizes(sizesCallback);
    171187        }
    172188
    173189        InspectorTest.addResult("Override: " + width + "x" + height);
     190        InspectorTest.waitForStyles("main", stylesCallback);
    174191        applyOverride(width, height, applyCallback);
    175192    }
    … …  
    183200            InspectorTest.addResult("Window from page: " + result.inner);
    184201            InspectorTest.addResult("Body from page: " + result.body);
    185             callback();
     202            if (callback)
     203                callback();
    186204        }
    187205

  • trunk/LayoutTests/inspector/styles/styles-update-from-js-expected.txt

    r126656 r128291  
    1 Tests that changes to an inline style from JavaScript are reflected in the Styles pane and Elements tree.
     1Tests that changes to an inline style and ancestor/sibling className from JavaScript are reflected in the Styles pane and Elements tree.
    22
    33
    … …  
    55
    66Running: testSetStyleAttribute
    7 <div id="container" style="color: #daC0DE; border: 1px solid black;"></div>
     7<div id="container" style="color: #daC0DE; border: 1px solid black;">…</div>
    88[expanded]
    99element.style  { ()
    … …  
    2323    border-left-width: 1px;
    2424
     25======== Matched CSS Rules ========
     26[expanded]
     27div  { (user agent stylesheet)
     28display: block;
     29
    2530
    2631
    2732Running: testSetStyleCSSText
    28 <div id="container" style="color: rgb(192, 255, 238);"></div>
     33<div id="container" style="color: rgb(192, 255, 238);">…</div>
    2934[expanded]
    3035element.style  { ()
    3136color: #C0FFEE;
    3237
     38======== Matched CSS Rules ========
     39[expanded]
     40div  { (user agent stylesheet)
     41display: block;
     42
    3343
    3444
    3545Running: testSetViaParsedAttributes
    36 <div id="container" style="color: rgb(192, 255, 238); border: 3px dashed green;"></div>
     46<div id="container" style="color: rgb(192, 255, 238); border: 3px dashed green;">…</div>
    3747[expanded]
    3848element.style  { ()
    … …  
    5262    border-left-width: 3px;
    5363
     64======== Matched CSS Rules ========
     65[expanded]
     66div  { (user agent stylesheet)
     67display: block;
    5468
    5569
     70
     71Running: testSetViaAncestorClass
     72<div id="child"></div>
     73[expanded]
     74element.style  { ()
     75
     76======== Matched CSS Rules ========
     77[expanded]
     78.red div:first-child  { (styles-update-from-js.html:4)
     79background-color: red;
     80
     81[expanded]
     82div  { (user agent stylesheet)
     83display: block;
     84
     85======== Inherited from div#container.red ========
     86[expanded]
     87Style Attribute  { ()
     88color: #C0FFEE;
     89
     90
     91
     92Running: testSetViaSiblingAttr
     93<div id="childSibling"></div>
     94[expanded]
     95element.style  { ()
     96
     97======== Matched CSS Rules ========
     98[expanded]
     99div[foo="bar"] + div  { (styles-update-from-js.html:8)
     100background-color: blue;
     101
     102[expanded]
     103div  { (user agent stylesheet)
     104display: block;
     105
     106======== Inherited from div#container.red ========
     107[expanded]
     108Style Attribute  { ()
     109color: #C0FFEE;
     110
     111
     112

  • trunk/LayoutTests/inspector/styles/styles-update-from-js.html

    r125014 r128291  
    11<html>
    22<head>
     3<style>
     4.red div:first-child {
     5    background-color: red;
     6}
     7
     8div[foo="bar"] + div {
     9    background-color: blue;
     10}
     11
     12</style>
    313<script src="../../http/tests/inspector/inspector-test.js"></script>
    414<script src="../../http/tests/inspector/elements-test.js"></script>
    … …  
    1929    style.border = "2px dashed green";
    2030    style.borderWidth = "3px";
     31}
     32
     33function modifyContainerClass()
     34{
     35    document.getElementById("container").className = "red";
     36}
     37
     38function modifyChildAttr()
     39{
     40    document.getElementById("child").setAttribute("foo", "bar");
    2141}
    2242
    … …  
    4565            waitAndDumpAttributeAndStyles(next);
    4666            InspectorTest.evaluateInPage("modifyParsedAttributes()");
     67        },
     68
     69        function testSetViaAncestorClass(next)
     70        {
     71            InspectorTest.selectNodeAndWaitForStyles("child", callback);
     72
     73            function callback()
     74            {
     75                waitAndDumpAttributeAndStyles(next, "child");
     76                InspectorTest.evaluateInPage("modifyContainerClass()");
     77            }
     78        },
     79
     80        function testSetViaSiblingAttr(next)
     81        {
     82            InspectorTest.selectNodeAndWaitForStyles("childSibling", callback);
     83
     84            function callback()
     85            {
     86                waitAndDumpAttributeAndStyles(next, "childSibling");
     87                InspectorTest.evaluateInPage("modifyChildAttr()");
     88            }
    4789        }
    4890    ]);
    4991
    50     function waitAndDumpAttributeAndStyles(next)
     92    function waitAndDumpAttributeAndStyles(next, id)
    5193    {
     94        id = id || "container";
    5295        function callback()
    5396        {
    54             dumpAttributeAndStyles();
     97            dumpAttributeAndStyles(id);
    5598            next();
    5699        }
    57         InspectorTest.waitForStyles("container", callback);
     100        InspectorTest.waitForStyles(id, callback);
    58101    }
    59102
    60     function dumpAttributeAndStyles()
     103    function dumpAttributeAndStyles(id)
    61104    {
    62         var treeElement = findNodeTreeElement()
     105        var treeElement = findNodeTreeElement(id);
    63106        if (!treeElement) {
    64             InspectorTest.addResult("'container' tree element not found");
     107            InspectorTest.addResult("'" + id + "' tree element not found");
    65108            return;
    66109        }
    67110        InspectorTest.addResult(treeElement.listItemElement.textContent.replace(/\u200b/g, ""));
    68         InspectorTest.dumpSelectedElementStyles(true, true);
     111        InspectorTest.dumpSelectedElementStyles(true);
    69112    }
    70113
    71     function findNodeTreeElement()
     114    function findNodeTreeElement(id)
    72115    {
    73116        WebInspector.panels.elements.treeOutline._updateModifiedNodes();
    74         var expandedNode = InspectorTest.expandedNodeWithId("container");
     117        var expandedNode = InspectorTest.expandedNodeWithId(id);
    75118        if (!expandedNode) {
    76             InspectorTest.addResult("'container' node not found");
     119            InspectorTest.addResult("'" + id + "' node not found");
    77120            InspectorTest.completeTest();
    78121        }
    … …  
    85128<body onload="runTest()">
    86129<p>
    87 Tests that changes to an inline style from JavaScript are reflected in the Styles pane and Elements tree.
     130Tests that changes to an inline style and ancestor/sibling className from JavaScript are reflected in the Styles pane and Elements tree.
    88131</p>
    89132
    90 <div id="container" style="font-weight:bold"></div>
     133<div id="container" style="font-weight:bold"><div id="child"></div><div id="childSibling"></div></div>
    91134
    92135</body>

  • trunk/Source/WebCore/ChangeLog

    r128290 r128291  
     12012-09-12  Alexander Pavlov  <apavlov@chromium.org>
     2
     3        Web Inspector: [Elements] Sidebar panes not updated on style changes due to "class" attribute modifications
     4        https://bugs.webkit.org/show_bug.cgi?id=95722
     5
     6        Reviewed by Vsevolod Vlasov.
     7
     8        The DOMAgent StyleInvalidated event has been removed in favor of the StylesSidebarPane explicitly listening on the
     9        AttrModified/AttrRemoved events that result in those same updates.
     10
     11        * inspector/front-end/DOMAgent.js:
     12        (WebInspector.DOMAgent.prototype._attributeModified):
     13        (WebInspector.DOMAgent.prototype._loadNodeAttributes):
     14        (WebInspector.DOMAgent.prototype._childNodeRemoved):
     15        * inspector/front-end/ElementsTreeOutline.js:
     16        (WebInspector.ElementsTreeElement.prototype.updateSelection): Drive-by: avoid a costly synchronous layout during DOM tree updates.
     17        * inspector/front-end/StylesSidebarPane.js:
     18        (WebInspector.StylesSidebarPane):
     19        (WebInspector.StylesSidebarPane.prototype._attributeChanged):
     20        (WebInspector.StylesSidebarPane.prototype._canAffectCurrentStyles):
     21
    1222012-09-12  Simon Hausmann  <simon.hausmann@nokia.com>
    223

  • trunk/Source/WebCore/inspector/front-end/DOMAgent.js

    r126576 r128291  
    795795    ChildNodeCountUpdated: "ChildNodeCountUpdated",
    796796    InspectElementRequested: "InspectElementRequested",
    797     StyleInvalidated: "StyleInvalidated",
    798797    UndoRedoRequested: "UndoRedoRequested",
    799798    UndoRedoCompleted: "UndoRedoCompleted"
    … …  
    917916        if (!node)
    918917            return;
    919         var issueStyleInvalidated = name === "style" && value !== node.getAttribute("style");
    920918
    921919        node._setAttribute(name, value);
    922920        this.dispatchEventToListeners(WebInspector.DOMAgent.Events.AttrModified, { node: node, name: name });
    923         if (issueStyleInvalidated)
    924           this.dispatchEventToListeners(WebInspector.DOMAgent.Events.StyleInvalidated, node)
    925921    },
    926922
    … …  
    966962            var node = this._idToDOMNode[nodeId];
    967963            if (node) {
    968                 if (node._setAttributesPayload(attributes)) {
     964                if (node._setAttributesPayload(attributes))
    969965                    this.dispatchEventToListeners(WebInspector.DOMAgent.Events.AttrModified, { node: node, name: "style" });
    970                     this.dispatchEventToListeners(WebInspector.DOMAgent.Events.StyleInvalidated, node);
    971                 }
    972966            }
    973967        }
    … …  
    10811075        parent._removeChild(node);
    10821076        this._unbind(node);
    1083         this.dispatchEventToListeners(WebInspector.DOMAgent.Events.NodeRemoved, {node:node, parent:parent});
     1077        this.dispatchEventToListeners(WebInspector.DOMAgent.Events.NodeRemoved, {node: node, parent: parent});
    10841078    },
    10851079

  • trunk/Source/WebCore/inspector/front-end/ElementsTreeOutline.js

    r128043 r128291  
    813813            return;
    814814
    815         if (document.body.offsetWidth <= 0) {
    816             // The stylesheet hasn't loaded yet or the window is closed,
    817             // so we can't calculate what is need. Return early.
    818             return;
     815        if (!this._readyToUpdateSelection) {
     816            if (document.body.offsetWidth > 0)
     817                this._readyToUpdateSelection = true;
     818            else {
     819                // The stylesheet hasn't loaded yet or the window is closed,
     820                // so we can't calculate what we need. Return early.
     821                return;
     822            }
    819823        }
    820824

  • trunk/Source/WebCore/inspector/front-end/StylesSidebarPane.js

    r127142 r128291  
    9898    WebInspector.cssModel.addEventListener(WebInspector.CSSStyleModel.Events.StyleSheetChanged, this._styleSheetOrMediaQueryResultChanged, this);
    9999    WebInspector.cssModel.addEventListener(WebInspector.CSSStyleModel.Events.MediaQueryResultChanged, this._styleSheetOrMediaQueryResultChanged, this);
    100     WebInspector.domAgent.addEventListener(WebInspector.DOMAgent.Events.AttrModified, this._attributesModified, this);
    101     WebInspector.domAgent.addEventListener(WebInspector.DOMAgent.Events.AttrRemoved, this._attributesRemoved, this);
    102     WebInspector.domAgent.addEventListener(WebInspector.DOMAgent.Events.StyleInvalidated, this._styleInvalidated, this);
     100    WebInspector.domAgent.addEventListener(WebInspector.DOMAgent.Events.AttrModified, this._attributeChanged, this);
     101    WebInspector.domAgent.addEventListener(WebInspector.DOMAgent.Events.AttrRemoved, this._attributeChanged, this);
    103102    WebInspector.settings.showUserAgentStyles.addChangeListener(this._showUserAgentStylesSettingChanged.bind(this));
    104103}
    … …  
    315314    },
    316315
    317     _attributesModified: function(event)
    318     {
    319         if (this.node !== event.data.node)
    320             return;
    321 
    322         // Changing style attribute will anyways generate _styleInvalidated message.
    323         if (event.data.name === "style")
    324             return;
    325 
    326         // "class" (or any other) attribute might have changed. Update styles unless they are being edited.
    327         if (!this._isEditingStyle && !this._userOperation)
    328             this._rebuildUpdate();
    329     },
    330 
    331     _attributesRemoved: function(event)
    332     {
    333         if (this.node !== event.data.node)
    334             return;
    335 
    336         // "style" attribute might have been removed.
    337         if (!this._isEditingStyle && !this._userOperation)
    338             this._rebuildUpdate();
    339     },
    340 
    341     _styleInvalidated: function(event)
    342     {
    343         if (this.node !== event.data)
    344             return;
    345 
    346         if (!this._isEditingStyle && !this._userOperation)
    347             this._rebuildUpdate();
     316    _attributeChanged: function(event)
     317    {
     318        // Any attribute removal or modification can affect the styles of "related" nodes.
     319        // Do not touch the styles if they are being edited.
     320        if (this._isEditingStyle || this._userOperation)
     321            return;
     322
     323        if (!this._canAffectCurrentStyles(event.data.node))
     324            return;
     325
     326        this._rebuildUpdate();
     327    },
     328
     329    _canAffectCurrentStyles: function(node)
     330    {
     331        return this.node && (this.node === node || node.parentNode === this.node.parentNode || node.isAncestor(this.node));
    348332    },
    349333
Note: See TracChangeset for help on using the changeset viewer.