crash due to a change to native JavaScript Array object

[tforgeardParticipant]

Hi,
In our application, we make some additions to native JavaScript objects.
For example, we add some functions to the prototype of ‘Array’:

Array.prototype.contains = function () { ... };

After quite a long time I realized that this makes some of your for loops crash.
If I add a ‘hasOwnProperty’ test at the beginning of these loops, the bug is fixed…

I encounter two major issues when using jqxChart:
1: additional ‘stop’ tags are added to the lineargradient:
<stop offset="undefined%" style="stop-color:#NANNANNAN"></stop>

2: a crash occurs in ‘a.extend._getLabelsSettings’. The error is ‘Uncaught TypeError: undefined is not a function’.
the call stack is:
a.extend._getLabelsSettings
a.extend._showLabel
a.extend._animColumns
(anonymous function)
a.extend._runAnimation
(anonymous function)

To simply reproduce these bugs, add a function to the prototype of ‘Array’ and rune any of your chart examples.

Thank you to take these issues into consideration.

Best regards,
Alain DEVICQ

[Peter StoevKeymaster]

Hi Alain DEVICQ,

Extending Native JavaScript array will probably break not only the Chart and usually people do not do such kind of extensions as they’re considered as bad programming practice. The good approach is to make a Wrapper. I am sorry, but we do not think that this is an issue in our Chart.

Best Regards,
Peter Stoev

jQWidgets Team
http://www.jqwidgets.com

[tforgeardParticipant]

Peter,

I am a bit surprised by your answer.
A simple solution is to add test within the loop with ‘hasOwnProperty’ which is considered as a good practice in JavaScript programming (every for (prop in obj) loop should use this test. cf. “Removed Link”

“It is wise to program defensively by using the hasOwnProperty method to distinguish the true members of the object:…”

Adding this test won’t have any impact on the normal behavior of the control and none of your users will see any difference.
We use many of your components and the charts are the only ones to be impacted. Honestly, it’s worth making this small change, don’t you think?

Regards,

Alain

[Peter StoevKeymaster]

Hi Alain DEVICQ,

We will consider whether to make or not such changes in the future. In general, I don’t think that adding “patch” because of native array object extension is good.

Best Regards,
Peter Stoev

jQWidgets Team
http://www.jqwidgets.com

[Author]

Posts