Bug 48963 – Web Inspector: Enable file system UI for chromium

• Component: Web Inspector (Deprecated)
• Status: RESOLVED WONTFIX

[Kavita Kanetkar]

Enable this only for chromium.

[Kavita Kanetkar]

Created attachment 72889 [details]
patch

In chromium File System can be disabled using --disable-file-system. Propagating that information to inspector UI.

[Adam Barth]

Comment on attachment 72889 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=72889&action=review

I wish we had more tests for the inspector...

> WebCore/inspector/front-end/FileSystemView.js:125
> +    },

Missing a blank line after this line?

[Kavita Kanetkar]

Thanks. I am working on tests for inspector in the next patch.

[WebKit Commit Bot]

Comment on attachment 72889 [details]
patch

Clearing flags on attachment: 72889

Committed r71366: <http://trac.webkit.org/changeset/71366>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Pavel Feldman]

Adam, I'd appreciate if you could talk to the inspector reviewers / contributors prior to blessing enabling a new feature in the UI. Have you seen the screenshots of the functionality?

[Adam Barth]

(In reply to comment #6)
> Adam, I'd appreciate if you could talk to the inspector reviewers / contributors prior to blessing enabling a new feature in the UI. Have you seen the screenshots of the functionality?

I'm sad that the inspector is its own world inside of WebKit.  I'll be more communicative in the future.

[Kavita Kanetkar]

I can set Preferences.enableFileSystem to false until you are happy with the UI.
But this patch also had 1) detecting whether or not File System is disabled and 2) calling the new readFileSystem instead of requestFileSystem which does not create root directory if it does not exist. Thanks.

[Pavel Feldman]

> I'm sad that the inspector is its own world inside of WebKit.  I'll be more communicative in the future.

Inspector is not more its world than HTML parser, DOM bindings or DB. It is just that when you become a WebKit reviewer, you take responsibility to r+ only changes in the area of your competence / expertise, being fully aware of the context of what's happening. It is not the first time I can see you either r+-ing or r- -ing changes without much context in the inspector feature. This time you approved unhiding of a feature without knowing what it looks like. Hence I bring it up.

I can give you a handful of names of people that are aware of the context in the inspector feature set, or you can use the tool you are authoring to find them out.

[Adam Barth]

I followed up with Pavel directly.  I'm going to try to engage more with the inspector part of the project by writing some code rather than by commenting on bugs / reviewing patches.

[Pavel Feldman]

Created attachment 73175 [details]
[IMAGE] Present looks.

I think it is worth hiding FS support behind the flag until UI is fixed and there is a user-friendly message for no FS case.

[Timothy Hatcher]

I agree.

[Pavel Feldman]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebKit/chromium/ChangeLog
	M	WebKit/chromium/src/js/DevTools.js
Committed r71478

[Pavel Feldman]

This functionality is on hold until we know what we want here (i.e. API itself is stable).