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

fix(patch): fix #746, check desc get is null and only patch window.resize additionally#747

Merged
mhevery merged 7 commits intoangular:masterfrom
JiaLiPassion:issue-741
Apr 25, 2017
Merged

fix(patch): fix #746, check desc get is null and only patch window.resize additionally#747
mhevery merged 7 commits intoangular:masterfrom
JiaLiPassion:issue-741

Conversation

@JiaLiPassion
Copy link
Copy Markdown
Collaborator

@JiaLiPassion JiaLiPassion commented Apr 22, 2017

fix #746,fix #741

  1. fix: resolve errors with closure #722 break window.onresize is not intercepted by zones #741, not patch onresize(maybe others), so I add it back in this PR
  2. fix Cannot read property 'apply' of undefined #746, check originalDescGet is null or not.

mhevery
mhevery reviewed Apr 22, 2017
View reviewed changes
Comment thread lib/common/utils.ts Outdated
if (prop.substr(0, 2) == 'on') {
onProperties.push(prop);
}
const prop = 'on' + properties[i];
Copy link
Copy Markdown
Contributor

@mhevery mhevery Apr 22, 2017

Choose a reason for hiding this comment

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

can we have an explicit list rather than iterating over everything? The issue is that closure compiler creates lots of properties on the window with mangled names, and it is quite easy for them to start with on

Copy link
Copy Markdown
Collaborator Author

@JiaLiPassion JiaLiPassion Apr 22, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Got it, I will first add on resize and figure out what others are needed.

guodong
guodong approved these changes Apr 22, 2017
View reviewed changes
Copy link
Copy Markdown

@guodong guodong left a comment

Choose a reason for hiding this comment

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

approve

@JiaLiPassion
Copy link
Copy Markdown
Collaborator Author

JiaLiPassion commented Apr 22, 2017

@mhevery , I have updated my PR.

  1. only patch specified properties
  2. window patch resize property
  3. check originalDesc is null or not

please review. thank you !

@JiaLiPassion JiaLiPassion changed the title fix(patch): fix #746, need to patch all on properties without duplicate fix(patch): fix #746, check desc get is null and only patch window.resize additionally Apr 22, 2017
This was referenced Apr 22, 2017
Merged
Closed
@mpalourdio mpalourdio mentioned this pull request Apr 22, 2017
Closed
2 tasks
@smorhaim
Copy link
Copy Markdown

smorhaim commented Apr 22, 2017

I tried the patch, it still gives the same error. Same steps to reproduce. Brand new project, ng serve, error.

@JiaLiPassion
Copy link
Copy Markdown
Collaborator Author

JiaLiPassion commented Apr 22, 2017

@smorhaim , you can try the attached zone.js, the error still exists because the dist/ folder is not updated.
dist.zip

@smorhaim
Copy link
Copy Markdown

smorhaim commented Apr 22, 2017

@JiaLiPassion it works! Thanks.

@JiaLiPassion JiaLiPassion mentioned this pull request Apr 23, 2017
Closed
@sangecz
Copy link
Copy Markdown

sangecz commented Apr 23, 2017

is it fixed? which version? i tried downgrade from 0.8.8 to 0.8.5 and 0.8.4 but same issue as described in #6063

@dkontorovskyy
Copy link
Copy Markdown

dkontorovskyy commented Apr 23, 2017

@JiaLiPassion, do you know when /dist folder is going to be updated?

Thanks!

@JiaLiPassion
Copy link
Copy Markdown
Collaborator Author

JiaLiPassion commented Apr 24, 2017

@dkontorovskyy , it will be the next version release, please wait for a while.

@pkozlowski-opensource pkozlowski-opensource mentioned this pull request Apr 24, 2017
Closed
This was referenced Apr 24, 2017
Closed
Closed
Closed
@intellix intellix mentioned this pull request Apr 25, 2017
Closed
@mhevery mhevery merged commit e598310 into angular:master Apr 25, 2017
bkbooth added a commit to loveisourweapon/liow2-client that referenced this pull request Apr 26, 2017
@bkbooth
Lock zone.js version to 0.8.5 temporarily
9551cd2 
* Fixes console error caused by [email protected]
* angular/angular-cli#6036
* angular/zone.js#747
@JiaLiPassion JiaLiPassion deleted the issue-741 branch May 6, 2017 04:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

3 more reviewers

@mhevery mhevery mhevery approved these changes

@rweng rweng rweng approved these changes

@guodong guodong guodong approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Cannot read property 'apply' of undefined window.onresize is not intercepted by zones

8 participants

@JiaLiPassion @smorhaim @sangecz @dkontorovskyy @mhevery @rweng @guodong @googlebot