New Features about Number.prototype.toLocaleString() for IE11 in win7 and 8, a lot of related new definitions (Final PR)#67
Conversation
|
when will this be merged? should I wait for cleaning up work finished? |
|
Hi! I'm in the process of preparing version 3. I'd suggest holding off on any code cleanup until that's done. Once the new version is ready, I'll take care of this PR. |
|
how does this PR work with current build? |
| description: | ||
| 'Localized number formatting exclusive to Interner Explorer 11 in Windows 8.\n' + | ||
| 'In this case, Latvian and Russian string representation of Infinity are both "∞".', | ||
| check: | ||
| function () | ||
| { | ||
| var available = Infinity.toLocaleString('lv') === '∞' && | ||
| Infinity.toLocaleString('ru') === '∞'; |
There was a problem hiding this comment.
Since Latvian is not used in definitions with this feature, it's fine to leave it out. We could mention only Russian.
Another thing I noticed is that this feature is supported in all modern browsers, so the name and the description don't seem to fit.
There was a problem hiding this comment.
Just repurposed that into RUSSIAN_INFINITY for modern browsers usage.
| case 'lv': | ||
| switch (+this) // In Internet Explorer 9, +this is different from this. | ||
| { | ||
| case Infinity: | ||
| return '∞'; | ||
| case -Infinity: | ||
| return '-∞'; | ||
| } | ||
| break; |
There was a problem hiding this comment.
As above, this isn't needed
| case 'lv': | |
| switch (+this) // In Internet Explorer 9, +this is different from this. | |
| { | |
| case Infinity: | |
| return '∞'; | |
| case -Infinity: | |
| return '-∞'; | |
| } | |
| break; |
| switch (locale) | ||
| { | ||
| case 'lv': | ||
| switch (+this) // In Internet Explorer 9, +this is different from this. |
There was a problem hiding this comment.
Internet Explorer 9 is no longer supported, so it's fine to use just this instead of +this.
| switch (+this) // In Internet Explorer 9, +this is different from this. | |
| switch (this) |
Same for the other occurrences in this file.
|
can we merge this now? |
|
I've merged the parts with |
fasttime
force-pushed
the
main
branch
2 times, most recently
from
39a28a2 to
d37266a
Compare
| [ | ||
| define('(RP_5_A + atob("NaNfalse"))[10]'), | ||
| ], | ||
| '\x1e': '(RP_5_A + atob("NaNfalse"))[10]', |
There was a problem hiding this comment.
The array indicates the definition is non-static, i.e. it can expand differently depending on the selected feature set. For example, currently:
console.log('DEFAULT', JScrewIt.encode('"\x1e"').length);
console.log('CHROME', JScrewIt.encode('"\x1e"', { features: "CHROME" }).length);prints:
DEFAULT 3605
CHROME 851
but with this PR applied it would become:
DEFAULT 3605
CHROME 3605
So we can't replace 1-element definition arrays with strings. I've added checks to prevent accidental changes like this.
2 participants
I've finally made all these local
InfinityandNaNstuff building normally. Is there anything else to do for us?