1291427 - Table headers are not removed when selecting an empty storage

Status: VERIFIED FIXED

(DevTools :: Storage Inspector, defect, P2)

Description

[Sebastian Zartner [:sebo]]

As of bug 1264582:

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.

You'd expect that the table headers are hidden when there are no entries.

Sebastian

Comment 1

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Filter on Brobdingnagian.

Comment 2

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Filter on HOTFROG.

Comment 3

[Sebastian Zartner [:sebo]]

I assume that should be easy to fix. So, adding 'good-first-bug'.

Sebastian

Comment 4

[Sebastian Zartner [:sebo]]

Mike, I've added you as mentor, if that's ok. I may tackle this bug myself at some point if no one else steps up.

Sebastian

Comment 5

[Luna Phipps-Costin]

Attachment: Picture of STR step 2

Comment 6

[Luna Phipps-Costin]

Attachment: Picture of STR step 3

Comment 7

[Luna Phipps-Costin]

Not seeing this on Nightly 57.0a1 64-bit on linux.

Comment 8

[Sebastian Zartner [:sebo]]

Your second screenshot shows the error, the headers of the 'Cookies' table ('Name', 'Domain', 'Path', etc.) are shown at the top of the panel.

Sebastian

Comment 9

[Luna Phipps-Costin]

Oh, sorry, only on my second look-through do I see the word "headers." I'm not sure that I agree they shouldn't show, but I'll take a look at figuring out a patch if that's the consensus.

Comment 10

[Luna Phipps-Costin]

Also of note: The + button still appears when the panel is first opened. Should I open a new bug for this?

Comment 11

[Luna Phipps-Costin]

Why do we allow clicking on the host at all? If it never displays any data, what's the point? I think the better solution would be to make clicking the host either a) select the first subitem or b) expand/collapse the element. Thoughts?

Anyway, I was trying to figure out how to hide the tree headers, and I can't seem to figure it out. I think it needs to be added in ui.js at line 606 [1], surrounded in `if (howManyNodesIn <= 1) { ... }`. I have tried `this.table.clear()`.

[1] https://searchfox.org/mozilla-central/source/devtools/client/storage/ui.js#606

Comment 12

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

(In reply to Christopher Phonis-Phine from comment #11)
> Why do we allow clicking on the host at all? If it never displays any data,
> what's the point? I think the better solution would be to make clicking the
> host either a) select the first subitem or b) expand/collapse the element.
> Thoughts?
> 
> Anyway, I was trying to figure out how to hide the tree headers, and I can't
> seem to figure it out. I think it needs to be added in ui.js at line 606
> [1], surrounded in `if (howManyNodesIn <= 1) { ... }`. I have tried
> `this.table.clear()`.
> 
> [1]
> https://searchfox.org/mozilla-central/source/devtools/client/storage/ui.
> js#606

If there is no data then clicking on the host should really show a panel saying "There is no data for the selected host."

The host is selectable because in the future we plan on showing data relating to the storage type when it is clicked e.g. when an indexedDB host is selected we can show the number of databases, tables etc. along with their sizes and remaining capacities.

Comment 13

[Sebastian Zartner [:sebo]]

(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #12)
> (In reply to Christopher Phonis-Phine from comment #11)
> > Why do we allow clicking on the host at all? If it never displays any data,
> > what's the point? I think the better solution would be to make clicking the
> > host either a) select the first subitem or b) expand/collapse the element.
> > Thoughts?
> 
> If there is no data then clicking on the host should really show a panel
> saying "There is no data for the selected host."
> 
> The host is selectable because in the future we plan on showing data
> relating to the storage type when it is clicked e.g. when an indexedDB host
> is selected we can show the number of databases, tables etc. along with
> their sizes and remaining capacities.

See also bug 1102716 and bug 1309824.

Sebastian

Comment 14

[Luna Phipps-Costin]

I thing bug 1102716 makes the most sense, and either way when either of those are resolved this bug becomes irrelevant.

Comment 15

[Parth]

Hey is it okay if i take care of this bug? Im a student in an open-source class and im looking to start off in this community

Comment 16

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

