Bug in JSObjectProxy

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Bug in JSObjectProxy

NorbertHartl
I've found a bug in JSObjectProxy. I think the line

<if(obj[jsSelector] != undefined) {return smalltalk.send(obj, jsSelector, arguments)}>.

is not correct. By using [] the property is evaluated. So it is false for the case if the property is missing and the case where the property is existing but undefined. For the message send it makes a difference if the selector is not found or return value is nil. So the line should look like

<if(jsSelector in obj) {return smalltalk.send(obj, jsSelector, arguments)}>.

"in" checks for the property plus looking the prototype chain which should be the desired behavior.

Norbert


Reply | Threaded
Open this post in threaded view
|

Re: Bug in JSObjectProxy

Nicolas Petton

Norbert,

Thanks for the bug report (and fix!).

Would you have a minimal testcase where it fails?
Also, could you send me a pull request on github with the fix (with a
test it would be really cool)?

Thanks,
Nico

Norbert Hartl <[hidden email]> writes:

> I've found a bug in JSObjectProxy. I think the line
>
> <if(obj[jsSelector] != undefined) {return smalltalk.send(obj, jsSelector, arguments)}>.
>
> is not correct. By using [] the property is evaluated. So it is false for the case if the property is missing and the case where the property is existing but undefined. For the message send it makes a difference if the selector is not found or return value is nil. So the line should look like
>
> <if(jsSelector in obj) {return smalltalk.send(obj, jsSelector, arguments)}>.
>
> "in" checks for the property plus looking the prototype chain which should be the desired behavior.
>
> Norbert
>
>

--
Nicolas Petton
http://nicolas-petton.fr
Reply | Threaded
Open this post in threaded view
|

Re: Bug in JSObjectProxy

NorbertHartl

Am 09.01.2013 um 16:43 schrieb Nicolas Petton <[hidden email]>:


Norbert,

Thanks for the bug report (and fix!).

Would you have a minimal testcase where it fails?
Also, could you send me a pull request on github with the fix (with a
test it would be really cool)?

https://github.com/NicolasPetton/amber/pull/290

Done. Well, I hope everything is ok. I'm still not familiar with git.

Norbert

Norbert Hartl <[hidden email]> writes:

I've found a bug in JSObjectProxy. I think the line

<if(obj[jsSelector] != undefined) {return smalltalk.send(obj, jsSelector, arguments)}>.

is not correct. By using [] the property is evaluated. So it is false for the case if the property is missing and the case where the property is existing but undefined. For the message send it makes a difference if the selector is not found or return value is nil. So the line should look like

<if(jsSelector in obj) {return smalltalk.send(obj, jsSelector, arguments)}>.

"in" checks for the property plus looking the prototype chain which should be the desired behavior.

Norbert



--
Nicolas Petton
http://nicolas-petton.fr

Reply | Threaded
Open this post in threaded view
|

Re: Bug in JSObjectProxy

Nicolas Petton

Thanks, I'll take a look and merge ASAP :)

Nico

Norbert Hartl <[hidden email]> writes:

> Am 09.01.2013 um 16:43 schrieb Nicolas Petton <[hidden email]>:
>
>>
>> Norbert,
>>
>> Thanks for the bug report (and fix!).
>>
>> Would you have a minimal testcase where it fails?
>> Also, could you send me a pull request on github with the fix (with a
>> test it would be really cool)?
>>
> https://github.com/NicolasPetton/amber/pull/290
>
> Done. Well, I hope everything is ok. I'm still not familiar with git.
>
> Norbert
>
>> Norbert Hartl <[hidden email]> writes:
>>
>>> I've found a bug in JSObjectProxy. I think the line
>>>
>>> <if(obj[jsSelector] != undefined) {return smalltalk.send(obj, jsSelector, arguments)}>.
>>>
>>> is not correct. By using [] the property is evaluated. So it is false for the case if the property is missing and the case where the property is existing but undefined. For the message send it makes a difference if the selector is not found or return value is nil. So the line should look like
>>>
>>> <if(jsSelector in obj) {return smalltalk.send(obj, jsSelector, arguments)}>.
>>>
>>> "in" checks for the property plus looking the prototype chain which should be the desired behavior.
>>>
>>> Norbert
>>>
>>>
>>
>> --
>> Nicolas Petton
>> http://nicolas-petton.fr
>

--
Nicolas Petton
http://nicolas-petton.fr
Reply | Threaded
Open this post in threaded view
|

Re: Bug in JSObjectProxy

NorbertHartl

Am 10.01.2013 um 10:58 schrieb Nicolas Petton <[hidden email]>:

>
> Thanks, I'll take a look and merge ASAP :)
>
Great. I just saw that the travis build failed. I don't understand the error message that is being displayed. It just does not really sound as the problem was introduced by me.

Norbert

