[Webkit-unassigned] [Bug 45982] Web Inspector: FileSystem integration

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Oct 13 06:09:26 PDT 2010


https://bugs.webkit.org/show_bug.cgi?id=45982


Pavel Feldman <pfeldman at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #70385|review?                     |review-
               Flag|                            |




--- Comment #15 from Pavel Feldman <pfeldman at chromium.org>  2010-10-13 06:09:26 PST ---
(From update of attachment 70385)
View in context: https://bugs.webkit.org/attachment.cgi?id=70385&action=review

Looks good. Few things to fix in code and UI before we land it!

> WebCore/inspector/InspectorFileSystemAgent.cpp:129
> +    AsyncFileSystem::Type asyncFileSystemType = static_cast<AsyncFileSystem::Type>(type);

This really should be handled outside, but that is the generator's problem, I understand.

> WebCore/inspector/InspectorFileSystemAgent.cpp:162
> +    m_frontend->didGetFileSystemPath("Error while retrieving root", static_cast<unsigned int>(type));

Nit: This is not exactly a 'path' you are passing in here. You should add a separate 'success' / 'errorMessage' out parameter to report about errors.

> WebCore/inspector/front-end/FileSystemView.js:49
> +    this._tabbedPane.appendTab("persistent", WebInspector.UIString("Persistent File System"), this._persistentFileSystemElement, this._selectFileSystemTab.bind(this, true));

Still no localizedStrings.js file changed.

> WebCore/inspector/front-end/FileSystemView.js:88
> +        WebInspector.FileSystem.getFileSystemPathAsync(this.fileSystemType.PERSISTENT);

Nit: given that these are always called next to each other, should we return array of objects?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list