Bug 90529

Summary:

Web Inspector: Add FileContentView for FileSystemView

Product:

WebKit

Reporter:

Taiju Tsuiki <tzik>

Component:

Web Inspector (Deprecated)

Assignee:

Taiju Tsuiki <tzik>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, vsevik, webkit.review.bot, yurys

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

90439, 90518

Bug Blocks:

68203, 90592

Attachments:

Description	Flags
Patch	none
Patch	none

Taiju Tsuiki

Reported 2012-07-04 01:46:42 PDT

For FileSystem support of Inspector, I'd like to add a view to show content of a file. This patch adds FileContentView, this view is only for text file.

Attachments
Patch (12.41 KB, patch) 2012-07-04 01:49 PDT, Taiju Tsuiki	no flags	Details Formatted Diff Diff
Patch (12.57 KB, patch) 2012-07-09 02:17 PDT, Taiju Tsuiki	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Taiju Tsuiki

Comment 1 2012-07-04 01:49:15 PDT

Created attachment 150738 [details] Patch

Vsevolod Vlasov

Comment 2 2012-07-05 13:58:24 PDT

Comment on attachment 150738 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150738&action=review > Source/WebCore/inspector/front-end/FileContentView.js:49 > + if (this._file.resourceType.isTextType()) You should use another way to detect text files, see https://bugs.webkit.org/show_bug.cgi?id=90518 for details. > Source/WebCore/inspector/front-end/FileContentView.js:61 > + if (errorCode !== 0 || !metadata) if (errorCode || !metadata) > Source/WebCore/inspector/front-end/FileContentView.js:82 > + if (this._file.resourceType.isTextType()) Ditto. > Source/WebCore/inspector/front-end/FileContentView.js:95 > +WebInspector.FileContentView.Content = function(file, metadata) WebInspector.FileContentProvider I would move all this logic to WebInspector.FileSystemModel.File actually. > Source/WebCore/inspector/front-end/FileSystemView.js:114 > + if (this._entry.isDirectory) { Please remove braces. > Source/WebCore/inspector/front-end/FileSystemView.js:196 > + } else { No braces for one liners.

Taiju Tsuiki

Comment 3 2012-07-09 02:17:26 PDT

Created attachment 151217 [details] Patch

Taiju Tsuiki

Comment 4 2012-07-09 02:26:03 PDT

Comment on attachment 150738 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150738&action=review >> Source/WebCore/inspector/front-end/FileContentView.js:49 >> + if (this._file.resourceType.isTextType()) > > You should use another way to detect text files, see https://bugs.webkit.org/show_bug.cgi?id=90518 for details. I moved it to back-end and added isTextFile property. >> Source/WebCore/inspector/front-end/FileContentView.js:61 >> + if (errorCode !== 0 || !metadata) > > if (errorCode || !metadata) Done >> Source/WebCore/inspector/front-end/FileContentView.js:95 >> +WebInspector.FileContentView.Content = function(file, metadata) > > WebInspector.FileContentProvider > I would move all this logic to WebInspector.FileSystemModel.File actually. Renamed to FileContentProvider. I'd like to separate file and metadata to keep file immutable. Anyway, should I move FileContentProvider under FileSystemModel? >> Source/WebCore/inspector/front-end/FileSystemView.js:114 >> + if (this._entry.isDirectory) { > > Please remove braces. Done >> Source/WebCore/inspector/front-end/FileSystemView.js:196 >> + } else { > > No braces for one liners. Done

Vsevolod Vlasov

Comment 5 2012-07-09 02:36:39 PDT

> Anyway, should I move FileContentProvider under FileSystemModel? Sounds reasonable.

WebKit Review Bot

Comment 6 2012-07-09 05:34:03 PDT

Comment on attachment 151217 [details] Patch Clearing flags on attachment: 151217 Committed r122104: <http://trac.webkit.org/changeset/122104>

WebKit Review Bot

Comment 7 2012-07-09 05:34:08 PDT

All reviewed patches have been landed. Closing bug.

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