The table headers of the previously selected storage are still shown when selecting a storage, which doesn't contain any items. Steps to reproduce: 1. Open the Storage Inspector on this page 2. Expand 'Cookies' and select 'bugzilla.mozilla.org' => Some cookies will be shown in a table with the headers 'Name', 'Path', ..., 'isHttpOnly' 3. Select 'Local Storage' => The headers of the 'Cookies' table are still shown. Sebastian

I might be wrong, but I think bugsahoy requires the two whiteboard flags to be split. Sebastian

This bug happens because the table headers are reset only after at least one row of data is received from the server [1]. If there are no data, the client doesn't even know which columns are there - it fully depends on the info provided by the server. Mike, how would you recommend to fix this? I see two ways: 1) The knowledge about which columns are displayed for each data type would be embedded in the client. This would also remove the need for the getEditableFields() method, as the client would already know what is editable and what isn't. 2) The getEditableFields() method would be transformed into getFields(), and would return the full column info about the table (names and editable flags). Independently from the getStoreObjects() return value. I like this approach more than #1, because it keeps the knowledge about data at one place, the server. [1] https://dxr.mozilla.org/mozilla-central/source/devtools/client/storage/ui.js#420

I like #2 best. If we do that we can also include data types for the columns (useful to e.g. display a date picker for dates) and, in the future, validation functions. Definitely the way to go.

I would like to take up this bug. Can you give me the specifics ? Thanks in advance.

First you need to set up a build environment as described in https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions. The relevant comments are comment 0, comment 2 and comment 3. Mike Ratcliffe may provide more info on this. Sebastian

@Michael, Since this bug hasn't been touched for almost 1 month, I'd like to take on it. Some question about the solution, I'd like to discuss with you: I can find out the getEditableFields() method in [1]. If we are going for the solution #2 at comment 2 proposed by Jarda. About the returned data format for column info, I am thinking maybe it could return like something below: ``` [ { name: "name", editable: 1 }, { name: "value", editable: 1 }, // For the future extension, such as including data type. It could be // { name: "last-updated-time", editable: 1, dataType: "date" }, ..... ] ``` How do you think about this data format? Please let know me any suggestion or concern, thank you. [1] https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/storage.js#1022

