[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