Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#9907 closed enhancement (fixed)

Dojo doesn't flag which built-in console.* methods exist

Reported by: cb1kenobi Owned by: cb1kenobi
Priority: high Milestone: 1.4
Component: Core Version: 1.3.2
Keywords: debug firebug Cc: James Burke, Mike Wilcox
Blocked By: Blocking:

Description

Dojo will automatically create a window.console object if it doesn't exist, then add stub functions for any methods of the Firebug API that the browser doesn't implement.

There is no way of knowing which console methods are stub functions and thus a flag needs to be added.

I need the flag so when a custom dojo debugger wants to do a full implementation of a console function, it knows which ones the browser implemented and which it doesn't.

Also, since Opera doesn't support console.log(), I substituted Opera's postError() function instead.

Attachments (2)

newConsoleCheck.patch (1.6 KB) - added by cb1kenobi 10 years ago.
console-methods.png (40.4 KB) - added by cb1kenobi 10 years ago.

Download all attachments as: .zip

Change History (12)

Changed 10 years ago by cb1kenobi

Attachment: newConsoleCheck.patch added

comment:1 Changed 10 years ago by cb1kenobi

Component: GeneralCore

Changed 10 years ago by cb1kenobi

Attachment: console-methods.png added

comment:2 Changed 10 years ago by cb1kenobi

Added chart of supported console methods by various browsers.

comment:3 Changed 10 years ago by Mike Wilcox

Actually you should be able to check for stubs. I didn't test this cross browser, just in FF:

console._log = function(){}
ts = function(f){
    return f.toString().replace(/\n|\s/g, "");
}
console.log(ts(console._log)=="function(){}")

But the reason I really piped in is that I wanted to renew my argument that we only stub the top methods: log, info, warn, debug, error. It seems reasonable to expect that one should not leave the other types in a expect it to work in other browsers.

Great chart BTW. Didn't know FB had groupCollapsed - that actually is useful (albeit lengthy)

comment:4 Changed 10 years ago by cb1kenobi

Well, there's 2 different stub functions: one that is just function(){} and another that redirects the function to console.log(). I thought of doing string comparisons, but felt that more hacky than just setting a flag.

Got a new idea!

What if we just add a djConfig.debugModule (http://bugs.dojotoolkit.org/ticket/9905) and remove all console.* junk from bootstrap.js including the thisloadFirebugConsole?()?

So, console.* is not touched and when isDebug=true, we load whatever dojo.config.debugModule is set to and then that debug module can do whatever it wants to with console.*, logging, etc.

This means you will not be able to count on any/all of the Firebug API functions being defined. But that's OK, because I'm thinking we can define a dojo.debug interface that mimics Firebug's API and debuggers must implement the interface.

So, in hostenv_browser.js, we could have

if(dojo.config.isDebug){
    dojo.require(dojo.config.debugModule);
}else{
    dojo.require("dojo.debug.stub");
}

dojo.debug.stub would be an empty implementation of the dojo.debug interface. My debugger would implement that interface too. One catch is dojo.debug.stub would need to be included in a base build of dojo.js, but it's kind of a trade off since we're removing the console.* from bootstrap.

We need to define the dojo.debug API. This should include your logging mechanism along with the assert(), count(), etc from Firebug Lite.

Maybe instead of having people get a logger for dojo and one for dijit, we just have separate loggers for each top-level namespace.

comment:5 Changed 10 years ago by Mike Wilcox

Cc: James Burke added

I agree. This is also something I proposed on the mailing list. We get our custom debugger and the base gets a savings of quite a few bytes.

There are two scenarios we have to account for though: Backwards compatibility and some sort of notification. Back-comapt works fine if the user is deving against the trunk or a full release, but not if they are using Base. I think those who dev just against Base tend to be higher level devs and can accept some changes, but we need to somehow tell them. Maybe:

try{
dojo.require("dojo.debug.stub");
}catch(e){  !djConfig.suppressConsoleMessage && console.warn("console methods are not squelched. Use dojo.debug.stub to do so or add djConfig.suppressConsoleMessage to remove this message" ); }

What do you think? And James, what do you think, since you were the one who raised this issue?

comment:6 Changed 10 years ago by James Burke

Unfortunately we have to keep the console stubbing in. I wanted to remove it, but that did not pan out after the dojo-contributors discussion.

It bothers me a bit to use postError in Opera for these calls, even for a log call. I would prefer not to do that.

As for detecting a stub, I am fine with a one-line patch that just adds the following after we define the function for console[tcn]:

console[tcn]._fake = true;

but I would leave the rest of the code as-is.

comment:7 Changed 10 years ago by James Burke

Milestone: 1.4future
Owner: changed from James Burke to cb1kenobi

cb1kenobi, if you want to put in the change for this:

console[tcn]._fake = true;

and test that it works for you, feel free. But we need to keep the console stubbing in. Marking this as future, but if feel free to put it in for 1.4 if you have the time.

comment:8 Changed 10 years ago by cb1kenobi

I tested adding this and it works for my purposes. It's too bad Firebug Lite recreates it's own console object, but that will get fixed the future and this enhancement is enough to allow other debuggers to work correctly.

I'm going to commit this 1 liner and close this bad boy out.

comment:9 Changed 10 years ago by cb1kenobi

Resolution: fixed
Status: newclosed

(In [20382]) Added a flag for which console methods are stubs created by the bootstrap. fixes #9907 !strict

comment:10 Changed 10 years ago by bill

Milestone: future1.4
Note: See TracTickets for help on using tickets.