[CLOSED] Fix JS Quick Edit for projects that have a function named "hasOwnProperty"

[core-ai-bot]

Issue by gruehle
Friday Mar 01, 2013 at 00:42 GMT
Originally opened as adobe/brackets#3004

---

Fixes #3002

If any files in your project define a function named hasOwnProperty, JS Quick Edit would throw an exception. (modernizr.js, which is included in the test files for phantom.js, defines a hasOwnProperty function. This is the cause of bug #3002).

@peterflynn - since you've fixed several bugs in this area, would you mind taking a look?

---

gruehle included the following code: https://github.com/adobe/brackets/pull/3004/commits

[core-ai-bot]

Comment by peterflynn
Monday Mar 18, 2013 at 23:23 GMT

---

I think this is basically plugging a hole in my earlier fix #2816 (sorry!). That patch made a similar []->hasOwnProperty() conversion in StringMatch too (https://github.com/adobe/brackets/pull/2816/files#L3L792) -- seems like this fix should also apply there?

It would also be good to augment the unit tests that #2816 added (https://github.com/adobe/brackets/pull/2816/files#L6R255 and https://github.com/adobe/brackets/pull/2816/files#L6R443 and https://github.com/adobe/brackets/pull/2816/files#L8R634) to cover this bug too.

[core-ai-bot]

Comment by peterflynn
Monday Mar 18, 2013 at 23:26 GMT

---

I'd bet we have similar bugs with some of the other built-in Object properties (constructor, isPrototypeOf, valueOf, etc.)... worth trying to fix those here, or should we just do a spin-off bug?

[Edit: or maybe not... I guess the original #2816 fix should in theory cover all cases except for hasOwnProperty() already]

[core-ai-bot]

Comment by peterflynn
Monday Mar 18, 2013 at 23:29 GMT

---

One other thought: if we're using this hasOwnProperty.apply() trick in a few places, is it worth adding a utility fn for it? (CollectionUtils maybe?)

[core-ai-bot]

Comment by peterflynn
Monday Mar 18, 2013 at 23:30 GMT

---

@gruehle done reviewing

[core-ai-bot]

Comment by gruehle
Friday Mar 22, 2013 at 00:17 GMT

---

@peterflynn - thanks for the review! I've made all your suggested changes. Ready for re-review.

[core-ai-bot]

Comment by TomMalbran
Friday Mar 22, 2013 at 00:22 GMT

---

Hehe, JSHint doesn't like the hasOwnProperty name maybe we can call it hasProperty?

[core-ai-bot]

Comment by gruehle
Friday Mar 22, 2013 at 00:31 GMT

---

Yep :-). This is one case where JSHint complained more than JSLInt. Changed to "hasProperty".

[core-ai-bot]

Comment by TomMalbran
Friday Mar 22, 2013 at 00:36 GMT

---

It kind of makes sense, since we are fixing the problem of a project having a hasOwnProperty function by adding a hasOwnProperty function. Maybe JSLint should complain about it too.

[core-ai-bot]

Comment by peterflynn
Thursday Mar 28, 2013 at 18:58 GMT

---

Looks good!