Clear Private Data does not clear JS Console. There sometimes is private data in the js console.

The JS console data is not saved across sessions, why should it matter?

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

use case. names have been changed to protect the guilty. 1) user goes to some seriously nasty sites. 2) wife comes home. 3) user goes to Tools / Clear Private Data... 4) user selects all checkboxes, and hits enter. 5) wife asks to login to webmail to check her mail. 6) webmail site doesn't layout properly 7) wife debugs webpage using js console. 8) user in shitload of trouble. seriously -- if you are suggesting that the user exits out of the application (ie, out of the session) before "Clear Private Data..." is completed successfully, then we should let the user know this via a dialog.

ok, if you're going to try and sneak around someone's back, don't marry someone who smart enough to catch you. b) a better example would be someone buying an engagement ring for their wife, but doesn't want her to know that he's been shopping around.

abstracting the use case we get to the same thing (i was _trying_ to be funny). Currently if you do not exit the browser your "Private Data" continues to exist. There might be other places, but this is the first thing I noticed.

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

This solution is more complete by actually allowing the console service to reset itself. The remaining issue would be whether (and if so how) to notify the console service listeners about the reset. The proposed solution is to not notify them and add a null message before the reset in the relevant places (which also nicely ensures compatibility with extensions relying on the null messages).

For Firefox 2 (aka the Mozilla 1.8 branch), we will need to invent a new interface nsIConsoleService_MOZILLA_1_8_BRANCH that extends from nsIConsoleService on which we can define the reset method. We have a policy in place of not changing interfaces between FF 1.5 and 2.

Comment on attachment 223137 [details] [diff] [review] less hacky solution (extends nsIConsoleService_MOZILLA_1_8_BRANCH) >Index: xpcom/base/nsIConsoleService.idl >+/** >+ * Temporary interface for Gecko 1.8.1. Should the console service not be >+ * improved/replaced for Gecko 1.9 this is be merged with the above. >+ * >+ * @status TEMPORARY >+ */ >+[scriptable, uuid(883472a0-ea9b-11da-8ad9-0800200c9a66)] >+interface nsIConsoleService_MOZILLA_1_8_BRANCH : nsIConsoleService >+{ >+ /** >+ * Clear the message buffer (e.g. for privacy reasons). >+ */ >+ void reset(); > }; Thanks for creating this interface. Please go ahead and prepare a second patch for the trunk that modifies nsIConsoleService directly. We don't want to land any MOZILLA_1_8_BRANCH interfaces on the trunk :) >Index: xpcom/base/nsConsoleService.cpp >+NS_IMETHODIMP >+nsConsoleService::Reset() >+{ >+ /* >+ * Make sure nobody trips into the buffer while it's being reset >+ */ >+ nsAutoLock lock(mLock); >+ >+ mCurrent = 0; >+ mFull = PR_FALSE; >+ >+ /* >+ * Free all messages stored so far and re-initialize the buffer >+ */ >+ for (PRUint32 i = 0; i < mBufferSize && mMessages[i] != nsnull; i++) >+ NS_RELEASE(mMessages[i]); >+ >+ memset(mMessages, 0, mBufferSize * sizeof(nsIConsoleMessage *)); >+ >+ return NS_OK; >+} A couple comments: 1. Maybe you could just loop from 0 to mCurrent instead? 2. NS_RELEASE nulls out its parameter, so the memset is redundant. >+ this.mCService = isupports.QueryInterface(Components.interfaces.nsIConsoleService_MOZILLA_1_8_BRANCH); Since the component implements nsIClassInfo, you should be able to remove the explicit QueryInterface altogether :-) I had to stop reviewing here. Will continue later...

