[Webkit-unassigned] [Bug 122546] Web Inspector: inspector forgets open navigation sidebar when re-opening
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Oct 9 14:35:04 PDT 2013
https://bugs.webkit.org/show_bug.cgi?id=122546
Timothy Hatcher <timothy at apple.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #213774|review? |review-
Flag| |
--- Comment #3 from Timothy Hatcher <timothy at apple.com> 2013-10-09 14:33:54 PST ---
(From update of attachment 213774)
View in context: https://bugs.webkit.org/attachment.cgi?id=213774&action=review
Looks good. Just fix up the layering violations.
> Source/WebInspectorUI/UserInterface/ApplicationCacheManager.js:115
> + return (matchOnTypeAlone && this._applicationCacheObjects.length) ? this._applicationCacheObjects[0] : null;
No need for the ( ).
> Source/WebInspectorUI/UserInterface/ContentBrowser.js:78
> + // These listeners are for events that could resolve a pending content view cookie.
> + WebInspector.applicationCacheManager.addEventListener(WebInspector.ApplicationCacheManager.Event.FrameManifestAdded, this._resolveAndShowPendingContentViewCookie, this);
> + WebInspector.frameResourceManager.addEventListener(WebInspector.FrameResourceManager.Event.MainFrameDidChange, this._resolveAndShowPendingContentViewCookie, this);
> + WebInspector.storageManager.addEventListener(WebInspector.StorageManager.Event.DatabaseWasAdded, this._resolveAndShowPendingContentViewCookie, this);
> + WebInspector.storageManager.addEventListener(WebInspector.StorageManager.Event.CookieStorageObjectWasAdded, this._resolveAndShowPendingContentViewCookie, this);
> + WebInspector.storageManager.addEventListener(WebInspector.StorageManager.Event.DOMStorageObjectWasAdded, this._resolveAndShowPendingContentViewCookie, this);
This is a layering violation. Moving this to Main.js and possibly calling into a public function on ContentBrowser for generic logic would be best.
> Source/WebInspectorUI/UserInterface/ContentBrowser.js:577
> + _resolveAndShowPendingContentViewCookie: function(matchOnTypeAlone)
> + {
> + var cookie = this._pendingContentViewCookie;
> + if (!cookie)
> + return false;
> +
> + var representedObject = null;
> +
> + if (cookie.type === WebInspector.ContentView.ResourceContentViewCookieType)
> + representedObject = WebInspector.frameResourceManager.objectForCookie(cookie);
> +
> + if (cookie.type === WebInspector.ContentView.TimelinesContentViewCookieType)
> + representedObject = WebInspector.timelineManager.objectForCookie(cookie);
> +
> + if (cookie.type === WebInspector.ContentView.CookieStorageContentViewCookieType || cookie.type === WebInspector.ContentView.DatabaseContentViewCookieType || cookie.type === WebInspector.ContentView.DatabaseTableContentViewCookieType || cookie.type === WebInspector.ContentView.DOMStorageContentViewCookieType)
> + representedObject = WebInspector.storageManager.objectForCookie(this._pendingContentViewCookie, matchOnTypeAlone);
> +
> + if (cookie.type === WebInspector.ContentView.ApplicationCacheContentViewCookieType)
> + representedObject = WebInspector.applicationCacheManager.objectForCookie(this._pendingContentViewCookie, matchOnTypeAlone);
> +
> + if (!representedObject)
> + return false;
> +
> + // If we reached this point, then we should be able to create and/or display a content view based on the cookie.
> + delete this._pendingContentViewCookie;
> + if (this._lastAttemptCookieCheckingTimeout)
> + clearTimeout(this._lastAttemptCookieCheckingTimeout);
> +
> + // Delay this work because other listeners of the originating event might not have fired yet.
> + // So displaying the content view before those listeners do their work might cause the
> + // dependent view states (navigation sidebar tree elements, path components) to be wrong.
> + function delayedWork()
> + {
> + this.showContentViewForRepresentedObject(representedObject, cookie);
> + }
> + setTimeout(delayedWork.bind(this), 0);
> + return true;
> + },
This should also move to Main.js since it touches many high level classes that ContentBrowser should not know about.
> Source/WebInspectorUI/UserInterface/ContentView.js:105
> +WebInspector.ContentView.ApplicationCacheContentViewCookieType = "application-cache";
> +WebInspector.ContentView.CookieStorageContentViewCookieType = "cookie-storage";
> +WebInspector.ContentView.DatabaseContentViewCookieType = "database";
> +WebInspector.ContentView.DatabaseTableContentViewCookieType = "database-table";
> +WebInspector.ContentView.DOMStorageContentViewCookieType = "dom-storage";
> +WebInspector.ContentView.TimelinesContentViewCookieType = "timeline";
> +WebInspector.ContentView.ResourceContentViewCookieType = "resource"; // includes Frame too.
This is also a layering violation. Main.js would be best. Also this format would be better and more like an enum:
WebInspector.ContentVIewCookieType = {
ApplicationCache: "application-cache",
...
};
--
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