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

Commit e598310

Browse files
JiaLiPassionmhevery
JiaLiPassion
authored and
mhevery
committed
fix(patch): fix #746, check desc get is null and only patch window.resize additionally (#747)
* fix(patch): fix #746, need to patch all on properties without duplicate * fix(patch): should check originalDesc.get value is null * fix(patch): revert to original version * fix(patch): revert to original version * fix(patch): patch resize of window * fix(test): refactor test cases for patch on properties * fix(patch): add test case for first time access no descriptor on property
1 parent 7b43b51 commit e598310

File tree

4 files changed

+105
-98
lines changed

4 files changed

+105
-98
lines changed

Diff for: ‎lib/browser/property-descriptor.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export function propertyDescriptorPatch(_global: any) {
2323
if (canPatchViaPropertyDescriptor()) {
2424
// for browsers that we can patch the descriptor: Chrome & Firefox
2525
if (isBrowser) {
26-
patchOnProperties(window, eventNames);
26+
patchOnProperties(window, eventNames.concat(['resize']));
2727
patchOnProperties(Document.prototype, eventNames);
2828
if (typeof(<any>window)['SVGElement'] !== 'undefined') {
2929
patchOnProperties((<any>window)['SVGElement'].prototype, eventNames);

Diff for: ‎lib/common/utils.ts

+8-5
Original file line numberDiff line numberDiff line change
@@ -126,20 +126,23 @@ export function patchProperty(obj: any, prop: string) {
126126
}
127127
if (target.hasOwnProperty(_prop)) {
128128
return target[_prop];
129-
} else {
129+
} else if (originalDescGet) {
130130
// result will be null when use inline event attribute,
131131
// such as <button onclick="func();">OK</button>
132132
// because the onclick function is internal raw uncompiled handler
133133
// the onclick will be evaluated when first time event was triggered or
134134
// the property is accessed, https://github.com/angular/zone.js/issues/525
135135
// so we should use original native get to retrieve the handler
136136
let value = originalDescGet.apply(this);
137-
value = desc.set.apply(this, [value]);
138-
if (typeof target['removeAttribute'] === 'function') {
139-
target.removeAttribute(prop);
137+
if (value) {
138+
desc.set.apply(this, [value]);
139+
if (typeof target['removeAttribute'] === 'function') {
140+
target.removeAttribute(prop);
141+
}
142+
return value;
140143
}
141-
return value;
142144
}
145+
return null;
143146
};
144147

145148
Object.defineProperty(obj, prop, desc);

Diff for: ‎test/browser/XMLHttpRequest.spec.ts

+5
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,11 @@ describe('XMLHttpRequest', function() {
6161
req.send();
6262
});
6363

64+
it('should return null when access ontimeout first time without error', function() {
65+
let req: XMLHttpRequest = new XMLHttpRequest();
66+
expect(req.ontimeout).toBe(null);
67+
});
68+
6469
const supportsOnProgress = function() {
6570
return 'onprogress' in new XMLHttpRequest();
6671
};

Diff for: ‎test/browser/browser.spec.ts

+91-92
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,20 @@ function promiseUnhandleRejectionSupport() {
2121
}
2222

2323
function canPatchOnProperty(obj: any, prop: string) {
24-
if (!obj) {
25-
return false;
26-
}
27-
const desc = Object.getOwnPropertyDescriptor(obj, prop);
28-
if (!desc || !desc.configurable) {
29-
return false;
30-
}
31-
return true;
32-
}
24+
const func = function() {
25+
if (!obj) {
26+
return false;
27+
}
28+
const desc = Object.getOwnPropertyDescriptor(obj, prop);
29+
if (!desc || !desc.configurable) {
30+
return false;
31+
}
32+
return true;
33+
};
3334

34-
(canPatchOnProperty as any).message = 'patchOnProperties';
35+
(func as any).message = 'patchOnProperties';
36+
return func;
37+
}
3538

3639
let supportsPassive = false;
3740
try {
@@ -84,95 +87,91 @@ describe('Zone', function() {
8487
expect(confirmSpy).toHaveBeenCalledWith('confirmMsg');
8588
});
8689

87-
describe('DOM onProperty hooks', ifEnvSupports(canPatchOnProperty, function() {
88-
let mouseEvent = document.createEvent('Event');
89-
let hookSpy: Spy, eventListenerSpy: Spy;
90-
const zone = rootZone.fork({
91-
name: 'spy',
92-
onScheduleTask: (parentZoneDelegate: ZoneDelegate, currentZone: Zone,
93-
targetZone: Zone, task: Task): any => {
94-
hookSpy();
95-
return parentZoneDelegate.scheduleTask(targetZone, task);
96-
}
90+
describe(
91+
'DOM onProperty hooks',
92+
ifEnvSupports(canPatchOnProperty(HTMLElement.prototype, 'onclick'), function() {
93+
let mouseEvent = document.createEvent('Event');
94+
let hookSpy: Spy, eventListenerSpy: Spy;
95+
const zone = rootZone.fork({
96+
name: 'spy',
97+
onScheduleTask: (parentZoneDelegate: ZoneDelegate, currentZone: Zone, targetZone: Zone,
98+
task: Task): any => {
99+
hookSpy();
100+
return parentZoneDelegate.scheduleTask(targetZone, task);
101+
}
102+
});
103+
104+
beforeEach(function() {
105+
mouseEvent.initEvent('mousedown', true, true);
106+
hookSpy = jasmine.createSpy('hook');
107+
eventListenerSpy = jasmine.createSpy('eventListener');
108+
});
109+
110+
it('window onclick should be in zone',
111+
ifEnvSupports(canPatchOnProperty(window, 'onmousedown'), function() {
112+
zone.run(function() {
113+
window.onmousedown = eventListenerSpy;
97114
});
98115

99-
beforeEach(function() {
100-
mouseEvent.initEvent('mousedown', true, true);
101-
hookSpy = jasmine.createSpy('hook');
102-
eventListenerSpy = jasmine.createSpy('eventListener');
116+
window.dispatchEvent(mouseEvent);
117+
118+
expect(hookSpy).toHaveBeenCalled();
119+
expect(eventListenerSpy).toHaveBeenCalled();
120+
window.removeEventListener('mousedown', eventListenerSpy);
121+
}));
122+
123+
it('window onresize should be patched',
124+
ifEnvSupports(canPatchOnProperty(window, 'onmousedown'), function() {
125+
window.onresize = eventListenerSpy;
126+
const innerResizeProp: any = (window as any)[zoneSymbol('_onresize')];
127+
expect(innerResizeProp).toBeTruthy();
128+
innerResizeProp();
129+
expect(eventListenerSpy).toHaveBeenCalled();
130+
window.removeEventListener('resize', eventListenerSpy);
131+
}));
132+
133+
it('document onclick should be in zone',
134+
ifEnvSupports(canPatchOnProperty(Document.prototype, 'onmousedown'), function() {
135+
zone.run(function() {
136+
document.onmousedown = eventListenerSpy;
103137
});
104138

105-
it('window onclick should be in zone',
106-
ifEnvSupports(
107-
() => {
108-
return canPatchOnProperty(window, 'onmousedown');
109-
},
110-
function() {
111-
zone.run(function() {
112-
window.onmousedown = eventListenerSpy;
113-
});
114-
115-
window.dispatchEvent(mouseEvent);
116-
117-
expect(hookSpy).toHaveBeenCalled();
118-
expect(eventListenerSpy).toHaveBeenCalled();
119-
window.removeEventListener('mousedown', eventListenerSpy);
120-
}));
121-
122-
it('document onclick should be in zone',
123-
ifEnvSupports(
124-
() => {
125-
return canPatchOnProperty(Document.prototype, 'onmousedown');
126-
},
127-
function() {
128-
zone.run(function() {
129-
document.onmousedown = eventListenerSpy;
130-
});
131-
132-
document.dispatchEvent(mouseEvent);
133-
134-
expect(hookSpy).toHaveBeenCalled();
135-
expect(eventListenerSpy).toHaveBeenCalled();
136-
document.removeEventListener('mousedown', eventListenerSpy);
137-
}));
138-
139-
it('SVGElement onclick should be in zone',
140-
ifEnvSupports(
141-
() => {
142-
return typeof SVGElement !== 'undefined' &&
143-
canPatchOnProperty(SVGElement.prototype, 'onmousedown');
144-
},
145-
function() {
146-
const svg = document.createElementNS('http://www.w3.org/2000/svg', 'svg');
147-
document.body.appendChild(svg);
148-
zone.run(function() {
149-
svg.onmousedown = eventListenerSpy;
150-
});
151-
152-
svg.dispatchEvent(mouseEvent);
153-
154-
expect(hookSpy).toHaveBeenCalled();
155-
expect(eventListenerSpy).toHaveBeenCalled();
156-
svg.removeEventListener('mouse', eventListenerSpy);
157-
document.body.removeChild(svg);
158-
}));
159-
160-
it('get window onerror should not throw error',
161-
ifEnvSupports(
162-
() => {
163-
return canPatchOnProperty(window, 'onerror');
164-
},
165-
function() {
166-
const testFn = function() {
167-
let onerror = window.onerror;
168-
window.onerror = function() {};
169-
onerror = window.onerror;
170-
};
171-
expect(testFn()).not.toThrow();
172-
}));
139+
document.dispatchEvent(mouseEvent);
173140

141+
expect(hookSpy).toHaveBeenCalled();
142+
expect(eventListenerSpy).toHaveBeenCalled();
143+
document.removeEventListener('mousedown', eventListenerSpy);
174144
}));
175145

146+
it('SVGElement onclick should be in zone',
147+
ifEnvSupports(
148+
canPatchOnProperty(SVGElement && SVGElement.prototype, 'onmousedown'), function() {
149+
const svg = document.createElementNS('http://www.w3.org/2000/svg', 'svg');
150+
document.body.appendChild(svg);
151+
zone.run(function() {
152+
svg.onmousedown = eventListenerSpy;
153+
});
154+
155+
svg.dispatchEvent(mouseEvent);
156+
157+
expect(hookSpy).toHaveBeenCalled();
158+
expect(eventListenerSpy).toHaveBeenCalled();
159+
svg.removeEventListener('mouse', eventListenerSpy);
160+
document.body.removeChild(svg);
161+
}));
162+
163+
it('get window onerror should not throw error',
164+
ifEnvSupports(canPatchOnProperty(window, 'onerror'), function() {
165+
const testFn = function() {
166+
let onerror = window.onerror;
167+
window.onerror = function() {};
168+
onerror = window.onerror;
169+
};
170+
expect(testFn).not.toThrow();
171+
}));
172+
173+
}));
174+
176175
describe('eventListener hooks', function() {
177176
let button: HTMLButtonElement;
178177
let clickEvent: Event;

0 commit comments

Comments
 (0)