(In reply to Parth from comment #15)
> Hey is it okay if i take care of this bug? I'm a student in an open-source
> class and im looking to start off in this community

Sure, just ping me if you need any help.

I have assigned you to this bug.

Comment 17

[Brian]

Hi, this bug has already been assigned, but it's been 5 months and I'm not sure if it's been fixed. I am new here and would like to work on this!

Comment 18

[Sebastian Zartner [:sebo]]

As Parth was not active on Bugzilla for half a year, I think it is fine to assign this bug to you, Brian.
Thank you for picking this up!

Sebastian

Comment 19

[Patrick Brosset <:pbro>]

This bug has not been updated in the last 6 months. Resetting the assignee field.
Please, feel free to pick it up again and add a comment outlining your plans for it if you do still intend to work on it.
This is just trying to clean our backlog of bugs and make bugs available for people.

Comment 20

[Sowmya Ganesh Gupta]

hi, I would like to work on this bug. I am new to this code base and I don't know where to start. Can someone help me to work on this bug?

Comment 21

[avi.mathur.engg+github]

Hello, I would also like to help with this. If some one can advice where to look for.

Comment 22

[avi.mathur.engg+github]

Mike I have fixed this bug, please advice me how to proceed ?

Comment 23

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

(In reply to avi.mathur.engg+github from comment #22)

Mike I have fixed this bug, please advice me how to proceed ?

Hi, so you have fixed the bug and have a patch ready for review? That's great!

Please work through the instructions here to get set up to request a code review.
https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html

In a notshell ():

• Go to https://phabricator.services.mozilla.com/
• Click Log in
• Register and log in.
• Install arc and generate a certificate using arc install-certificate
• Install moz-phab
• Ensure your patch commit message looks like this: Bug 1291427 - Table headers are not removed when selecting an empty storage r?mratcliffe
• moz-phab submit <revisionNumber>

If you need any more help you are best to ping me in the #general channel of https://devtools-html.slack.com/

Comment 24

[avi.mathur.engg+github]

Attachment: Bug 1291427 - Remove table headers when selecting an empty storage type r?mratcliffe

Before this change clicking on Storage Type use to show table headers from previous selection. Now clicking on Storage Type will reset table headers also in case empty table headers will get cleared.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c8a4f2586fc0
Table headers are not removed when selecting an empty storage r=miker

Comment 26

[Oana Pop-Rus]

Backed out changeset c8a4f2586fc0 (bug 1291427) for devtools on browser_storage_cookies_edit.js on a CLOSED TREE

Backout: https://hg.mozilla.org/integration/autoland/rev/d3d719ba069153ae29a99f60ec537133345c7ff0

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=superseded%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&selectedJob=232670539&searchStr=linux%2Cx64%2Cdebug%2Cmochitests%2Cwith%2Ce10s%2Ctest-linux64%2Fdebug-mochitest-devtools-chrome-e10s-3%2Cm-e10s%28dt3%29&revision=c8a4f2586fc01b1e13303f55eaa096c306674416

Failure log:[task 2019-03-08T15:11:58.750Z] 15:11:58 INFO - TEST-PASS | devtools/client/storage/test/browser_storage_cookies_edit.js | value column has the right value for newTest3{9d414cc5-8319-0a04-0586-c0a6ae01670a}test1.example.org{9d414cc5-8319-0a04-0586-c0a6ae01670a}/ -
[task 2019-03-08T15:11:58.751Z] 15:11:58 INFO - Typing true
[task 2019-03-08T15:11:58.752Z] 15:11:58 INFO - Pressing KEY_Enter
[task 2019-03-08T15:11:58.753Z] 15:11:58 INFO - Validating results... waiting for ROW_EDIT event.
[task 2019-03-08T15:11:58.755Z] 15:11:58 INFO - Buffered messages finished
[task 2019-03-08T15:11:58.756Z] 15:11:58 INFO - TEST-UNEXPECTED-FAIL | devtools/client/storage/test/browser_storage_cookies_edit.js | Test timed out -
[task 2019-03-08T15:11:58.759Z] 15:11:58 INFO - GECKO(3030) | ++DOMWINDOW == 16 (0x7f69eec5e400) [pid = 3030] [serial = 78] [outer = 0x7f69ef50c3e0]
[task 2019-03-08T15:11:58.760Z] 15:11:58 INFO - GECKO(3030) | console.warn: "Error while detaching the thread front: 'detach' request packet to 'server1.conn10.child1/context22' can't be sent as the connection is closed."
[task 2019-03-08T15:11:59.192Z] 15:11:59 INFO - Removing tab.
[task 2019-03-08T15:11:59.193Z] 15:11:59 INFO - Waiting for event: 'TabClose' on [object XULElement].
[task 2019-03-08T15:11:59.254Z] 15:11:59 INFO - Got event: 'TabClose' on [object XULElement].
[task 2019-03-08T15:11:59.290Z] 15:11:59 INFO - Tab removed and finished closing
[task 2019-03-08T15:11:59.372Z] 15:11:59 INFO - GECKO(3030) | MEMORY STAT | vsize 2049MB | residentFast 323MB | heapAllocated 103MB
[task 2019-03-08T15:11:59.376Z] 15:11:59 INFO - TEST-OK | devtools/client/storage/test/browser_storage_cookies_edit.js | took 91507ms
[task 2019-03-08T15:11:59.392Z] 15:11:59 INFO - GECKO(3030) | ++DOCSHELL 0x7fcebe53c800 == 1 [pid = 3187] [id = {d3742d67-5344-49e6-b7f1-f059b8a7eb9b}]
[task 2019-03-08T15:11:59.396Z] 15:11:59 INFO - GECKO(3030) | ++DOMWINDOW == 1 (0x7fcebe9d9d40) [pid = 3187] [serial = 17] [outer = (nil)]
[task 2019-03-08T15:11:59.460Z] 15:11:59 INFO - GECKO(3030) | ++DOMWINDOW == 2 (0x7fcebe583400) [pid = 3187] [serial = 18] [outer = 0x7fcebe9d9d40]
[task 2019-03-08T15:11:59.488Z] 15:11:59 INFO - checking window state

Comment 27

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

This didn't stay landed for very long, never mind, it happens.

Can you go to Phabricator and address my comment?

Comment 28

[avi.mathur.engg+github]

Done. Is this okay ?

Comment 29

[avi.mathur.engg+github]

Hello Mike, I have made those changes. Please review.
Tried contacting you over IRC however I can't seem to find you. That's why commenting here.
The slack channels you have provided only allows mozilla.com accounts let me know if I can create one to join the slack channel.

Comment 30

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=10f387eeb0dcb68f43e7e441a3045c23e40295a5

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ae39470fc216
Remove table headers when selecting an empty storage type r=miker

Comment 32

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/ae39470fc216

Comment 33

[Sebastian Zartner [:sebo]]

Looks good to me on Nightly 68.0a1 (2019-03-22). Thank you for fixing this!

Sebastian

Comment 34

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

I have reproduced this issue using Firefox 51.0a1 Win 10 x64.
I can confirm this issue is fixed, I verified using Firefox 68.0b7 on Win 10 x64, macOS 10.13.6 and Ubuntu 18.04 x64.