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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Feb 28 02:49:15 PST 2013


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


Alexander Pavlov (apavlov) <apavlov at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #190613|review?                     |
               Flag|                            |




--- Comment #14 from Alexander Pavlov (apavlov) <apavlov at chromium.org>  2013-02-28 02:51:38 PST ---
(From update of attachment 190613)
View in context: https://bugs.webkit.org/attachment.cgi?id=190613&action=review

> Source/WebCore/ChangeLog:11
> +        the document by creating a style tag in the documents head and in the head of each

Could you test that the feature works for nested frames, too? Please see LayoutTests/inspector/styles/resources/styles-source-lines-inline-iframe.html for how to synchronize on iframe loading.

> Source/WebCore/inspector/front-end/DOMAgent.js:800
> +        var classString = this.getAttribute("class") || "";

You could implement this using the same RuntimeAgent.evaluate() approach as below if you could retrieve the target node in the evaluated code. Pavel can advise on this. If that were possible, the code would be as simple as

var toggleClass = function(className, enable)
{
    var node = ...
    if (enable)
        node.classList.add(className);
    else
        node.classList.remove(className);
}

> Source/WebCore/inspector/front-end/ElementsTreeOutline.js:620
> +     * @param {function()=} userCallback

This signature should match what toggleClassName() accepts:

{function(?Protocol.Error)=}

> Source/WebCore/inspector/front-end/ElementsTreeOutline.js:624
> +        var className = "web-inspector-hide-shortcut";

I would use some weird system-like prefix for these className and styleTagId, perhaps "__" or similar, in order to avoid collisions with the user code as much as possible.

> Source/WebCore/inspector/front-end/ElementsTreeOutline.js:628
> +        var injectStyleRuleCode = [

You can just declare "var injectStyleRuleCode = function() {...}" and then evaluate something like
"(" + injectStyleRuleCode.toString() + ")()"
(take a look at how a similar thing is done in DOMAgent.prototype._emulateTouchEventsChanged. That approach will barely work for you, since it requires a page reload for the code to get injected).

> Source/WebCore/inspector/front-end/ElementsTreeOutline.js:629
> +            "var style = document.head.querySelector('style#" + styleTagId + "');",

Excellent

> Source/WebCore/inspector/front-end/ElementsTreeOutline.js:646
> +        RuntimeAgent.evaluate(injectStyleRuleCode, runtimeCallback.bind(this));

Are you sure it evaluates the script in every frame out there?

> LayoutTests/inspector/elements/hide-shortcut.html:137
> +Tests the hide shortcut, which toggles a class name on the node and sets visibility: hidden on the node and it's ancestors.

We usually specify a link to the respective webkit bug after the description:

...and its ancestors. <a href="https://bugs.webkit.org/show_bug.cgi?id=110641">Bug 110641</a>.

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



More information about the webkit-unassigned mailing list