Bug 97524 – Web Inspector: Add Breadcrumb on FileSystemView status bar

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

[Taiju Tsuiki]

Before removing its directory tree, we need other navigation UI to parent directory. I'm adding breadcrumb in this patch.
This UI might not scale for deep directory tree. So, we should add crumb collapsion like ElementPanel does, before expose it by default.

[Taiju Tsuiki]

Created attachment 165597 [details]
Patch

[WebKit Review Bot]

Comment on attachment 165597 [details]
Patch

Attachment 165597 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14036034

New failing tests:
http/tests/inspector/resource-tree/resource-tree-frame-add.html
http/tests/inspector/resource-tree/resource-tree-reload.html
http/tests/inspector/appcache/appcache-iframe-manifests.html
http/tests/inspector/resource-tree/resource-tree-non-unique-url.html
http/tests/inspector/appcache/appcache-swap.html
inspector/console/command-line-api-inspect.html
inspector/debugger/source-frame.html
inspector/storage-panel-dom-storage-update.html
inspector/database-table-name-excaping.html
inspector/storage-panel-dom-storage.html
http/tests/inspector/search/resources-search-match-index.html
http/tests/inspector/appcache/appcache-manifest-with-non-existing-file.html
http/tests/inspector/filesystem/directory-tree.html
http/tests/inspector/indexeddb/resources-panel.html

[Build Bot]

Comment on attachment 165597 [details]
Patch

Attachment 165597 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14030095

New failing tests:
http/tests/inspector/appcache/appcache-iframe-manifests.html
http/tests/inspector/appcache/appcache-swap.html
inspector/console/command-line-api-inspect.html
inspector/debugger/source-frame.html
inspector/storage-panel-dom-storage-update.html
inspector/database-table-name-excaping.html
inspector/storage-panel-dom-storage.html
http/tests/inspector/search/resources-search-match-index.html
http/tests/inspector/appcache/appcache-manifest-with-non-existing-file.html
http/tests/inspector/resource-tree/resource-tree-non-unique-url.html

[Vsevolod Vlasov]

Comment on attachment 165597 [details]
Patch

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

> Source/WebCore/inspector/front-end/BreadcrumbList.js:35
> +WebInspector.BreadcrumbList = function()

This doesn't look like a ui component to me.
It looks like a lot of boilerplate (most of it could be omitted if breadcrumb list implementation is moved to FileSystemView) with no meaningful code.
I think you should either inline this implementation in FileSystemView or extract something that could be reused by ElementsPanel (e.g. click handling logic definitely belongs to the generic breadcrumb list implementation).

> Source/WebCore/inspector/front-end/BreadcrumbList.js:46
> +    set children(x)

Please don't use setters/getters. We prefer not to use them as they cause some closure compilation problems.

> Source/WebCore/inspector/front-end/BreadcrumbList.js:56
> +    get element()

You could make element field public instead.

> Source/WebCore/inspector/front-end/BreadcrumbList.js:78
> +    this._element.wrapper = this;

redundant?

> Source/WebCore/inspector/front-end/FileSystemView.js:31
> +importScript("BreadcrumbList.js");

import scripts implied dependencies should match those in compile-front-end.
You should move add <script> element to inspector.html instead.

> Source/WebCore/inspector/front-end/FileSystemView.js:41
> +    this.registerRequiredCSS("breadcrumbList.css");

This should be moved to BreadcrumbList.js ?
Also there is no such file in the patch.

> Source/WebCore/inspector/front-end/FileSystemView.js:123
> +        this._crumbs.updateSizes();

This method could be public (to be used in some resize handler) but I think children setter should call it implicitly.

> Source/WebCore/inspector/front-end/FileSystemView.js:271
> +    this._fileSystemView = fileSystemView;

This one is not used currently.

> Source/WebCore/inspector/front-end/FileSystemView.js:281
> +        this._treeElement.select();

I don't think crumb element should depend on another ui representation of the folder (the one you are going to remove soon actually).
I would use fileSystemView and path pair instead.

[Brian Burg]

This feature is no longer in WebKit.