[webkit-reviews] review denied: [Bug 54965] Web Inspector: factor search logic out of inspector.js : [Attachment 83332] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Feb 22 10:48:08 PST 2011


Pavel Feldman <pfeldman at chromium.org> has denied Andrey Kosyakov
<caseq at chromium.org>'s request for review:
Bug 54965: Web Inspector: factor search logic out of inspector.js
https://bugs.webkit.org/show_bug.cgi?id=54965

Attachment 83332: patch
https://bugs.webkit.org/attachment.cgi?id=83332&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=83332&action=review

> Source/WebCore/inspector/front-end/Object.js:92
> +WebInspector.declareSingleton = function(constructor)

I don't see why search controller is different from domAgent, debuggerAgent,
etc. We should simply initialize it onload.

> Source/WebCore/inspector/front-end/ScriptsPanel.js:-1029
> -	   WebInspector.updateSearchMatchesCount(0, this);

I think this call is still needed.

> Source/WebCore/inspector/front-end/SearchController.js:198
> +	       for (var panelName in this.panels) {

I don't think your controller has panels. Please test this change thoroughly.

> Source/WebCore/inspector/front-end/SearchController.js:224
> +	   this.currentQuery = query;

Could you encapsulate this?


More information about the webkit-reviews mailing list