Last Comment Bug 564577 - __noSuchMethod__ no longer invoked for defined non-function properties
: __noSuchMethod__ no longer invoked for defined non-function properties
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Linux
-- normal (vote)
: ---
Assigned To: Matthew Draper
: Jason Orendorff [:jorendorff]
Depends on:
  Show dependency treegraph
Reported: 2010-05-07 20:58 PDT by Matthew Draper
Modified: 2011-05-02 15:59 PDT (History)
5 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

naive patch (1.29 KB, patch)
2010-05-08 03:49 PDT, Matthew Draper
igor: review+
Details | Diff | Splinter Review
potential tests (2.48 KB, patch)
2010-05-09 04:11 PDT, Matthew Draper
igor: review+
Details | Diff | Splinter Review
updated patch (1.42 KB, patch)
2010-11-16 22:56 PST, Matthew Draper
igor: review+
Details | Diff | Splinter Review
unrotted, folded, and exported (3.62 KB, patch)
2011-04-08 23:13 PDT, Phil Ringnalda (:philor)
philringnalda: review-
Details | Diff | Splinter Review
fully unrotted (5.43 KB, patch)
2011-04-21 12:21 PDT, Matthew Draper
igor: review+
Details | Diff | Splinter Review

Description User image Matthew Draper 2010-05-07 20:58:46 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-au) AppleWebKit/531.2+ (KHTML, like Gecko) Safari/531.2+ Debian/squeeze/sid () Epiphany/2.30.2
Build Identifier: ca591da19a26

Trying to call a property that was set to a non-function value used to invoke __noSuchMethod__, but now it just throws "is not a function". Brendan's comments in bug 224956, comment 15 suggest the previous behaviour was a conscious choice.

Reproducible: Always

Steps to Reproduce:
js> var o = { a: 'xx', __noSuchMethod__: function() { return 'nsm'; } };
js> o.a()
Actual Results:  
typein:2: TypeError: o.a is not a function

Expected Results:  

Bisecting suggests the change occurred in bug 420399
Comment 1 User image Matthew Draper 2010-05-08 03:49:55 PDT
Created attachment 444249 [details] [diff] [review]
naive patch

Seems to restore the previous behaviour, in my testing: __noSuchMethod__ is called if the property is undefined, null, a number, or a string -- but not if it's an object or a regexp. Or a function. :)

I actually wonder whether js_OnUnknownMethod should get a chance in js_Call, so any object without a gets treated the same way as a primitive... but that's really a separate matter.
Comment 2 User image Matthew Draper 2010-05-09 04:11:51 PDT
Created attachment 444301 [details] [diff] [review]
potential tests
Comment 3 User image Igor Bukanov 2010-07-15 05:17:01 PDT
Comment on attachment 444249 [details] [diff] [review]
naive patch

This should restore the old behavior.
Comment 4 User image Matthew Draper 2010-11-16 22:56:34 PST
Created attachment 491109 [details] [diff] [review]
updated patch
Comment 5 User image Jonathan Watt [:jwatt] 2010-12-04 07:42:36 PST
I'm searching for patches with checkin-needed to check in at the same time as my patches. This bug should not have that keyword set since it doesn't have approval and isn't a blocker, so the keyword is just creating noise and wasting other people's time. Please remove it and - assuming it's there to keep this bug on someone's radar - use your own whiteboard flags for that purpose.
Comment 6 User image Phil Ringnalda (:philor) 2011-04-08 22:55:03 PDT
Brendan, Igor: any reason why this patch (which should have been going into TM anyway, and thus shouldn't have gotten yelled at for not having the approval it would have only needed if it was headed into mozilla-central) shouldn't land?
Comment 7 User image Phil Ringnalda (:philor) 2011-04-08 23:08:36 PDT
tryservered in
Comment 8 User image Phil Ringnalda (:philor) 2011-04-08 23:13:54 PDT
Created attachment 524801 [details] [diff] [review]
unrotted, folded, and exported

Just a tiny bit of bitrot in the context, and I folded them into a single patch so we're less likely to fail to push one, and hg exported them so we're less likely to wind up gumming up your name/email/commit message.
Comment 9 User image Phil Ringnalda (:philor) 2011-04-09 09:34:14 PDT
Comment on attachment 524801 [details] [diff] [review]
unrotted, folded, and exported

Could have answered my own question, just by waiting for tryserver results: according to the jsreftests, "o.fff is not a function item 6."
Comment 10 User image Jonathan Watt [:jwatt] 2011-04-11 11:01:13 PDT
Matthew, someone just pointed out to me that comment 5 came across as sounding curt and a bit rude to them, and in retrospect I have to agree. More to the point I failed to notice that you're a new contributor and shouldn't have been expected to know the subtleties of checkin-needed in use at the time. Apologies for the flyby comment. Had I realized you were just getting started I'd have made a much more friendly and helpful comment. I hope my carelessness doesn't put you off, since your involvement in Mozilla is certainly very welcome. :)
Comment 11 User image Matthew Draper 2011-04-21 12:21:22 PDT
Created attachment 527616 [details] [diff] [review]
fully unrotted

Addressed new instances of HAS_NO_SUCH_METHOD in methodjit/, and updated test to reflect regular expressions not being callable (#582717; the error mentioned in comment 9).
Comment 12 User image Jonathan Watt [:jwatt] 2011-04-26 10:40:08 PDT
Comment on attachment 527616 [details] [diff] [review]
fully unrotted

I'm guessing this needs review so as not to fall off the radar. Please cancel the review request if I'm wrong.
Comment 13 User image Paul Biggar 2011-04-26 10:44:14 PDT
Comment on attachment 527616 [details] [diff] [review]
fully unrotted

This probably needs to be igor.
Comment 15 User image Chris Leary [:cdleary] (not checking bugmail) 2011-05-02 15:59:35 PDT
cdleary-bot mozilla-central merge info:

Note You need to log in before you can comment on or make changes to this bug.