> Norbert Hartl <[hidden email]> writes:
>
>> Am 09.01.2013 um 16:43 schrieb Nicolas Petton <[hidden email]>:
>>
>>>
>>> Norbert,
>>>
>>> Thanks for the bug report (and fix!).
>>>
>>> Would you have a minimal testcase where it fails?
>>> Also, could you send me a pull request on github with the fix (with a
>>> test it would be really cool)?
>>>
>> https://github.com/NicolasPetton/amber/pull/290
>>
>> Done. Well, I hope everything is ok. I'm still not familiar with git.
>>
>> Norbert
>>
>>> Norbert Hartl <[hidden email]> writes:
>>>
>>>> I've found a bug in JSObjectProxy. I think the line
>>>>
>>>> <if(obj[jsSelector] != undefined) {return smalltalk.send(obj, jsSelector, arguments)}>.
>>>>
>>>> is not correct. By using [] the property is evaluated. So it is false for the case if the property is missing and the case where the property is existing but undefined. For the message send it makes a difference if the selector is not found or return value is nil. So the line should look like
>>>>
>>>> <if(jsSelector in obj) {return smalltalk.send(obj, jsSelector, arguments)}>.
>>>>
>>>> "in" checks for the property plus looking the prototype chain which should be the desired behavior.
>>>>
>>>> Norbert
>>>>
>>>>
>>>
>>> --
>>> Nicolas Petton
>>> http://nicolas-petton.fr
>>
>
> --
> Nicolas Petton
> http://nicolas-petton.fr

Reply | Threaded
Open this post in threaded view
|

Re: Bug in JSObjectProxy

Nicolas Petton

Indeed. I'll merge into a separate branch first to see how travis
reacts.

Nico

Norbert Hartl <[hidden email]> writes:

> Am 10.01.2013 um 10:58 schrieb Nicolas Petton <[hidden email]>:
>
>>
>> Thanks, I'll take a look and merge ASAP :)
>>
> Great. I just saw that the travis build failed. I don't understand the error message that is being displayed. It just does not really sound as the problem was introduced by me.
>
> Norbert
>
>> Norbert Hartl <[hidden email]> writes:
>>
>>> Am 09.01.2013 um 16:43 schrieb Nicolas Petton <[hidden email]>:
>>>
>>>>
>>>> Norbert,
>>>>
>>>> Thanks for the bug report (and fix!).
>>>>
>>>> Would you have a minimal testcase where it fails?
>>>> Also, could you send me a pull request on github with the fix (with a
>>>> test it would be really cool)?
>>>>
>>> https://github.com/NicolasPetton/amber/pull/290
>>>
>>> Done. Well, I hope everything is ok. I'm still not familiar with git.
>>>
>>> Norbert
>>>
>>>> Norbert Hartl <[hidden email]> writes:
>>>>
>>>>> I've found a bug in JSObjectProxy. I think the line
>>>>>
>>>>> <if(obj[jsSelector] != undefined) {return smalltalk.send(obj, jsSelector, arguments)}>.
>>>>>
>>>>> is not correct. By using [] the property is evaluated. So it is false for the case if the property is missing and the case where the property is existing but undefined. For the message send it makes a difference if the selector is not found or return value is nil. So the line should look like
>>>>>
>>>>> <if(jsSelector in obj) {return smalltalk.send(obj, jsSelector, arguments)}>.
>>>>>
>>>>> "in" checks for the property plus looking the prototype chain which should be the desired behavior.
>>>>>
>>>>> Norbert
>>>>>
>>>>>
>>>>
>>>> --
>>>> Nicolas Petton
>>>> http://nicolas-petton.fr
>>>
>>
>> --
>> Nicolas Petton
>> http://nicolas-petton.fr
>

--
Nicolas Petton
http://nicolas-petton.fr
Reply | Threaded
Open this post in threaded view
|

Re: Bug in JSObjectProxy

Manfred Kröhnert
Hi,

This seems to be a Travis issue.
The tests run fine on my computer and don't exceed a time limit of more than one minute.

So I would vote for including the patch with the option of reverting in case it causes trouble.

Best,
Manfred


On Thu, Jan 10, 2013 at 11:07 AM, Nicolas Petton <[hidden email]> wrote:

Indeed. I'll merge into a separate branch first to see how travis
reacts.

Nico

Norbert Hartl <[hidden email]> writes:

> Am 10.01.2013 um 10:58 schrieb Nicolas Petton <[hidden email]>:
>
>>
>> Thanks, I'll take a look and merge ASAP :)
>>
> Great. I just saw that the travis build failed. I don't understand the error message that is being displayed. It just does not really sound as the problem was introduced by me.
>
> Norbert
>
>> Norbert Hartl <[hidden email]> writes:
>>
>>> Am 09.01.2013 um 16:43 schrieb Nicolas Petton <[hidden email]>:
>>>
>>>>
>>>> Norbert,
>>>>
>>>> Thanks for the bug report (and fix!).
>>>>
>>>> Would you have a minimal testcase where it fails?
>>>> Also, could you send me a pull request on github with the fix (with a
>>>> test it would be really cool)?
>>>>
>>> https://github.com/NicolasPetton/amber/pull/290
>>>
>>> Done. Well, I hope everything is ok. I'm still not familiar with git.
>>>
>>> Norbert
>>>
>>>> Norbert Hartl <[hidden email]> writes:
>>>>
>>>>> I've found a bug in JSObjectProxy. I think the line
>>>>>
>>>>> <if(obj[jsSelector] != undefined) {return smalltalk.send(obj, jsSelector, arguments)}>.
>>>>>
>>>>> is not correct. By using [] the property is evaluated. So it is false for the case if the property is missing and the case where the property is existing but undefined. For the message send it makes a difference if the selector is not found or return value is nil. So the line should look like
>>>>>
>>>>> <if(jsSelector in obj) {return smalltalk.send(obj, jsSelector, arguments)}>.
>>>>>
>>>>> "in" checks for the property plus looking the prototype chain which should be the desired behavior.
>>>>>
>>>>> Norbert
>>>>>
>>>>>
>>>>
>>>> --
>>>> Nicolas Petton
>>>> http://nicolas-petton.fr
>>>
>>
>> --
>> Nicolas Petton
>> http://nicolas-petton.fr
>

--
Nicolas Petton
http://nicolas-petton.fr

Reply | Threaded
Open this post in threaded view
|

Re: Bug in JSObjectProxy

Nicolas Petton
Agreed.
I'll do it ASAP.

Nico

Manfred Kröhnert <[hidden email]> writes:

> Hi,
>
> This seems to be a Travis issue.
> The tests run fine on my computer and don't exceed a time limit of more
> than one minute.
>
> So I would vote for including the patch with the option of reverting in
> case it causes trouble.
>
> Best,
> Manfred
>
>
> On Thu, Jan 10, 2013 at 11:07 AM, Nicolas Petton
> <[hidden email]>wrote:
>
>>
>> Indeed. I'll merge into a separate branch first to see how travis
>> reacts.
>>
>> Nico
>>
>> Norbert Hartl <[hidden email]> writes:
>>
>> > Am 10.01.2013 um 10:58 schrieb Nicolas Petton <[hidden email]
>> >:
>> >
>> >>
>> >> Thanks, I'll take a look and merge ASAP :)
>> >>
>> > Great. I just saw that the travis build failed. I don't understand the
>> error message that is being displayed. It just does not really sound as the
>> problem was introduced by me.
>> >
>> > Norbert
>> >
>> >> Norbert Hartl <[hidden email]> writes:
>> >>
>> >>> Am 09.01.2013 um 16:43 schrieb Nicolas Petton <
>> [hidden email]>:
>> >>>
>> >>>>
>> >>>> Norbert,
>> >>>>
>> >>>> Thanks for the bug report (and fix!).
>> >>>>
>> >>>> Would you have a minimal testcase where it fails?
>> >>>> Also, could you send me a pull request on github with the fix (with a
>> >>>> test it would be really cool)?
>> >>>>
>> >>> https://github.com/NicolasPetton/amber/pull/290
>> >>>
>> >>> Done. Well, I hope everything is ok. I'm still not familiar with git.
>> >>>
>> >>> Norbert
>> >>>
>> >>>> Norbert Hartl <[hidden email]> writes:
>> >>>>
>> >>>>> I've found a bug in JSObjectProxy. I think the line
>> >>>>>
>> >>>>> <if(obj[jsSelector] != undefined) {return smalltalk.send(obj,
>> jsSelector, arguments)}>.
>> >>>>>
>> >>>>> is not correct. By using [] the property is evaluated. So it is
>> false for the case if the property is missing and the case where the
>> property is existing but undefined. For the message send it makes a
>> difference if the selector is not found or return value is nil. So the line
>> should look like
>> >>>>>
>> >>>>> <if(jsSelector in obj) {return smalltalk.send(obj, jsSelector,
>> arguments)}>.
>> >>>>>
>> >>>>> "in" checks for the property plus looking the prototype chain which
>> should be the desired behavior.
>> >>>>>
>> >>>>> Norbert
>> >>>>>
>> >>>>>
>> >>>>
>> >>>> --
>> >>>> Nicolas Petton
>> >>>> http://nicolas-petton.fr
>> >>>
>> >>
>> >> --
>> >> Nicolas Petton
>> >> http://nicolas-petton.fr
>> >
>>
>> --
>> Nicolas Petton
>> http://nicolas-petton.fr
>>

--
Nicolas Petton
http://nicolas-petton.fr