Skip to content

Do not try to replace window.navigator#215

Merged
raphinesse merged 1 commit intoapache:masterfrom
raphinesse:untouched-navigator
Oct 14, 2019
Merged

Do not try to replace window.navigator#215
raphinesse merged 1 commit intoapache:masterfrom
raphinesse:untouched-navigator

Conversation

@raphinesse
Copy link
Contributor

@raphinesse raphinesse commented Oct 13, 2019

window.navigator is a read-only property. Thus the current code for replacing it cannot succeed in a spec compliant browser. In strict mode the attempt will throw an error. In sloppy mode, the one Cordova is currently running in, the attempt is silently ignored.

Since apps seem to have been doing OK with the original Navigator object we should just remove the code trying to replace it.

window.navigator is a read-only property. Thus the current code for
replacing it cannot succeed in a [spec][1] compliant browser. In strict
mode the attempt will throw an error. In sloppy mode, the one Cordova is
currently running in, the attempt is silently ignored.

Since apps seem to have been doing OK with the original navigator object
we should just remove the code trying to replace it.

[1]: https://html.spec.whatwg.org/multipage/window-object.html#the-window-object
@breautek
Copy link

In strict mode the attempt will throw an error.

After these changes, is using "use strict;" possible? I think with files that are strict mode friendly we should enable strict mode as it will allow the JS engines to enable optimisations that is otherwise impossible, and will prevent some bad practices being implemented in the future as well.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode

@raphinesse
Copy link
Contributor Author

raphinesse commented Oct 13, 2019

@breautek I'd be definitely in favor of enabling strict mode in the future. cordova-js should be strict mode ready after this change. But we need to proof the platforms too (since they are bundled into the same file as the base from cordova-js). There's at least one minor bug in cordova-android that will have to be fixed for this.

@raphinesse raphinesse merged commit c8aca1e into apache:master Oct 14, 2019
@raphinesse raphinesse deleted the untouched-navigator branch October 14, 2019 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments