fix: resolve errors with closure (#722)

vikerman · mhevery · commit 51e7ffe9c3f5 · 2017-04-21T13:23:31.000-07:00
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
diff --git a/lib/browser/property-descriptor.ts b/lib/browser/property-descriptor.ts
@@ -25,8 +25,8 @@ export function propertyDescriptorPatch(_global: any) {
     if (isBrowser) {
       patchOnProperties(window, eventNames);
       patchOnProperties(Document.prototype, eventNames);
-      if (typeof SVGElement !== 'undefined') {
-        patchOnProperties(SVGElement.prototype, eventNames);
+      if (typeof(<any>window)['SVGElement'] !== 'undefined') {
+        patchOnProperties((<any>window)['SVGElement'].prototype, eventNames);
       }
       patchOnProperties(HTMLElement.prototype, eventNames);
     }
diff --git a/lib/common/utils.ts b/lib/common/utils.ts
@@ -15,6 +15,7 @@
 declare const WorkerGlobalScope: any;
 
 export const zoneSymbol: (name: string) => string = (n) => `__zone_symbol__${n}`;
+const VALUE = zoneSymbol('value');
 const _global: any =
     typeof window === 'object' && window || typeof self === 'object' && self || global;
 
@@ -99,6 +100,12 @@ export function patchProperty(obj: any, prop: string) {
       target.removeEventListener(eventName, target[_prop]);
     }
 
+    if (typeof fn === 'string') {
+      const src: string = fn;
+      fn = new Function(src);
+      fn[VALUE] = src;
+    }
+
     if (typeof fn === 'function') {
       const wrapFn = function(event: Event) {
         let result;
@@ -147,26 +154,28 @@ export function patchProperty(obj: any, prop: string) {
         }
       }
     }
-    return target[_prop] || null;
+    const value = target[_prop] || null;
+    return value && value.hasOwnProperty(VALUE) ? value[value] : value;
   };
 
   Object.defineProperty(obj, prop, desc);
 }
 
 export function patchOnProperties(obj: any, properties: string[]) {
-  const onProperties = [];
-  for (const prop in obj) {
-    if (prop.substr(0, 2) == 'on') {
-      onProperties.push(prop);
-    }
-  }
-  for (let j = 0; j < onProperties.length; j++) {
-    patchProperty(obj, onProperties[j]);
-  }
   if (properties) {
     for (let i = 0; i < properties.length; i++) {
       patchProperty(obj, 'on' + properties[i]);
     }
+  } else {
+    const onProperties = [];
+    for (const prop in obj) {
+      if (prop.substr(0, 2) == 'on') {
+        onProperties.push(prop);
+      }
+    }
+    for (let j = 0; j < onProperties.length; j++) {
+      patchProperty(obj, onProperties[j]);
+    }
   }
 }
 
diff --git a/test/browser/browser.spec.ts b/test/browser/browser.spec.ts
@@ -530,7 +530,7 @@ describe('Zone', function() {
 
         zone.run(function() {
           button.setAttribute('onclick', 'return');
-          expect(button.onclick).not.toBe(null);
+          expect(button.onclick).not.toBe('return');
         });
       });
 

Original file line number	Diff line number	Diff line change
`@@ -25,8 +25,8 @@ export function propertyDescriptorPatch(_global: any) {`
`25`	`25`	`if (isBrowser) {`
`26`	`26`	`patchOnProperties(window, eventNames);`
`27`	`27`	`patchOnProperties(Document.prototype, eventNames);`
`28`		`- if (typeof SVGElement !== 'undefined') {`
`29`		`- patchOnProperties(SVGElement.prototype, eventNames);`
	`28`	`+ if (typeof(<any>window)['SVGElement'] !== 'undefined') {`
	`29`	`+ patchOnProperties((<any>window)['SVGElement'].prototype, eventNames);`
`30`	`30`	`}`
`31`	`31`	`patchOnProperties(HTMLElement.prototype, eventNames);`
`32`	`32`	`}`