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

Add parentNode check when removing JSONP request file #15595

Open
FarSeeing opened this issue Jan 11, 2017 · 3 comments
Open

Add parentNode check when removing JSONP request file #15595

FarSeeing opened this issue Jan 11, 2017 · 3 comments

Comments

@FarSeeing
Copy link
Contributor

Note: for support questions, please use one of these channels: https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#question. This repository's issues are reserved for feature requests and bug reports.

Do you want to request a feature or report a bug?
A bug.

What is the current behavior?
Currently the script element created for JSONP request is appended to document.body and removed on load or error callback:
httpBackend.js#L187

rawDocument.body.removeChild(script);

This will fail if the script was removed before the callback was fired.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://plnkr.co or similar (template: http://plnkr.co/edit/tpl:yBpEi4).

What is the expected behavior?
Add a parentNode check instead of hardcoded document.body reference:

if (script.parentNode) script.parentNode.removeChild(script);

What is the motivation / use case for changing the behavior?

Which versions of Angular, and which browser / OS are affected by this issue? Did this work in previous versions of Angular? Please also test with the latest stable and snapshot (https://code.angularjs.org/snapshot/) versions.

Other information (e.g. stacktraces, related issues, suggestions how to fix)

@gkalpak
Copy link
Member

gkalpak commented Jan 11, 2017

I assume you ran into this issue. In order to better understand the issue, under what circumstances can this happen (the script being removed before the callback being called)?

@FarSeeing
Copy link
Contributor Author

In my scenario we're scraping the page with PhantomJS and then remove (nearly) all the sripts from the page. This ends in document.body.removeChild(script) call to already removed/detached element.
I do understand it's a very rare condition but the check itself is not big either.

@gkalpak
Copy link
Member

gkalpak commented Jan 11, 2017

I am not arguing against the change. I just want to understand the usecase.
It is a reasonable change, so feel free to submit a pull request.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants