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

Commit

Permalink
fix(core): fix #1108, window.onerror should have (message, source, li…
Browse files Browse the repository at this point in the history
…neno, colno, error) signiture (#1109)
  • Loading branch information
JiaLiPassion authored and mhevery committed Jul 17, 2018
1 parent 875086f commit 49e0548
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 10 deletions.
2 changes: 1 addition & 1 deletion file-size-limit.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
{
"path": "dist/zone.min.js",
"checkTarget": true,
"limit": 41600
"limit": 42000
}
]
}
9 changes: 6 additions & 3 deletions lib/browser/property-descriptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);

Expand Down
34 changes: 30 additions & 4 deletions lib/common/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down Expand Up @@ -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;
Expand Down
22 changes: 20 additions & 2 deletions test/browser/browser.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -190,7 +190,7 @@ describe('Zone', function() {
'onvrdisplayactivate', 'onvrdisplayblur', 'onvrdisplayconnect',
'onvrdisplaydeactivate', 'onvrdisplaydisconnect', 'onvrdisplayfocus',
'onvrdisplaypointerrestricted', 'onvrdisplaypointerunrestricted',
'onorientationchange'
'onorientationchange', 'onerror'
]);
});

Expand Down Expand Up @@ -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() {
Expand Down
9 changes: 9 additions & 0 deletions test/test-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

0 comments on commit 49e0548

Please sign in to comment.