(In reply to comment #11) > 1. Maybe you could just loop from 0 to mCurrent instead? No: when mFull is NS_TRUE the buffer has wrapped around and we'll have to make sure to release all 250 slots (cf. destructor).

(In reply to comment #11) > Since the component implements nsIClassInfo, you should be able to remove > the explicit QueryInterface altogether :-) I prefer to leave that to a clean-up bug.

Comment on attachment 223285 [details] [diff] [review] trunk patch (updated to comments) >Index: toolkit/components/console/content/consoleBindings.xml >+ if (msg.message) >+ this.appendMessage(msg.message); >+ else // observed a null/"clear" message >+ this.clearConsole(); It is generally good for readability to use {}'s whenever coding an else statement. I recommend the following: if (msg.message) { this.appendMessage(msg.message); } else { // observed a null/"clear" message this.clearConsole(); } OK, r=darin I'd like mconnor to also review this change.

Will review this later today.

Comment on attachment 223285 [details] [diff] [review] trunk patch (updated to comments) Looks good for trunk.

mozilla/browser/components/nsBrowserGlue.js 1.20 mozilla/toolkit/components/console/content/consoleBindings.xml 1.12 mozilla/xpcom/base/nsConsoleService.cpp 3.22 mozilla/xpcom/base/nsIConsoleService.idl 3.6

Drivers: This patch extends the console service with a Reset method (on a branch-specific interface), makes the console use it and adds an observer for Clear Private Data which also triggers it. Hardly risky.

Comment on attachment 229998 [details] [diff] [review] branch patch (including fix for bug 345327) a=mconnor on behalf of drivers for checkin to the 1.8 branch

mozilla/toolkit/components/console/content/consoleBindings.xml 1.9.2.3 mozilla/browser/components/nsBrowserGlue.js 1.4.2.12 mozilla/xpcom/base/nsConsoleService.h 3.7.8.1 mozilla/xpcom/base/nsConsoleService.cpp 3.19.8.1 mozilla/xpcom/base/nsIConsoleService.idl 3.5.28.1

Not sure if this is the cause but using build: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060725 BonEcho/2.0b1 ID:2006072518 I no longer have any data in the Error Console but did in the previous build (build prior to this checkin). This occurs in both FF modes (Normal/Safe Mode). The Error Console is "empty" for every single site I go to in both modes. ~B

Also... Shouldn't there be an option to enable/disable this in the configuration for "Clear Private Data" to be consistent? ~B

Gavin: It seems that this one line was somehow garbled during check-in. Could you please correct it?

Bryan, please feel free to file a follow-up bug on the issue you mentioned and make it depend/block this bug, but commenting in this bug is not very productive.

Gah, sorry. To get your patch to apply I needed to replace the instance of MOZILLA_1_8 in the file paths with mozilla, but I wasn't careful enough to verify that the changes I made didn't affect other parts of the patch. I checked in the fix.

(In reply to comment #26) > Gah, sorry. To get your patch to apply I needed to replace the instance of > MOZILLA_1_8 in the file paths with mozilla, but I wasn't careful enough to > verify that the changes I made didn't affect other parts of the patch. > > I checked in the fix. Well after the fix for the fix "Clear Private Data" now does not clear the Error Console in both FF modes (Normal / Safe Mode) using build: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060725 BonEcho/2.0b1 ID:2006072601 ~B

(In reply to comment #27) > Well after the fix for the fix "Clear Private Data" now does not clear the > Error Console Indeed. If LXR now has finally caught up with Gavin's check-in, then it seems that the s/mozilla/MOZILLA_1_8/g issue affects the whole patch (i.e. also nsBrowserGlue.js, nsIConsoleService.idl and nsConsoleService.*). Not sure what the best solution is for this issue. Maybe backing both branch changes out and reapplying the original branch patch with s|MOZILLA_1_8/|mozilla/|g ? Anyway, sorry for these issues. I'll look into making branch patches which apply immediately. BTW: Gavin, maybe you could check-in the patch to bug 345327 as well. It's got implicit r+ through the a+ to the branch patch and is probably annoying the heck out of some trunk testers.

(In reply to comment #28) > Indeed. If LXR now has finally caught up with Gavin's check-in, then it seems > that the s/mozilla/MOZILLA_1_8/g issue affects the whole patch (i.e. also > nsBrowserGlue.js, nsIConsoleService.idl and nsConsoleService.*). Argh! I had already fixed the compiled files, since the change had caused bustage, but I didn't think to check the other files. I've fixed the one occurrence that LXR finds (in nsBrowserGlue), and tested that it does indeed fix the Clear button issue.

I just filed BUG: https://bugzilla.mozilla.org/show_bug.cgi?id=346372 requesting a Clear private data option enhancement. Now that this bug has landed and clears the Error Console every time I "Clear Private Data" I have no way of configuring this in the "Clear Private Data - Select items to be cleared dialog". Many users would expect this to be configurable by way of a seperate "Error Console" checkbox!

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

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

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