[Webkit-unassigned] [Bug 91528] Embeddable Developer Tools

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Andrey Kosyakov <caseq at chromium.org>  2012-07-27 02:50:53 PST ---
(From update of attachment 153026)
View in context: https://bugs.webkit.org/attachment.cgi?id=153026&action=review

> Source/WebCore/inspector/front-end/InspectorFrontendHostStub.js:111
> +        if (this._hiddenPanels.indexOf(panel) == -1)

NB: == => ===

> Source/WebCore/inspector/front-end/InspectorFrontendHostStub.js:118
> +        if (panelIndex != -1)

NB: != => !==

> Source/WebCore/inspector/front-end/TimelineModel.js:170
> +     * @param {Object[]} records

Please run Source/WebCore/inspector/compile-front-end.py (you would need a closure compiler as ~/closure/compiler.jar).
The proper way to specify an array of objects is {Array.<Object>}

> Source/WebCore/inspector/front-end/TimelineModel.js:225
> +                    console.error(e.message);

This needs to be shown to the inspector user, so please use WebInspector.log(). Also, please add some test to help users to understand the error, e.g. WebInspector.UIString("Failed to load timeline log: %s", e.message);

> Source/WebCore/inspector/front-end/TimelineModel.js:226
> +                    return null;

Just "return;"?

> Source/WebCore/inspector/front-end/inspector.js:47
> +        var hiddenPanels = InspectorFrontendHost.hiddenPanels();

How about just:
var hiddenPanelsString = InspectorFrontendHost.hiddenPanels() || "";
if (WebInspector.queryParamsObject["hiddenPanels"])
    hiddenPanelsString += "," + WebInspector.queryParamsObject["hiddenPanels"];
var hiddenPanels = hiddenPanelsString.split(",")

> Source/WebCore/inspector/front-end/inspector.js:420
> +        WebInspector.inspectorView.setCurrentPanel(WebInspector.firstPanel());

There's InspectorView._panelOrder that may be used to determine first panel. Also, in case we have multiple panels, we should probably open that last used one.

> Source/WebCore/inspector/front-end/inspector.js:421
> +        if (typeof WebInspector.queryParamsObject["url"] !== "undefined")

We may want to load other files as well, so using just a presence of "url" parameter to trigger loading of timeline does not look fair. How about having something like command=openTimeline&url=foo, then have panels register command handlers for particular values of command parameter?

> Source/WebCore/inspector/front-end/inspector.js:582
> +WebInspector.firstPanel = function()

As above, this does not seem necessary.

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