FF debugging functions conflicting with can.Map #2069

Closed
julia-allyce opened this Issue Nov 9, 2015 · 8 comments

Comments

Projects
None yet
5 participants
@julia-allyce
Contributor

julia-allyce commented Nov 9, 2015

This is from PR #1877
"FF introduced a debugging function called watch. This meant that if you had a property labeled watch, it would throw an exception."
cc: @tracer99

@julia-allyce julia-allyce added the bug label Nov 9, 2015

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Nov 12, 2015

Contributor

@julia-allyce @tracer99 do we have a property named watch? What's the actual problem?

Contributor

justinbmeyer commented Nov 12, 2015

@julia-allyce @tracer99 do we have a property named watch? What's the actual problem?

@tracer99

This comment has been minimized.

Show comment
Hide comment
@tracer99

tracer99 Nov 12, 2015

Contributor

This was lost in the original ticket.
FF has an internal function called watch.
So if you want to have a map with a property watch, map.attr("watch") will fail.
I solved it by using hasOwnerProperty in the attr function in CanJS. However, @matthewp was concerned that this approach would not handle inherited properties.

Contributor

tracer99 commented Nov 12, 2015

This was lost in the original ticket.
FF has an internal function called watch.
So if you want to have a map with a property watch, map.attr("watch") will fail.
I solved it by using hasOwnerProperty in the attr function in CanJS. However, @matthewp was concerned that this approach would not handle inherited properties.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Nov 12, 2015

Contributor

@tracer99 where in the attr function did you use this? in ___get? https://github.com/canjs/canjs/blob/master/map/map.js#L260

Contributor

justinbmeyer commented Nov 12, 2015

@tracer99 where in the attr function did you use this? in ___get? https://github.com/canjs/canjs/blob/master/map/map.js#L260

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Nov 12, 2015

Contributor

FF has an internal function called watch.

Is this on Object.prototype?

My concern would be more about performance. We'd want to feature-detect this and only add this check for this property, specifically when the feature is detected.

Contributor

justinbmeyer commented Nov 12, 2015

FF has an internal function called watch.

Is this on Object.prototype?

My concern would be more about performance. We'd want to feature-detect this and only add this check for this property, specifically when the feature is detected.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Nov 12, 2015

Contributor

@tracer99 let me know if you'd want to pair on it. I'm available most times. Just hit me up on gitter.

Contributor

justinbmeyer commented Nov 12, 2015

@tracer99 let me know if you'd want to pair on it. I'm available most times. Just hit me up on gitter.

@prashantsharmain prashantsharmain self-assigned this Dec 11, 2015

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Dec 15, 2015

Contributor

@prashantsharmain do you want to pair on it this week?

Contributor

justinbmeyer commented Dec 15, 2015

@prashantsharmain do you want to pair on it this week?

@justinbmeyer justinbmeyer added this to the 2.3.7 milestone Dec 15, 2015

@prashantsharmain

This comment has been minimized.

Show comment
Hide comment
@prashantsharmain

prashantsharmain Dec 15, 2015

Member

@justinbmeyer Sure, how about Thursday (12/17) ? We could set it up at the same time we have the contributor's meeting.

If I manage to find a fix before that, I'll share an update.

Member

prashantsharmain commented Dec 15, 2015

@justinbmeyer Sure, how about Thursday (12/17) ? We could set it up at the same time we have the contributor's meeting.

If I manage to find a fix before that, I'll share an update.

@julia-allyce

This comment has been minimized.

Show comment
Hide comment
@julia-allyce

julia-allyce Dec 15, 2015

Contributor

Also just FYI, the original ticket/PR still exists along with all the code changes. It was only closed not deleted.
You can see the original pr here #1877

Contributor

julia-allyce commented Dec 15, 2015

Also just FYI, the original ticket/PR still exists along with all the code changes. It was only closed not deleted.
You can see the original pr here #1877

@daffl daffl removed this from the 2.3.7 milestone Dec 16, 2015

prashantsharmain added a commit to prashantsharmain/canjs that referenced this issue Dec 20, 2015

Fix #2069 (FF debugging functions conflicting with can.Map)
Gecko exposes methods named "watch" and "unwatch" on Object.prototype. If a can.Map doesn't define these attributes, the ___get method in map.js incorrectly treats these as computed attributes. Fix it by ignoring these properties before invoking the compute function on the attribute.

prashantsharmain added a commit to prashantsharmain/canjs that referenced this issue Dec 28, 2015

Fix #2069 (FF debugging functions conflicting with can.Map)
Gecko exposes methods named "watch" and "unwatch" on Object.prototype. If a can.Map doesn't define these attributes, the ___get method in map.js incorrectly treats these as computed attributes. Fix it by calling the compute function only if it exists on the computed attribute.

@daffl daffl added this to the 2.3.8 milestone Dec 28, 2015

@daffl daffl closed this in #2148 Dec 28, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment