fix(core): fix #1108, window.onerror should have (message, source, lineno, colno, error) signiture (#1109)

Author: JiaLiPassion

file-size-limit.json

@@ -3,7 +3,7 @@
     {
       "path": "dist/zone.min.js",
       "checkTarget": true,
-      "limit": 41600
+      "limit": 42000
     }
   ]
 }

lib/browser/property-descriptor.ts

@@ -10,7 +10,7 @@
  * @suppress {globalThis}
  */
 
-import {isBrowser, isMix, isNode, ObjectDefineProperty, ObjectGetOwnPropertyDescriptor, ObjectGetPrototypeOf, patchClass, patchOnProperties, wrapWithCurrentZone, zoneSymbol} from '../common/utils';
+import {isBrowser, isIE, isMix, isNode, ObjectDefineProperty, ObjectGetOwnPropertyDescriptor, ObjectGetPrototypeOf, patchClass, patchOnProperties, wrapWithCurrentZone, zoneSymbol} from '../common/utils';
 
 import * as webSocketPatch from './websocket';
 
@@ -241,7 +241,7 @@ export interface IgnoreProperty {
 
 function filterProperties(
     target: any, onProperties: string[], ignoreProperties: IgnoreProperty[]): string[] {
-  if (!ignoreProperties) {
+  if (!ignoreProperties || ignoreProperties.length === 0) {
     return onProperties;
   }
 
@@ -276,10 +276,13 @@ export function propertyDescriptorPatch(api: _ZonePrivate, _global: any) {
     // for browsers that we can patch the descriptor:  Chrome & Firefox
     if (isBrowser) {
       const internalWindow: any = window;
+      const ignoreErrorProperties =
+          isIE ? [{target: internalWindow, ignoreProperties: ['error']}] : [];
       // in IE/Edge, onProp not exist in window object, but in WindowPrototype
       // so we need to pass WindowPrototype to check onProp exist or not
       patchFilteredProperties(
-          internalWindow, eventNames.concat(['messageerror']), ignoreProperties,
+          internalWindow, eventNames.concat(['messageerror']),
+          ignoreProperties ? ignoreProperties.concat(ignoreErrorProperties) : ignoreProperties,
           ObjectGetPrototypeOf(internalWindow));
       patchFilteredProperties(Document.prototype, eventNames, ignoreProperties);
 

lib/common/utils.ts

@@ -135,11 +135,26 @@ const wrapFn = function(event: Event) {
   }
   const target = this || event.target || _global;
   const listener = target[eventNameSymbol];
-  let result = listener && listener.apply(this, arguments);
-
-  if (result != undefined && !result) {
-    event.preventDefault();
+  let result;
+  if (isBrowser && target === internalWindow && event.type === 'error') {
+    // window.onerror have different signiture
+    // https://developer.mozilla.org/en-US/docs/Web/API/GlobalEventHandlers/onerror#window.onerror
+    // and onerror callback will prevent default when callback return true
+    const errorEvent: ErrorEvent = event as any;
+    result = listener &&
+        listener.call(
+            this, errorEvent.message, errorEvent.filename, errorEvent.lineno, errorEvent.colno,
+            errorEvent.error);
+    if (result === true) {
+      event.preventDefault();
+    }
+  } else {
+    result = listener && listener.apply(this, arguments);
+    if (result != undefined && !result) {
+      event.preventDefault();
+    }
   }
+
   return result;
 };
 
@@ -475,6 +490,17 @@ export function attachOriginToPatched(patched: Function, original: any) {
 let isDetectedIEOrEdge = false;
 let ieOrEdge = false;
 
+export function isIE() {
+  try {
+    const ua = internalWindow.navigator.userAgent;
+    if (ua.indexOf('MSIE ') !== -1 || ua.indexOf('Trident/') !== -1) {
+      return true;
+    }
+  } catch (error) {
+  }
+  return false;
+}
+
 export function isIEOrEdge() {
   if (isDetectedIEOrEdge) {
     return ieOrEdge;

test/browser/browser.spec.ts

@@ -9,7 +9,7 @@
 import {patchFilteredProperties} from '../../lib/browser/property-descriptor';
 import {patchEventTarget} from '../../lib/common/events';
 import {isBrowser, isIEOrEdge, isMix, zoneSymbol} from '../../lib/common/utils';
-import {getIEVersion, ifEnvSupports, ifEnvSupportsWithDone, isEdge} from '../test-util';
+import {getEdgeVersion, getIEVersion, ifEnvSupports, ifEnvSupportsWithDone, isEdge} from '../test-util';
 
 import Spy = jasmine.Spy;
 declare const global: any;
@@ -190,7 +190,7 @@ describe('Zone', function() {
               'onvrdisplayactivate', 'onvrdisplayblur', 'onvrdisplayconnect',
               'onvrdisplaydeactivate', 'onvrdisplaydisconnect', 'onvrdisplayfocus',
               'onvrdisplaypointerrestricted', 'onvrdisplaypointerunrestricted',
-              'onorientationchange'
+              'onorientationchange', 'onerror'
             ]);
           });
 
@@ -390,6 +390,24 @@ describe('Zone', function() {
                };
                expect(testFn).not.toThrow();
              }));
+
+          it('window.onerror callback signiture should be (message, source, lineno, colno, error)',
+             ifEnvSupportsWithDone(canPatchOnProperty(window, 'onerror'), function(done: DoneFn) {
+               let testError = new Error('testError');
+               window.onerror = function(
+                   message: any, source?: string, lineno?: number, colno?: number, error?: any) {
+                 expect(message).toContain('testError');
+                 if (getEdgeVersion() !== 14) {
+                   // Edge 14, error will be undefined.
+                   expect(error).toBe(testError);
+                 }
+                 setTimeout(done);
+                 return true;
+               };
+               setTimeout(() => {
+                 throw testError;
+               }, 100);
+             }));
         }));
 
     describe('eventListener hooks', function() {

test/test-util.ts

@@ -123,3 +123,12 @@ export function isEdge() {
   const userAgent = navigator.userAgent.toLowerCase();
   return userAgent.indexOf('edge') !== -1;
 }
+
+export function getEdgeVersion() {
+  const ua = navigator.userAgent.toLowerCase();
+  const edge = ua.indexOf('edge/');
+  if (edge === -1) {
+    return -1;
+  }
+  return parseInt(ua.substring(edge + 5, ua.indexOf('.', edge)), 10);
+}