[webkit-reviews] review denied: [Bug 39822] Web Inspector: Implement additional tabs support in ResourceView : [Attachment 57218] [PATCH] Suggested solution

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu May 27 07:10:45 PDT 2010


Pavel Feldman <pfeldman at chromium.org> has denied Alexander Pavlov (apavlov)
<apavlov at chromium.org>'s request for review:
Bug 39822: Web Inspector: Implement additional tabs support in ResourceView
https://bugs.webkit.org/show_bug.cgi?id=39822

Attachment 57218: [PATCH] Suggested solution
https://bugs.webkit.org/attachment.cgi?id=57218&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
WebCore/inspector/front-end/ResourceView.js: 
 +   *	   notice, this list of conditions and the following disclaimer. 
Please do not mix formatting changes with actual refactorings.


WebCore/inspector/front-end/ResourceView.js:125
 +	    this._tabObjects[id] = {tab: tabElement, content: contentElement};
I think you should extract this functionality into a separate tabbed pane class
that is capable of showing views (or dom elements) in it. It then would be
reusable in other places of the ui such as styles sidebar.

WebCore/inspector/front-end/SourceView.js:208
 +		this._appendTab("local", WebInspector.UIString("Local"),
this.localContentElement, this.selectLocalContentTab.bind(this));
You should not call private methods - make this one public!


More information about the webkit-reviews mailing list