When the IDB inspector was implemented only string-type key paths were supported for the IDBObjectStore.keyPath and IDBIndex.keyPath properties. Null was not distinguished from the empty string. https://bugs.webkit.org/show_bug.cgi?id=84207 is adding support for array-type keypaths, which will need to be displayed somehow. Also, null and empty string should be distinguished, as the behavior differs.
This can be implemented as soon as 85298 lands (a subset of 84207)
Ready to go. A stub was landed with 85298 but should be replaced.
Created attachment 146575 [details] Patch
Comment on attachment 146575 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=146575&action=review overall lgtm > Source/WebCore/inspector/InspectorIndexedDBAgent.cpp:244 > + Vector<String> stringArray = idbKeyPath.array(); Can you make this const Vector<String>& stringArray to avoid a copy? > Source/WebCore/inspector/front-end/IndexedDBModel.js:158 > + return "\"" + idbKeyPath + "\""; Do we care about escaping strings that contain " characters (etc)? E.g. using JSON.stringify(s) or something? (I don't think I've seen this in the inspector myself, so as long as we're consistent)
Created attachment 146605 [details] Screenshot1
Created attachment 146606 [details] Screenshot2
(In reply to comment #5) > Created an attachment (id=146605) [details] > Screenshot1 Nice - I notice that the closing ) is missing or truncated in the column headers in the screenshots, i.e.: Key (keyPath: "string" Primary key (keyPath: "id.index" Key (keyPath: ["number", "string"] Just a display issue? It looks like it's present in the code in _keyPathHeader
You caught me in the middle of uploading a new patch :) I've already spotted the closing bracket problem and a couple of others. I am going to fix these and upload a new version soon.
Created attachment 146607 [details] Patch
(In reply to comment #4) > (From update of attachment 146575 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=146575&action=review > > overall lgtm > > > Source/WebCore/inspector/InspectorIndexedDBAgent.cpp:244 > > + Vector<String> stringArray = idbKeyPath.array(); > > Can you make this const Vector<String>& stringArray to avoid a copy? Done. > > Source/WebCore/inspector/front-end/IndexedDBModel.js:158 > > + return "\"" + idbKeyPath + "\""; > > Do we care about escaping strings that contain " characters (etc)? E.g. using JSON.stringify(s) or something? > > (I don't think I've seen this in the inspector myself, so as long as we're consistent) keyPathString is now used only for tooltips and keyPath itself comes from the backend so I don't think we really need escaping here.
Comment on attachment 146607 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=146607&action=review > Source/WebCore/inspector/front-end/IndexedDBViews.js:170 > + keyColumnHeaderFragment.appendChild(document.createTextNode(" (" + WebInspector.UIString("keyPath") + ": ")); Do you need to localize "keyPath"? If yes, it should be UIString("(keyPath: "), etc.
Committed r119901: <http://trac.webkit.org/changeset/119901>