Skip to content
This repository was archived by the owner on Feb 26, 2024. It is now read-only.

fix: resolve errors with closure #722

Closed
wants to merge 1 commit into from
Closed

Conversation

vikerman
Copy link
Contributor

@vikerman vikerman commented Apr 4, 2017

Also don't patch all "on" properties if a list of properties is being
passed in. Otherwise various global variables in window get clobbered.

@vikerman vikerman requested a review from mhevery April 4, 2017 17:08
Also don't patch all "on" properties if a list of properties is being
passed in. Otherwise various global variables in window get clobbered.
@mhevery
Copy link
Contributor

mhevery commented Apr 10, 2017

Could you get this green?

if (properties) {
for (let i = 0; i < properties.length; i++) {
patchProperty(obj, 'on' + properties[i]);
}
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vikerman , I think we should merge the properties array and prop in obj and then remove duplicate ones, because if we use if/else here, some properties may not be patched.

so I think the logic should be.

 if (properties) {
   for (let i = 0; i < properties.length; i++) {
      const onProp = 'on' + properties[i];
      if (onProperties.filter(op => op === onProp)).length > 0) {
          continue;
      } 
      patchProperty(obj, onProp);
    }
  }

mhevery pushed a commit that referenced this pull request Apr 21, 2017
Also don't patch all "on" properties if a list of properties is being
passed in. Otherwise various global variables in window get clobbered.

Closes #722
mhevery pushed a commit that referenced this pull request Apr 21, 2017
Also don't patch all "on" properties if a list of properties is being
passed in. Otherwise various global variables in window get clobbered.

Closes #722
mhevery pushed a commit that referenced this pull request Apr 21, 2017
Also don't patch all "on" properties if a list of properties is being
passed in. Otherwise various global variables in window get clobbered.

Closes #722
@mhevery mhevery closed this in 51e7ffe Apr 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants