171507 - Need a cookie history log

Status: RESOLVED FIXED

(Core :: Networking: Cookies, enhancement, P2)

Description

[Tony Daniel]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.1) Gecko/20020826
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.1) Gecko/20020826

The problem that led to this enhancement request was due to use of
Preferences /Advanced / Scripts & Plugins.  That panel allows users to
grant or deny javascripts the permission to create or read cookies.
When a website that requires cookies fails, you cannot tell whether
the problem was due to the site being on the list of sites that cannot
set cookies, or, as in this case, it was due to a setting in the Scripts
& Plugins panel.  It would be useful to have somewhere to go where you
could see what recent cookie operations were prevented and why.

Another place to address this problem is to have a "Cookie History" in
addition to the URL History.

Reproducible: Always

Steps to Reproduce:

Comment 1

[Stephen P. Morse]

Attachment: implement cookie logging

Comment 2

[Stephen P. Morse]

Attachment: Add yet more logging info

Comment 3

[Mitchell Stoltz (not reading bugmail)]

Comment on attachment 101726 [details] [diff] [review]
Add yet more logging info

I think you should combine LogRejects and LogNotSent into a single function,
since they're nearly identical. Likewise for LogSent and LogAccepts, these two
can be combined. This will eliminate some duplicated code.

Comment 4

[Stephen P. Morse]

Attachment: combining similar routines

Comment 5

[Stephen P. Morse]

Attachment: previous patch was the wrong one, here's the right one.

Comment 6

[Mitchell Stoltz (not reading bugmail)]

Comment on attachment 101786 [details] [diff] [review]
previous patch was the wrong one, here's the right one.

Looks great - thanks for making that change.
r=mstoltz

Comment 7

[Daniel Veditz [:dveditz]]

Comment on attachment 101786 [details] [diff] [review]
previous patch was the wrong one, here's the right one.

>+cookie_LogFailure(PRBool set_cookie, nsIURI * curURL, const char * cookieString, const char * 

The code would have been more readable with an enum, consts or even #defines
for set_cookie values instead of PR_TRUE and PR_FALSE throughout. e.g. at its
simplest
  #define SET_COOKIE PR_TRUE
  #define GET_COOKIE PR_FALSE

sr=dveditz

Comment 8

[Stephen P. Morse]

Attachment: added #defines as suggested in comment 7

Comment 9

[Stephen P. Morse]

Comment on attachment 101955 [details] [diff] [review]
added #defines as suggested in comment 7

bringing reviews forward from previous patch

Comment 10

[Stephen P. Morse]

Fix checked in.  Tony Daniel, thanks for this suggestion.  It has already saved
me considerable time in tracking down the causes of cookie problems in other bug
reports.

Comment 11

[:deanis74]

Does this actually display somewhere in the cookie manager, as requested in the
original report?

Comment 12

[Stephen P. Morse]

No it does not.  It gets written to a file.

I implemented the spirit rather than the letter of the request.  The reporter
presented a problem and described two possible solutions.  I implemented my own
solution to the problem that is more in line with what we do for logging other
diagnostic information.

Comment 13

[Stephen P. Morse]

*** Bug 113091 has been marked as a duplicate of this bug. ***

Comment 14

[:deanis74]

Is PR_LOGGING defined in optimized builds?

Comment 15

[Stephen P. Morse]

> Is PR_LOGGING defined in optimized builds?

Yes, I believe it is.  But I believe it is not defined in Netscape commercial
builds, only in mozilla builds.

Can someone make a definitive statement on this?

Comment 16

[Alec Flett]

generally it is not, unless you use FORCE_PR_LOG, which in this patch is
conditional off of MOZ_LOGGING, which is probably as morse has described. So
more or less it is on for this patch.

I'm pretty sure that there's no way to re-capture the logging back into the
product though.

You might want to take a look at the console service for some ideas about what
to do here. my suggestion would be to make a generic logging service in
xpfe/appshell or xpfe/components, and then make use of it in wallet.

Comment 17

[Jay Davis]

Where in God's name is this log residing?

Comment 18

[benc]

changed summary to match the "spirit of the implmentation". As I understand it,
this feature only works in debug builds, so adding it to the UI would be more
complicated. I'll file a new bug if someone wants.