Florian Scholz (Open Web Docs) Reporter
	Description • 8 years ago

Once we support the SameSite attribute for cookies (bug 795346), we should add a column for this flag in the Storage Inspector.

	Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe] Assignee
	Comment 2 • 8 years ago

If you want to apply the patch you will need to apply the rebased dependency first.

	Patrick Brosset <:pbro>
	Comment 3 • 8 years ago
mozreview-review

Comment on attachment 8816278 [details]
Bug 1298370 - Add SameSite attribute to Cookie Inspector r+pbrosset

https://reviewboard.mozilla.org/r/97066/#review99094

Sorry for the delay here. The code changes are really simple. It's nice to see how extensible the storage panel is.
I have just one comment about defining the values in the properties file. I don't think they should be there, or they risk being translated, and that might not make sense for DevTools users.
Worth pinging :flod if you're not sure.

::: devtools/client/locales/en-US/storage.properties:48
(Diff revision 1)
> +storage.cookie.sameSite.lax=Lax
> +storage.cookie.sameSite.strict=Strict
> +storage.cookie.sameSite.unset=Unset

Are these ever supposed to be translated? If I'm not mistaken these are special values to the sameSite property. So they are words that people will use whatever their spoken language may be, they won't change.
So maybe they need to be `const` inside a Js file instead?

	Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe] Assignee
	Comment 4 • 8 years ago

(In reply to Patrick Brosset <:pbro> from comment #3)
> Comment on attachment 8816278 [details]
> Bug 1298370 - Add SameSite attribute to Cookie Inspector
> 
> https://reviewboard.mozilla.org/r/97066/#review99094
> 
> Sorry for the delay here. The code changes are really simple. It's nice to
> see how extensible the storage panel is.

Yes, it is becoming very easy to make changes.

> I have just one comment about defining the values in the properties file. I
> don't think they should be there, or they risk being translated, and that
> might not make sense for DevTools users.
> Worth pinging :flod if you're not sure.
> 
> ::: devtools/client/locales/en-US/storage.properties:48
> (Diff revision 1)
> > +storage.cookie.sameSite.lax=Lax
> > +storage.cookie.sameSite.strict=Strict
> > +storage.cookie.sameSite.unset=Unset
> 
> Are these ever supposed to be translated? If I'm not mistaken these are
> special values to the sameSite property. So they are words that people will
> use whatever their spoken language may be, they won't change.
> So maybe they need to be `const` inside a Js file instead?

You are right, these should never be translated so I will remove them from the properties file and add appropriate comments.

	Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe] Assignee
	Comment 6 • 7 years ago

Filter on Brobdingnagian.

	Patrick Brosset <:pbro>
	Comment 8 • 7 years ago
mozreview-review

Comment on attachment 8932528 [details]
Bug 1298370 - Add SameSite attribute to Cookie Inspector

https://reviewboard.mozilla.org/r/203576/#review209298

	Pulsebot
	Comment 9 • 7 years ago

Pushed by mratcliffe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ff9b8f883b0c
Add SameSite attribute to Cookie Inspector r=pbro

	Bogdan Tara[:bogdan_tara | bogdant]
	Comment 10 • 7 years ago
bugherder

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

	Francesco Lodolo [:flod]
	Comment 11 • 7 years ago

Any reason to spell it "sameSite", where I can only find it spelled as SameSite (uppercase S)?
https://hg.mozilla.org/mozilla-central/diff/ff9b8f883b0c/devtools/client/locales/en-US/storage.properties

	Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe] Assignee
	Comment 12 • 7 years ago

(In reply to Francesco Lodolo [:flod] from comment #11)
> Any reason to spell it "sameSite", where I can only find it spelled as
> SameSite (uppercase S)?
> https://hg.mozilla.org/mozilla-central/diff/ff9b8f883b0c/devtools/client/
> locales/en-US/storage.properties

We go by the name of the JavaScript property itself and the sameSite property is accessible via cookie.sameSite whereas cookie.SameSite would be undefined.

You can see the nsICookie2 IDL at https://searchfox.org/mozilla-central/source/netwerk/cookie/nsICookie2.idl

	Francesco Lodolo [:flod]
	Comment 13 • 7 years ago

https://searchfox.org/mozilla-central/source/netwerk/cookie/nsICookie2.idl#72-74

sameSite is spelled like that only when it's mentioned explicitly as an attribute, but it becomes SameSite in the middle of a sentence (e.g. "the SameSite attribute is not present"). Or in https://tools.ietf.org/html/draft-west-first-party-cookies-07#section-4.1

Since this is a label in the UI, I would expect the same treatment. 

Having said that, thanks for the explanation and confirming it's not a typo, that's what I wanted to clarify.

	Chris Mills [:cmills]
	Comment 14 • 6 years ago

I've documented this:

Added a bullet to the cookies section of the Storage Inspector page:
https://developer.mozilla.org/en-US/docs/Tools/Storage_Inspector#Cookies

Added a note to the Fx59 rel notes:
https://developer.mozilla.org/en-US/Firefox/Releases/59#Changes_for_web_developers

Let me know if this is OK. Thanks!

Also, one more question - the hostonly and secure columns don't appear to be there any more — when we these removed?

	Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe] Assignee
	Comment 15 • 6 years ago

(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #14)
> I've documented this:
> 
> Added a bullet to the cookies section of the Storage Inspector page:
> https://developer.mozilla.org/en-US/docs/Tools/Storage_Inspector#Cookies
> 
> Added a note to the Fx59 rel notes:
> https://developer.mozilla.org/en-US/Firefox/Releases/
> 59#Changes_for_web_developers
> 
> Let me know if this is OK. Thanks!
> 

Yes, great job.

> Also, one more question - the hostonly and secure columns don't appear to be
> there any more — when we these removed?

They are still there... right-click on a table header to show these columns that are hidden by default.

At the moment column settings are not stored so you have to show them each time, which is annoying. I am planning on fixing that at some point.

	Chris Mills [:cmills]
	Comment 16 • 6 years ago

(In reply to Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe] from comment #15)
> They are still there... right-click on a table header to show these columns
> that are hidden by default.
> 
> At the moment column settings are not stored so you have to show them each
> time, which is annoying. I am planning on fixing that at some point.

Ah, thanks for letting me know Mike. I've added a note to

https://developer.mozilla.org/en-US/docs/Tools/Storage_Inspector#Cookies

to make this clear.