Bug 39822

Summary:

Web Inspector: Implement additional tabs support in ResourceView

Product:

WebKit

Reporter:

Alexander Pavlov (apavlov) <apavlov>

Component:

Web Inspector (Deprecated)

Assignee:

Alexander Pavlov (apavlov) <apavlov>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, bweinstein, eric, joepeck, keishi, pfeldman, pmuellr, rik, timothy, webkit.review.bot, yurys

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

38578

Attachments:

Description	Flags
[PATCH] Suggested solution	pfeldman: review-
[PATCH] Comments addressed	pfeldman: review+

Alexander Pavlov (apavlov)

Reported 2010-05-27 02:27:05 PDT

Since classes extending ResourceView may need to show additional tabs, besides "Headers" and "Content", a generified approach should be used, to be able to add and hide tabs using a tab ID (name).

Attachments
[PATCH] Suggested solution (11.80 KB, patch) 2010-05-27 04:00 PDT, Alexander Pavlov (apavlov)	pfeldman: review-	Details Formatted Diff Diff
[PATCH] Comments addressed (11.69 KB, patch) 2010-05-31 08:12 PDT, Alexander Pavlov (apavlov)	pfeldman: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Alexander Pavlov (apavlov)

Comment 1 2010-05-27 04:00:44 PDT

Created attachment 57218 [details] [PATCH] Suggested solution

Pavel Feldman

Comment 2 2010-05-27 07:10:46 PDT

Comment on attachment 57218 [details] [PATCH] Suggested solution 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!

Alexander Pavlov (apavlov)

Comment 3 2010-05-31 08:12:08 PDT

Created attachment 57467 [details] [PATCH] Comments addressed

Pavel Feldman

Comment 4 2010-05-31 09:34:14 PDT

Comment on attachment 57467 [details] [PATCH] Comments addressed WebCore/inspector/front-end/ResourceView.js:111 + this.tabbedPane.hideTab("content"); Now that the tabbed pane is a component, you should simply not add content tab in this case instead of adding and hiding it. WebCore/inspector/front-end/SourceView.js:54 + if (this.localSourceFrame) I think what we should do is to come up with the tab factory for resources. So that the factory was creating tabs of applicable types for each resource. As a result, you would break SourceView -> ResourceView inheritance, will prevent SourceView from being localSourceFrame-aware, will make local changes tab absolutely separate extension, get rid of nasty show / hide headers when SourceView is re-used in Scripts tab. Lots of goodness!

Alexander Pavlov (apavlov)

Comment 5 2010-05-31 10:20:56 PDT

Landed with the contentElement creation/hiding avoided Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/inspector/front-end/ResourceView.js M WebCore/inspector/front-end/SourceView.js M WebCore/inspector/front-end/TextViewer.js M WebCore/inspector/front-end/inspector.html Committed r60445

WebKit Review Bot

Comment 6 2010-05-31 10:42:42 PDT

http://trac.webkit.org/changeset/60445 might have broken GTK Linux 32-bit Release

Note You need to log in before you can comment on or make changes to this bug.