The list of cookies appears to show only one cookie for each name, ignoring differences in path, domain, etc., thus a developer could think they only have one cookie when they might have several.

We need to have our table widget work with compound keys to fix this.

It goes without saying that this column should be hidden and lets name the column "uniqueKey." Ideally this would be hidden from the user but I don't think it needs to be.

If we are enabling the storage panel by default we should fix this. We don't really need the md5 hash, the key can be name.host.path.

This is taking longer than I would have expected and there appear to be some issues setting cookies in Firefox that have the same name over different paths... especially when running local pages. Attaching my WIP patch and going back to working on browser.html.

Comment on attachment 8809871 [details] Bug 1146194 - Multiple cookies with the same name not shown https://reviewboard.mozilla.org/r/92372/#review94108 Sorry for the delay to getting to these reviews. It has been a busy week to say the least. Please address the comments I have before landing. ::: devtools/client/shared/widgets/TableWidget.js:619 (Diff revision 1) > > /** > * Populates the header context menu with the names of the columns along with > * displaying which columns are hidden or visible. > */ > - populateMenuPopup: function () { > + populateMenuPopup: function (reallyHiddenColumns = []) { Add a JSDoc for the reallyHiddenColumns param. ::: devtools/client/shared/widgets/TableWidget.js:679 (Diff revision 1) > * @param {string} sortOn > * The id of the column on which the table will be initially sorted on. > * @param {array} hiddenColumns > * Ids of all the columns that are hidden by default. > */ > - setColumns: function (columns, sortOn = this.sortedOn, hiddenColumns = []) { > + setColumns: function (columns, sortOn = this.sortedOn, hiddenColumns = [], Add a JSDoc for the reallyHiddenColumns parameter. ::: devtools/client/shared/widgets/TableWidget.js:710 (Diff revision 1) > if (this.firstColumn && id == this.firstColumn) { > continue; > } > > this.columns.set(id, new Column(this, id, columns[id])); > - if (hiddenColumns.indexOf(id) > -1) { > + if (hiddenColumns.includes(id) || reallyHiddenColumns.includes(id)) { I would add a comment that states the column is hidden. It was not easy to know that toggleColumn() would hide the column. We don't have enough JSDocs for the TableWidget which makes it hard to really know what is going on in all this logic. So, any time we edit this widget, we should try to make it a slightly more developer friendly. ::: devtools/client/storage/ui.js:803 (Diff revision 1) > > + if (f.hidden) { > + hiddenFields.push(f.name); > + } > + > + if (f.reallyHidden) { I am not really a fan of the reallyHidden attribute name. Perhaps we should call it private? ::: devtools/server/actors/storage.js:827 (Diff revision 1) > _removeCookies(host, opts = {}) { > + // We use a uniqueId to emulate compound keys for cookies. We need to > + // extract the cookie name to remove the correct cookie. > + if (opts.name) { > + opts.path = opts.name.split(SEPARATOR_GUID)[2]; > + opts.name = opts.name.split(SEPARATOR_GUID)[0]; Move opts.name above opts.path. That way it will appear more orderly with the indices (0, 2). I also think we should save the name.split array into a variable to avoid splitting twice. ::: devtools/server/tests/browser/browser_storage_cookies-duplicate-names.js:6 (Diff revision 1) > +/* vim: set ft=javascript ts=2 et sw=2 tw=80: */ > +/* Any copyright is dedicated to the Public Domain. > + http://creativecommons.org/publicdomain/zero/1.0/ */ > + > +"use strict"; > + Add a test description ::: devtools/server/tests/browser/browser_storage_cookies-duplicate-names.js:10 (Diff revision 1) > +"use strict"; > + > +const {StorageFront} = require("devtools/shared/fronts/storage"); > +Services.scriptloader.loadSubScript("chrome://mochitests/content/browser/devtools/server/tests/browser/storage-helpers.js", this); > + > +const cookies = { I would rename cookies to COOKIES or testData. Something to make it easier to know this is the cookies data to test and match for. ::: devtools/server/tests/browser/browser_storage_cookies-duplicate-names.js:71 (Diff revision 1) > + is(numHosts, 1, "Correct number of host entries for cookies"); > + return testCookiesObjects(0, cookiesActor.hosts, cookiesActor); > +} > + > +var testCookiesObjects = Task.async(function* (index, hosts, cookiesActor) { > + let host = Object.keys(hosts)[index]; Using Object.keys(hosts)[index] to traverse the hosts object felt pretty weird to me. I don't have a recommended fix for this, but wanted to point it out. ::: devtools/server/tests/browser/browser_storage_cookies-duplicate-names.js:73 (Diff revision 1) > +} > + > +var testCookiesObjects = Task.async(function* (index, hosts, cookiesActor) { > + let host = Object.keys(hosts)[index]; > + let matchItems = data => { > + debugger; Remove debugger statement.

Multiple test failures due to the compound primary keys.

Comment on attachment 8809871 [details] Bug 1146194 - Multiple cookies with the same name not shown More failing tests.

Change summary: 1. Fixed the original issue and allow cookies of the same name and different compound keys to be displayed. 2. Added a private uniqueKey column and made it possible for other storage types to do the same. 3. Fixed a bug with bookmarks. Bookmarks are row placeholders needed for when an items key is changed. Now that a key can consist of compound keys we need to update the bookmark more often. 4. Split browser_storage_dynamic_updates.js into 3 files: - browser_storage_dynamic_updates_cookies.js - browser_storage_dynamic_updates_localStorage.js - browser_storage_dynamic_updates_sessionStorage.js 5. Fixed many eslint issues in the files that I edited. 6. Changed tests to work with cookies, which now have compound keys. 7. For server-side tests we were not clearing cookies between tests. Some tests appear to have been written to expect the incorrect input. We now clear cookies between tests and work with the correct data. 8. localStorage was not cleared at the end of some tests potentially causing issues. 9. The order of the first three columns in the cookie table have been changed from name, path, domain to name, domain, path to be consistent with the order of fields in the compound key. 10. When querying cells in tests we now include data in hidden columns as this ultimately simplifies tests. 11. Improved error reporting in tests to show available keys when a row can't be found. This makes debugging tests much easier. 12. Changed numeric values in getFields() metheds to Boolean instead of 1 and 0 for readability.

Pushed by mratcliffe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/34fce7c12173 Multiple cookies with the same name not shown r=gl