Last Comment Bug 564577 - __noSuchMethod__ no longer invoked for defined non-function properties
: __noSuchMethod__ no longer invoked for defined non-function properties
Status: RESOLVED FIXED
[fixed-in-tracemonkey]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: ---
Assigned To: Matthew Draper
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks:
  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:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
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 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:  
nsm

Bisecting suggests the change occurred in bug 420399
Comment 1 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 JSClass.call gets treated the same way as a primitive... but that's really a separate matter.
Comment 2 Matthew Draper 2010-05-09 04:11:51 PDT
Created attachment 444301 [details] [diff] [review]
potential tests
Comment 3 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 Matthew Draper 2010-11-16 22:56:34 PST
Created attachment 491109 [details] [diff] [review]
updated patch
Comment 5 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 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 Phil Ringnalda (:philor) 2011-04-08 23:08:36 PDT
tryservered in http://tbpl.mozilla.org/?tree=MozillaTry&rev=653854347eef
Comment 8 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 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 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 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 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 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 Chris Leary [:cdleary] (not checking bugmail) 2011-05-02 15:59:35 PDT
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/72a5969b0c29

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