1139794 - Remove __noSuchMethod__ from the console object

Status: RESOLVED FIXED

(DevTools :: Console, defect)

Description

[Jan de Mooij [:jandem]]

The console object currently uses __noSuchMethod__ to silently accept console.foo() etc. __noSuchMethod__ is a non-standard SpiderMonkey feature that we really want to remove. It also doesn't work reliably with the JITs.

Bug 629607 was filed to replace console's __noSuchMethod__ with a proxy, not sure why that bug was marked fixed because it didn't actually fix this.

Anyway, do we really need this silently-accept-anything behavior? I checked Chrome and Safari and they don't do this either. It seems like we could get a list of 'console' methods in Chrome, Safari and IE, and simply add no-ops for those or something. Would that be acceptable? What am I missing?

To me it also seems confusing to silently ignore calls. For instance, if I use console.warning("Foo") nothing shows up because I need console.warn instead. In this case, throwing a good old TypeError seems preferable.

Using a proxy could also work, but proxies will likely slow down all calls and I don't know if that's really worth it for this feature.

I'd be happy to work on this bug, once we decide how we're going to fix it.

Comment 1

[Jan de Mooij [:jandem]]

(In reply to Jan de Mooij [:jandem] from comment #0)
> It seems like we could get
> a list of 'console' methods in Chrome, Safari and IE, and simply add no-ops
> for those or something.

Also, if we do this we can maybe show a nice warning: "Sorry, we don't support console.foo() (yet)!", which seems more useful than completely ignoring the call.

Comment 2

[Andrea Marchesini [:baku]]

I agree with you, and I'm happy to review your patches if you want to work on this bug.
BTW, we really need to standardize the console API.

Comment 3

[Jan de Mooij [:jandem]]

FWIW, in Chrome 42 I get:

> Object.getOwnPropertyNames(console.__proto__.__proto__)
["debug", "error", "info", "log", "warn", "dir", "dirxml", "table", "trace", "assert", "count", "markTimeline", "profile", "profileEnd", "time", "timeEnd", "timeStamp", "timeline", "timelineEnd", "group", "groupCollapsed", "groupEnd", "clear", "constructor"]

In Nightly:

> Object.getOwnPropertyNames(console.__proto__).toSource()
["log", "info", "warn", "error", "exception", "debug", "table", "trace", "dir", "group", "groupCollapsed", "groupEnd", "time", "timeEnd", "profile", "profileEnd", "assert", "count", "__noSuchMethod__", "constructor"]

Properties in Chrome but not in Nightly:

["dirxml", "markTimeline", "timeStamp", "timeline", "timelineEnd", "clear"]

console.clear() clears the console, that seems pretty useful. Maybe we should add that one too?

The others all return undefined and markTimeline, timeline and timelineEnd print a deprecation warning.

Comment 4

[Jan de Mooij [:jandem]]

(In reply to Jan de Mooij [:jandem] from comment #3)
> console.clear() clears the console, that seems pretty useful. Maybe we
> should add that one too?

I found bug 659625, nevermind.

Comment 5

[Jan de Mooij [:jandem]]

In Safari 8 but not in Nightly: ["clear", "dirxml", "timeStamp"]

So that's just a subset of the extra methods Chrome has. Will check IE later today.

Comment 6

[Panos Astithas (he/him) [:past] (please ni?)]

console.timeStamp is bug 922221. console.dirxml is bug 922212. console.timeline/timelineEnd were discussed in bug 1050502.

See also the following doc which is the best and most recent attempt at standardizing this that I know of:

https://github.com/DeveloperToolsWG/console-object/blob/master/api.md

Comment 7

[Jan de Mooij [:jandem]]

Attachment: WIP

This patch adds the no-op methods mentioned before.

Comment 8

[Jan de Mooij [:jandem]]

Attachment: Patch

This works and fixes the test failures I saw on Try.

There are some similar uses of __noSuchMethod__ related to the console object in the addon-sdk, but I can fix these later.

Comment 9

[Andrea Marchesini [:baku]]

Comment on attachment 8573320 [details] [diff] [review]
Patch

Review of attachment 8573320 [details] [diff] [review]:
-----------------------------------------------------------------

looks good to me!

Comment 10

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

This was added so that console methods that other browsers support but we don't yet don't break functionality when testing quickly in firefox. Additionally, other browsers have indicated they plan on implementing this as well. Saying it is nonstandard paints it in a worse light than it really is because the whole console object is nonstandard.

https://github.com/DeveloperToolsWG/console-object#ideas

I think we should maintain the never-throw behavior, but it doesn't matter to me whether it is implemented with __noSuchMethod__ or with a proxy. A warning message would be a nice addition, but it isn't an either or situation.

Comment 11

[Jan de Mooij [:jandem]]

(In reply to Nick Fitzgerald [:fitzgen] from comment #10)
> Additionally, other browsers have indicated they plan on implementing this
> as well.

Do you know if there are any WebKit/Blink/Chromium bugs or mailing list threads for this?

> I think we should maintain the never-throw behavior, but it doesn't matter
> to me whether it is implemented with __noSuchMethod__ or with a proxy.

Unfortunately it's not easy to do this because:

(1) __noSuchMethod__ is non-standard and buggy, and we really want to remove it from SpiderMonkey. It's a lot of complexity to keep in the engine.

(2) Proxies are much slower and more heavyweight. This may be fine for a feature everybody else supports, but considering no other browser (and Firebug) has this I'm not sure it's really all that useful/necessary.

Anyways, I won't land the patch in this bug if you're not OK with it, but I'm not sure how we can move forward without it.

Comment 12

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Sounds like Chrome may not be doing it for a while: https://twitter.com/fitzgen/status/573578731534487552

I guess I'm ok with removing it, but I don't feel great about it.

Comment 13

[Jeff Walden [:Waldo]]

If you find other browser vendors thinking about implementing this, I'm happy to argue any and all browser vendors into the ground to not have everyone implement this thoroughly-broken API.  People tried to add something like it to ES6, but the attempt failed -- because the ECAMScript meta-object protocol just doesn't work the way this API thinks it works.

Ultimately, __noSuchMethod__ in the console object is a horrible hackaround for the failure to standardize the console object.  When we preserve the hackaround, we actively hinder bringing the console object into the standardized, open web, instead requiring that everyone reverse-engineer what the major browsers have done.  Put bluntly, that violates the Mozilla mission.

Comment 14

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/95fe77826c1f

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/95fe77826c1f