74816 - Need pref to control dump() output

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Chris McAfee]

We should have a pref that toggles dump() output.
Adding matt, alecf for suggestions

Comment 1

[Alec Flett]

what chris is trying to say is that we should suppress dump() output by default,
but have a hidden/advanced pref which re-enables it.
jband, what do you think?

Comment 2

[timeless]

Can't we offer to dump to js console? We could even have it as a 4th category.  
js console should have copy support by now, and does allow you to filter 
by category.

Comment 3

[John Bandhauer]

Fine with me. This is DOM not JS engine. Reassigning to jst.

Comment 4

[Johnny Stenback (:jst)]

We over-use dump and print out so much crap nowadays that it's just sad, I
really really don't want all that on the console, at all! I can add a pref that
turns it off, that's no big deal, and the pref will be on by default (i.e. no
dump output) and ignored in debug builds so that developers do see the dumps...

Reasonable? Do we need something in the pref UI for this?

Comment 5

[Blake Ross]

Unfortunately this just masks the problem instead of encouraging people to get 
rid of all the useless dumps.  Cc'ing some people who took part in a tangential 
discussion tonight on IRC :-)

Comment 6

[Johnny Stenback (:jst)]

How so?

Comment 7

[Alec Flett]

while it's true we have too much dump() output as it is, this came out of a
discussion between mcafee, matt, and myself, where we wanted an easy way to be
able to spit debug output out in release builds to QA with things like page load
success/failure, timing results, etc, without burdening everyone else with crap
on the screen...then we realized that 99% of people with release builds don't
want crap spit out on their screen anyway.. so putting it behind a pref is
probably good.

It would be even nicer if we had a really easy pref-based logging system that
was accessable from JS.. i.e. if we could say:
smartDump("performance", "it took " + secs + " seconds to load this page\n");

and then only people with "performance" set in some pref would actually see this

Comment 8

[Alec Flett]

I should add that I'm not trying to morph this bug - I still think we should put
dump() output behind a pref.

Comment 9

[Phil Schwartau]

Setting default QA

Comment 10

[Johnny Stenback (:jst)]

Attachment: Proposed fix.

Comment 11

[Eric Pollmann]

r=[e-mail], looks good to me!

Comment 12

[Johnny Stenback (:jst)]

alecf, sr=?

Comment 13

[Christopher Blizzard (:blizzard)]

I think that we should have a pref in there and leave the code enabled for
optimized builds so that we can use dump() even on those builds with it
defaulting to off on optimized builds. You can use a pref to turn it on. Maybe
in the debug pref panel?

Comment 14

[Johnny Stenback (:jst)]

Blizzard, the code is enabled for optimized builds *only* (#ifndef NS_DEBUG) so
in optimized builds you can enable or disable the dump() output with a pref, in
debug builds it's always on, I don't see why we'd ever want dump() output turned
off in debug builds.

Comment 15

[Ben Goodger (use ben at mozilla dot org for email)]

sr=[e-mail]

Comment 16

[Christopher Blizzard (:blizzard)]

Look at me!  I'm a dork!

a=blizzard on behalf of drivers for 0.9.2

Comment 17

[Gervase Markham [:gerv]]

Whoa there. Just saw this in the SR list. Isn't it a bit of a perf. problem to 
be checking a pref like this all the time? I'm hazy on the details, but couldn't 
we either 
a) make it so that changing the pref on a running Moz had no effect (i.e. use a 
static member variable or something and only check the pref at startup) or 
b) use pref listeners and callbacks and such?

Gerv

Comment 18

[Johnny Stenback (:jst)]

While getting a service and checkin a pref is not free, it's not *that*
expensive, and IMO it's not close to enough to worry about, we don't call
window.dump() *that* often, even if we do abuse it.

Fix checked in, prove me wrong wrt the performace concerns and I'll fix it, but
I really don't think it's worth worrying about.

Comment 19

[Christopher Blizzard (:blizzard)]

Plus, we don't call dump() all that often.