Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parseInt() shims for ES5 whitespace that is not ES2017 whitespace #433

Open
tjosseau opened this issue Mar 20, 2017 · 4 comments
Open

parseInt() shims for ES5 whitespace that is not ES2017 whitespace #433

tjosseau opened this issue Mar 20, 2017 · 4 comments
Assignees

Comments

@tjosseau
Copy link

The new version of Firefox (52) broke the parseInt() behavior on the test if (parseInt(ws + '08') !== 8 || parseInt(ws + '0x16') !== 22) (line 1956) because of wrong whitespace characters ; returning NaN for both instead of 8 and 22.

Thus parseInt() is redefined by your shim. However, the case parseInt() (empty parameter, or undefined, or null) throws an exception while using trim(string) (line 1962) instead of returning NaN (as default behavior) - trim() should never be called with undefined or null as parameter.

This fixes the issue : if (str === undefined || str === null) str = ""; (to add before line 1962) ; Do not attempt to return NaN directly instead as it has bad side effects on some libraries (such as PerfectScrollbar) - I don't know why but it does.

Full example:

// ES-5 15.1.2.2
/* eslint-disable radix */
if (parseInt(ws + '08') !== 8 || parseInt(ws + '0x16') !== 22) {
/* eslint-enable radix */
    /* global parseInt: true */
    parseInt = (function (origParseInt) {
        var hexRegex = /^[\-+]?0[xX]/;
        return function parseInt(str, radix) {
            if (str === undefined || str === null) str = "" ; // Fix here
            var string = trim(str);
            var defaultedRadix = $Number(radix) || (hexRegex.test(string) ? 16 : 10);
            return origParseInt(string, defaultedRadix);
        };
    }(parseInt));
}

The redefined function parseFloat() should have the same fix too (before line 1974).


I've reported the parseInt() issue to Bugzilla, and here is the answer : https://bugzilla.mozilla.org/show_bug.cgi?id=1347869

To sum up, \u180E is no longer supported in Unicode 9 as a whitespace. This particular test should be removed in the ws variable (line 1903).

@ljharb
Copy link
Member

ljharb commented Mar 23, 2017

@tjosseau are you using the latest version of es5-shim? The current source has trim(String(str)).

@tjosseau
Copy link
Author

Oops, indeed, I tested that on an older version. Just tested with the last version, and it works well.
Thanks for the reply, you can close or remove this issue. :)

@ljharb
Copy link
Member

ljharb commented Mar 23, 2017

Thanks for confirming!

@ljharb ljharb closed this as completed Mar 23, 2017
@ljharb
Copy link
Member

ljharb commented Mar 23, 2017

Actually though I'll reopen to think about the Unicode issue; it won't break anything to shim unnecessarily but it's good to avoid.

@ljharb ljharb reopened this Mar 23, 2017
@ljharb ljharb changed the title parseInt() throws exception instead of returning NaN parseInt() shims for ES5 whitespace that is not ES2017 whitespace Mar 23, 2017
@ljharb ljharb self-assigned this Mar 23, 2017
ljharb added a commit to ljharb/es5-shim that referenced this issue Mar 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants