All users were logged out of Bugzilla on October 13th, 2018

PAC: removed console logging

VERIFIED FIXED

Status

()

VERIFIED FIXED
18 years ago
7 years ago

People

(Reporter: blizzard, Assigned: blizzard)

Tracking

Trunk
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Assignee)

Description

18 years ago
The PAC module dumps a lot of debug messages to the console.  Here's a patch to
remove them:

Index: nsProxyAutoConfig.js
===================================================================
RCS file: /cvsroot/mozilla/netwerk/base/src/nsProxyAutoConfig.js,v
retrieving revision 1.9
diff -u -r1.9 nsProxyAutoConfig.js
--- nsProxyAutoConfig.js        2001/04/26 14:06:22     1.9
+++ nsProxyAutoConfig.js        2001/06/11 21:40:27
@@ -35,11 +35,6 @@
 const nsIIOService = Components.interfaces['nsIIOService'];
 const nsIDNSService = Components.interfaces.nsIDNSService;
 
-function debug(msg)
-{
-    dump("\nPAC:" + msg + "\n\n");
-}
-
 // implementor of nsIProxyAutoConfig
 function nsProxyAutoConfig() {};
 
@@ -70,7 +65,6 @@
         var uri = url.QueryInterface(Components.interfaces.nsIURI);
         // Call the original function-
         var proxy = LocalFindProxyForURL(uri.spec, uri.host);
-        debug("Proxy = " + proxy);
         if (proxy == "DIRECT") {
             host.value = null;
             type.value = "direct";
@@ -89,7 +83,6 @@
     },
 
     LoadPACFromURL: function(uri, ioService) {
-        debug("Loading PAC from " + uri.spec);
         this.done = false;
         var channel = ioService.newChannelFromURI(uri);
         pacURL = uri.spec;
(Assignee)

Comment 1

18 years ago
I'm looking for rubber stamps on this really obvious change.
Assignee: neeti → blizzard

Comment 2

18 years ago
rs=tor

Comment 4

18 years ago
a= asa@mozilla.org for checkin to the trunk.
(on behalf of drivers)
(Assignee)

Comment 5

18 years ago
*** Bug 80885 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 6

18 years ago
Checked in.
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
(Assignee)

Comment 7

18 years ago
I ended up removing two other dump() statements in the registration bits of the
module, too.

Comment 8

18 years ago
Was this necessary? Given the number of bugs in PAC right now, and the
difficulty in verifying the behavior, this is going to make qa difficult.
If every other module did this, qa using printfs would also be difficult because
you'd be seeing tons of other junk from everyone else.

Comment 10

18 years ago
changed summary
Summary: proxy autoconfig module needs to shut the hell up → PAC: removed console loggin

Comment 11

18 years ago
Other modules probably also have other methods of troubleshooting. Without this,
PAC users are going to be flying blind if they are trying to figure out where
the connections are going, esp on Mac (which doesn't have netstat) and somewhat
on Windows (because netstat sucks and doesn't show every open connection).

The point is, even if the output was too verbose, there should have been *SOME*
discussion rather than slamming the code in. Heck, there are almost a dozen PAC
bugs that actually block reall functionality, and nobody is fixing any of those,
but two coders have run out and tried to "fix" this problem.
printing to the console in release builds causes a number of problems.  I don't
know the bug number offhand that describe these problems, but they can include
filling up the disk or memory with the text printed.  So our policy is basically
that we shouldn't print in release builds.

Comment 13

18 years ago
Where is that conversation happening? If people are going to remove features
that other people are using, the solution would be ideally implemented before
the removal occurs.

Right now, when people need to analyze their PAC file problems, I tell them to
downgrade to Mozilla 0.9 so they can see the output of conosole..

Comment 14

18 years ago
all: what is the right way to get the information? benc has a real problem.

Why can't dumb() be smart enough to not print on teh console in release builds?

Comment 15

18 years ago
We might be able to solve this if we implement alert() bug 86846. We could
document that people should put in break points.

david: which bug documents console filling up disk and memory?
Summary: PAC: removed console loggin → PAC: removed console logging

Comment 16

16 years ago
VERIFIED: Win and Linux, mozilla for the last year.

I never use console logging on Mac, but this was fixed for Mac OS classic, which
is not a primary test platform for me anymore.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.