1402485 - Option to remove all session cookies is shown for non-cookie storages

Status: VERIFIED FIXED

(DevTools :: Storage Inspector, enhancement, P3)

Description

[Sebastian Zartner [:sebo]]

The option "Delete All Session Cookies" is not only shown for cookie storages but also for other storages.

Test case:
1. Go to https://www.vanamco.com/2014/11/14/indexeddb-fundamentals-plus-a-indexeddb-example-tutorial/
2. Click the "add 6 predefined todos" button on the page
3. Reload the page
4. Expand the "https://www.vanamco.com" Indexed DB storage until you get to the "todo" item and select it.
5. Right-click the item list.

=> The option "Delete All Session Cookies" is shown.

Sebastian

Comment 1

[Sebastian Zartner [:sebo]]

To fix this, the code that was introduced in https://hg.mozilla.org/mozilla-central/rev/217f398fd59d needs to be adjusted so that the option is only shown for cookie storage lists.

Sebastian

Comment 2

[Sebastian Zartner [:sebo]]

Just to be clear, the option "Delete All Session Cookies" should only be shown when you right-click the entry list (at the right side of the Storage Inspector) of a cookie storage. For all other storage types like e.g. local storages it must not appear.

Sebastian

Comment 3

[Mithilan Sivanesan]

Hey! I am new contributor looking to get started on my first couple bugs.Would it be okay if I got started on this bug?

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to mithilan91 from comment #3)
> Hey! I am new contributor looking to get started on my first couple
> bugs.Would it be okay if I got started on this bug?
Sounds great to me!
Honza

Comment 5

[Abhinav Koppula]

Hi @mithilan91,
Are you working on this issue? Is it fine if I can take this one if you aren't working.

Comment 6

[Mithilan Sivanesan]

Hey, I am still working on it but I was trying to make a push to mozreview for another bug and it said I "cannot submit commits referencing multiple bugs" so thought maybe being assigned to two bugs was the issue. I'am not sure if this is the case but if its, feel free to to take it because I won't be able to push for mozreview.

Comment 7

[Abhinav Koppula]

Hey @mithilan91,
No that's not the reason why you are seeing that message, I guess.
So this is my understanding.
I'm guessing this is how your tree looks like

-- your commit for bug1,bug2
--> commit from mozilla-central

It should ideally be:
-- your commit for bug1
--> commit from mozilla-central
push to mozreview should work now

and

-- your commit for bug2
--> commit from mozilla-central
push to mozreview should work now.

Just have commits related to a single bug on top of commits from mozilla-central and reference only that one bug in the mozreview commit.

Comment 8

[Mithilan Sivanesan]

Thank you for replying! Is there a way to reference a bug when making a commit for the future and is there a way to fix my previous commits so I can make a push to mozreview now?

Comment 9

[Abhinav Koppula]

So assuming your tree is right now in this state,
-- your commit for bug1,bug2
--> commit from mozilla-central

I would run the following
hg strip --keep -r -1
This would bring it to the state you were before creating the commit .i.e it would remove the commit from the log but still keep your changes. Run `hg log` and you can see that your commit is no longer there. Run `hg diff` and see that your changes are there.

Now you can store your changes of Bug2 to some diff file like this:
hg diff file1.js > ~/some/location/bug2.diff

Now you would need to revert your changes of Bug2 since those are not needed for Bug1's commit and since those are stored safely
hg revert --no-backup file1.js 

Now you only have Bug1 related changes
hg commit      (commit msg with Bug1 reference)
hg push review (should work now)

Next again strip one commit
hg strip --keep -r -1
Now you are at mozilla-central commit

import Bug2 changes
hg import ~/some/location/bug2.diff --no-commit
Run hg diff to verify the diff

Create a commit
hg commit      (commit msg with Bug2 reference)
hg push review (should work now)

Comment 10

[Sagar Bharadwaj]

Hello Mithilan,
Are you still working on this? Or would it be okay if I take this up?

Comment 11

[Sagar Bharadwaj]

Attachment: Bug 1402485 - Remove Delete all Session cookies option for non cookie staorage

Borrowed code from onTreePopupShowing into onTablePopupShowing

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

Comment 12

[Sagar Bharadwaj]

Hello,
I have submitted a review request. This is my first time contributing and first time using mercurial as well. 
Apologies if I have not adhered to some workflow or convention.

Comment 13

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

Sorry about the wait... the patch looks good to me so I am running it through try (our automated testing system) to make sure it doesn't have any undesirable side effects.

If it runs through the testing system okay I will land your code for you.

You can see the try run here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e4bd80d5d606

Comment 14

[Sagar Bharadwaj]

Thank you for you review Mike :)

Comment 15

[Sebastian Zartner [:sebo]]

Sorry Sagar that I didn't do the review. I am simply too busy lately. And thank you Mike for taking it over!

Sebastian

Comment 16

[Sagar Bharadwaj]

Hello,
I think the patch can be landed. I cannot add the checkin needed keyword because the bug is not assigned to me.

Comment 17

[Sebastian Zartner [:sebo]]

I've assigned the bug to you now, Sagar. Thank you for the patch!
Mithilan, feel free to pick another one when you have time again.

Sebastian

Comment 18

[Ryan VanderMeulen [:RyanVM]]

I don't see where this ever officially got an r+? Autoland can't push it because of that.

Comment 19

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

Comment on attachment 8936129 [details]
Bug 1402485 - Remove Delete all Session cookies option for non cookie staorage

https://reviewboard.mozilla.org/r/206904/#review215002

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aa7c634fd552
Remove Delete all Session cookies option for non cookie staorage r=miker

Comment 21

[Andreea Pavel [:apavel]]

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

Comment 22

[Sebastian Zartner [:sebo]]

Works fine for me in Firefox 59.0a1 2017-12-22. Thank you for the fix, Sagar!

Sebastian