(In reply to [:Fischer]Fischer from comment #6) > @Michael, > Since this bug hasn't been touched for almost 1 month, I'd like to take on > it. > > Some question about the solution, I'd like to discuss with you: > I can find out the getEditableFields() method in [1]. If we are going for > the solution #2 at comment 2 proposed by Jarda. > About the returned data format for column info, I am thinking maybe it could > return like something below: > ``` > [ > { name: "name", editable: 1 }, > { name: "value", editable: 1 }, > // For the future extension, such as including data type. It could be > // { name: "last-updated-time", editable: 1, dataType: "date" }, > ..... > ] > ``` > > How do you think about this data format? > > Please let know me any suggestion or concern, thank you. > > > [1] > https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/ > storage.js#1022 Yes, this is exactly the way we should go. Columns can also be hidden but I am not sure if you will need to include that in the column meta.

@Mike, Please see my patch:1264582-patch-v1 for this bug. Just tested and now even no data for one storage type, the table column name would still be updated when selecting host. Here are some questions, could you help to give me some feedbacks: - Currently it is that selecting one *host* will trigger the table column update not *Storage title*. This is to follow existing logic of "onHostSelect" event. I am not sure if this is enough since the bug description is "Select Local Storage"(Storage title*). But right now the situation is improved comparing to the old situation that the table only updates when data exists under one host. - The "getEditableFields" is replaced by the "getFields" method. However, for the indexedDB type, the situation is a little bit complex because it have sub types: database and object store, not only host. As a result, we need to do sub type handling. Also, the metadata of indexedDB is kind of different from field info so I don't add the field info into metadata. Please let me if there is any better approach. Thanks

@Fischer: I have no access to the review / feedback request until you publish it (it is currently a draft). Can you open it in reviewboard and click publish?

Review commit: https://reviewboard.mozilla.org/r/62308/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/62308/

@Mike, Sorry, didn't notice that. Now it is published, thank you.

I am reviewing things at the moment but will review this soon.

Comment on attachment 8770361 [details] Bug 1264582 - Table headers are not removed when selecting an empty storage https://reviewboard.mozilla.org/r/62308/#review63372 A great set of changes that begins to introduce the possibility of specifying column data types in the future. In addition it fixes the table headers. If you correct the grammatical errors I have highlighted and the try run is green I will be happy to r+ it. ::: devtools/client/storage/ui.js:221 (Diff revision 1) > > - makeFieldsEditable: function* () { > - let actor = this.getCurrentActor(); > - > - if (typeof actor.getEditableFields !== "undefined") { > - let fields = yield actor.getEditableFields(); > + /** > + * Make column fields editable > + * > + * @param {Array} editableFields > + * An array of keys of columns which being made editable An array of keys of columns which to be made editable ::: devtools/server/actors/storage.js:84 (Diff revision 1) > * store objects. > * - toStoreObject : Given a store object, convert it to the required format > * so that it can be transferred over wire. > * - populateStoresForHost : Given a host, populate the map of all store > * objects for it > + * - getFields: Given a subType(optional), get an array of object containing get an array of objects containing

Sorry, the above corrections should be: An array of keys of columns to be made editable And: get an array of objects containing Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=11e6f29db3a8

https://reviewboard.mozilla.org/r/62308/#review63372 > An array of keys of columns which to be made editable Sorry, that should be: An array of keys of columns to be made editable

Comment on attachment 8770361 [details] Bug 1264582 - Table headers are not removed when selecting an empty storage Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62308/diff/1-2/

Comment on attachment 8770361 [details] Bug 1264582 - Table headers are not removed when selecting an empty storage @Mike, Thank you for correcting the grammar and pushing the try server. I will do it next time. I have correct the grammar. Please see the patch. Thanks

Comment on attachment 8770361 [details] Bug 1264582 - Table headers are not removed when selecting an empty storage https://reviewboard.mozilla.org/r/62308/#review63582

Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/fx-team/rev/e6c96d5612d3 Table headers are not removed when selecting an empty storage. r=mratcliffe

Sorry to say that, but this bug is not fixed for me as of Nightly 51.0a1 (2016-08-02). The headers are still not removed when the list is empty. See the test case in comment 0. Mike, should we reopen this bug or create a new one? Sebastian

(In reply to Sebastian Zartner [:sebo] from comment #21) > Sorry to say that, but this bug is not fixed for me as of Nightly 51.0a1 > (2016-08-02). The headers are still not removed when the list is empty. See > the test case in comment 0. > Mike, should we reopen this bug or create a new one? > > Sebastian Please can you create a new one?

(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #22) > (In reply to Sebastian Zartner [:sebo] from comment #21) > > Sorry to say that, but this bug is not fixed for me as of Nightly 51.0a1 > > (2016-08-02). The headers are still not removed when the list is empty. See > > the test case in comment 0. > > Mike, should we reopen this bug or create a new one? > > > > Sebastian > > Please can you create a new one? Ok, I've filed bug 1291427. Maybe you can change the summary of this one to reflect the changes done here. Sebastian

I have reproduced this bug with Nightly 48.0a1 (2016-04-14) on Windows 7 , 64 Bit ! This bug's fix is verified with latest Beta Build ID : 20161010144024 User Agent : Mozilla/5.0(Windows NT 6.1; Win64; x64; rv:50.0) Gecko/20100101 Firefox/50.0 [bugday-20161012]

According to comment 24 I mark this as VERIFIED. Sebastian

To be clear, what has changed is that the column headers are now reset when you select another storage. So I changed the summary to reflect that. Though the headers are still shown for empty storages and when selecting a storage type. This is now covered by bug 1291427. Sebastian