| Summary: | Uncaught Exception: Can't make a ContentView for an unknown representedObject of type: IndexedDatabase | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | John Wilander <wilander> | ||||||||||||||||||||||
| Component: | Web Inspector | Assignee: | Nikita Vasilyev <nvasilyev> | ||||||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||||||
| Severity: | Normal | CC: | bburg, commit-queue, hi, inspector-bugzilla-changes, joepeck, mattbaker, nvasilyev, webkit-bug-importer | ||||||||||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||||||||||
| Hardware: | All | ||||||||||||||||||||||||
| OS: | All | ||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||
Thanks John, we are probably just missing a case for this. Steps: 1. Open http://www.girliemac.com/stickies/ 2. Click on "New note...", type "foo", press Enter 3. In Web Inspector Storage tab, expand Indexed Databases 4. Click "stickieDB" 5. Close Web Inspector 6. Open Web Inspector Created attachment 308617 [details]
[Animated GIF] Missing content view
There's no content view for:
1. Indexed Databases folder.
2. Indexed Database root item.
Currently, we only have a content view for an IndexedDB table.
Created attachment 308621 [details]
[Image] Chrome DevTools IndexedDB content view
Chrome DevTools show a DB name, security origin, and a version. There's also a button to delete a DB. We should do something like this, too.
> Chrome DevTools show a DB name, security origin, and a version. There's also
> a button to delete a DB. We should do something like this, too.
We should be showing must of this information in a context sensitive sidebar.
(In reply to Joseph Pecoraro from comment #6) > > Chrome DevTools show a DB name, security origin, and a version. There's also > > a button to delete a DB. We should do something like this, too. > > We should be showing must of this information in a context sensitive sidebar. Yes, we do show all of it in the sidebar. I think it's strange to switch from, say, Cookies to an indexed database and not update the content view. It could be misleading. Created attachment 308642 [details]
Patch
Created attachment 308643 [details]
[Image] With patch applied
What right sidebars are available with this patch? Is the IndexedDB sidebar available? Created attachment 308656 [details] [Image] Details sidebar (In reply to Joseph Pecoraro from comment #10) > What right sidebars are available with this patch? Is the IndexedDB sidebar > available? Maybe we shouldn't show a sidebar here at all. (In reply to Nikita Vasilyev from comment #11) > Created attachment 308656 [details] > [Image] Details sidebar > > (In reply to Joseph Pecoraro from comment #10) > > What right sidebars are available with this patch? Is the IndexedDB sidebar > > available? > > Maybe we shouldn't show a sidebar here at all. I'm not sure how to do this. When I remove WebInspector.IndexedDatabase from WebInspector.StorageTabContentView#canShowRepresentedObject clicking on the indexed database item in the sidebar ("stickieDB" on the screenshot) doesn't change the content view any longer :/ (In reply to Nikita Vasilyev from comment #12) > (In reply to Nikita Vasilyev from comment #11) > > Created attachment 308656 [details] > > [Image] Details sidebar > > > > (In reply to Joseph Pecoraro from comment #10) > > > What right sidebars are available with this patch? Is the IndexedDB sidebar > > > available? > > > > Maybe we shouldn't show a sidebar here at all. > > I'm not sure how to do this. When I remove WebInspector.IndexedDatabase from > WebInspector.StorageTabContentView#canShowRepresentedObject clicking on the > indexed database item in the sidebar ("stickieDB" on the screenshot) doesn't > change the content view any longer :/ Sorry, ignore this comment, it doesn't make a lot of sense :) (In reply to Nikita Vasilyev from comment #13) > (In reply to Nikita Vasilyev from comment #12) > > (In reply to Nikita Vasilyev from comment #11) > > > Created attachment 308656 [details] > > > [Image] Details sidebar > > > > > > (In reply to Joseph Pecoraro from comment #10) > > > > What right sidebars are available with this patch? Is the IndexedDB sidebar > > > > available? > > > > > > Maybe we shouldn't show a sidebar here at all. > > > > I'm not sure how to do this. When I remove WebInspector.IndexedDatabase from > > WebInspector.StorageTabContentView#canShowRepresentedObject clicking on the > > indexed database item in the sidebar ("stickieDB" on the screenshot) doesn't > > change the content view any longer :/ > > Sorry, ignore this comment, it doesn't make a lot of sense :) The confusion arises because we continue to show the view for current represented object when selecting a tree item without a valid represented object. I think your patch helps address this. Will review more thoroughly. Created attachment 308787 [details]
Patch
Same patch, except the details sidebar for IndexedDB top level item is always hidden since it has the same content as the content view.
Created attachment 308796 [details]
[Image] Other UI ideas
I like the IndexedDatabaseContentView added by this patch. In the future we may want to have it serve as a summary/dashboard for the whole database (see attached mockup).
There is some redundant information already contained in the breadcrumbs/sidebar, but might still be useful to think about for a future patch.
Comment on attachment 308787 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308787&action=review > Source/WebInspectorUI/ChangeLog:9 > + When an indexed database is selected in the Network navigation sidebar, show its host, security origin, and version. I think you mean Storage navigation sidebar. > Source/WebInspectorUI/ChangeLog:11 > + Previously, selecting an indexed database didn't change the content view. It could led to a misleading state typo: lead > Source/WebInspectorUI/ChangeLog:12 > + when an indexed database is selected in the sidebar, but the content view showed previosly selected item such as typo: previously > Source/WebInspectorUI/UserInterface/Main.html:513 > + <script src="Views/CloseButtonView.js"></script> Looks like it was included by mistake. You could do this with a DetailsSection directly in the ContentView. It would be easy then to add groups for object stores, etc, in the future.
To get the UI as you have it now, you'll want to hide the DetailsSection's header in CSS, and add some styles to ContentView.css to make it prettier:
.content-view .details-section {
font-size: 11px;
background-color: initial;
}
.content-view .details-section > .header {
font-size: 13px;
font-weight: 600;
background-color: initial;
}
.content-view .details-section > .content > .group > .row.simple > .label {
width: 120px;
font-weight: 600;
}
.content-view .details-section > .content > .group > .row.simple > .value {
color: var(--text-color-gray-medium);
}
.content-view .details-section:last-child {
border-bottom: none;
}
And I think you'll need to adjust the spacing around the section itself. (In reply to Matt Baker from comment #16) > Created attachment 308796 [details] > [Image] Other UI ideas > > I like the IndexedDatabaseContentView added by this patch. In the future we > may want to have it serve as a summary/dashboard for the whole database (see > attached mockup). > > There is some redundant information already contained in the > breadcrumbs/sidebar, but might still be useful to think about for a future > patch. I can't find larger versions of Database.png and DatabaseTable.png to use in the content view. Also, these icons are pre iOS 7 style, e.g. they aren't flat. We should update them at some point. (In reply to Nikita Vasilyev from comment #20) > (In reply to Matt Baker from comment #16) > > Created attachment 308796 [details] > > [Image] Other UI ideas > > > > I like the IndexedDatabaseContentView added by this patch. In the future we > > may want to have it serve as a summary/dashboard for the whole database (see > > attached mockup). > > > > There is some redundant information already contained in the > > breadcrumbs/sidebar, but might still be useful to think about for a future > > patch. > > I can't find larger versions of Database.png and DatabaseTable.png to use in > the content view. I think your original UI, using a DetailsSection rather than a custom table, is fine for this patch. > Also, these icons are pre iOS 7 style, e.g. they aren't flat. We should > update them at some point. Agree. I'd like something vector and monochrome (or at least flat colors). Created attachment 308868 [details]
Patch
Created attachment 308869 [details]
[Image] With patch applied
Comment on attachment 308868 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308868&action=review Looks good! r=me, with comments. > Source/WebInspectorUI/UserInterface/Views/ContentView.css:45 > +} This rule should go in IndexedDatabaseContentView.css. Headers will be nice to have when we have multiple sections in a ContentView. > Source/WebInspectorUI/UserInterface/Views/ContentView.css:52 > + width: auto; Much nicer than having a fixed width. > Source/WebInspectorUI/UserInterface/Views/IndexedDatabaseContentView.js:47 > + { Can the represented object change during the lifetime of the view? If not this should move to constructor. Comment on attachment 308868 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308868&action=review >> Source/WebInspectorUI/UserInterface/Views/IndexedDatabaseContentView.js:47 >> + { > > Can the represented object change during the lifetime of the view? If not this should move to constructor. Layout is called repeatedly as the view is resized, so this will need to move before landing. Created attachment 308954 [details]
Patch
Comment on attachment 308954 [details] Patch Clearing flags on attachment: 308954 Committed r216147: <http://trac.webkit.org/changeset/216147> All reviewed patches have been landed. Closing bug. |
------- Inspected URL: Local page Loading completed: true Frontend User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/604.1.5+ (KHTML, like Gecko) Uncaught Exceptions: - Can't make a ContentView for an unknown representedObject of type: IndexedDatabase (at ContentView.js:152:24) createFromRepresentedObject @ ContentView.js:152:24 contentViewForRepresentedObject @ ContentView.js:173:82 showContentViewForRepresentedObject @ ContentBrowser.js:134:63 showDefaultContentViewForTreeElement @ NavigationSidebarPanel.js:223:82 _checkElementsForPendingViewStateCookie @ NavigationSidebarPanel.js:781:79 _treeElementAddedOrChanged @ NavigationSidebarPanel.js:638:57 dispatch @ Object.js:170:30 dispatchEventToListeners @ Object.js:177:17 appendChild @ TreeOutline.js:177:54 _addIndexedDatabase @ StorageSidebarPanel.js:261:47 _indexedDatabaseWasAdded @ StorageSidebarPanel.js:246:33 dispatch @ Object.js:170:30 dispatchEventToListeners @ Object.js:177:17 processDatabase @ StorageManager.js:275:42 processDatabase @ [native code] _dispatchResponseToCallback @ Connection.js:146:27 _dispatchResponse @ Connection.js:116:45 dispatch @ Connection.js:69:35 dispatch @ InspectorBackend.js:151:49 dispatchNextQueuedMessageFromBackend @ MessageDispatcher.js:42:34 Additional Details: cause --> An uncaught exception was thrown while dispatching response callback for command IndexedDB.requestDatabase. ------- * STEPS TO REPRODUCE 1. The data records for the domain had been cleared, including IndexedDB. 2. The IndexedDB instance still showed in Web Inspector | Storage. I clicked it and got the crash.