[Webkit-unassigned] [Bug 110641] WebInspector: Switch hide element shortcut in ElementsPanel to use a selector

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=110641





--- Comment #5 from egraether at chromium.org  2013-02-25 17:42:04 PST ---
(From update of attachment 189824)
View in context: https://bugs.webkit.org/attachment.cgi?id=189824&action=review

I tried to work on your comments, but I ended up with almost the same as before. It already works exactly the way we want, but because of how the methods for accessing the inspector resource are implemented on CSSStyleModel, this looks a bit weird. For example I can't just ask if the inspector resource exists, I can only check if a certain rule is associated with the resource (CSSStyleModel::getViaInspectorResourceForRule), also I can't just create the resource, I have to use CSSStyleModel::addRule which then creates the resource in the backend.

The new patch encapsulates the functionality of the hide shortcut a bit better and has some comments explaining why it works.

>> Source/WebCore/inspector/front-end/ElementsTreeOutline.js:491
>> +                this._addHideShortcutRuleToInspectorStyleSheet(node.id, hideShortcutSelector);
> 
> You only need to do this once.

This method is called more than once, but the style rule is only once added to the inspector resource.

>> Source/WebCore/inspector/front-end/ElementsTreeOutline.js:634
>> +            if (!resource)
> 
> Resource may exist due to user's add rule action.

Yes, but if this._hideRule was not set yet, then no resource is found. I found no better way of checking whether the resource has the rule so far.

>> Source/WebCore/inspector/front-end/ElementsTreeOutline.js:638
>> +        WebInspector.cssModel.getViaInspectorResourceForRule(this._hideRule || {}, resourceCallback.bind(this));
> 
> We need to move this into model via creating addInspectorRule method that receives both selector and rule body.

CSSStyleModel.addRule() already adds the rule to the inspector stylesheet. A method that also takes the rule body as argument would look something like this:

addInspectorRule: function(nodeID, selector, name, value)
{
    function successCallback(rule)
    {
        rule.style.appendProperty(name, value);
    }
    function failureCallback() {}

    this.addRule(nodeID, selector, successCallback, failureCallback);
}

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.