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

New Features about Number.prototype.toLocaleString() for IE11 in win7 and 8, a lot of related new definitions #52

Closed
wants to merge 27 commits into from

Conversation

Clayblockunova
Copy link
Contributor

I think useLocaleNumeralDefinition() should also be updated to ensure all paddings included.

@Clayblockunova
Copy link
Contributor Author

can I remove all -Infinity references?

{
var available =
checkLocaleNumeral('ar', NaN, /^ليس برقم/) &&
checkLocaleNumeral('ar', Infinity, /^+لا.نهاية/) &&
Copy link
Owner

Choose a reason for hiding this comment

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

I'm getting a syntax error on this regular expression.

@fasttime
Copy link
Owner

can I remove all -Infinity references?

If the tests pass I'd say yes.

checkLocaleNumeral('ar', NaN, /^ليس.برقم/) &&
checkLocaleNumeral('ar', Infinity, /^\+لا.نهاية/) &&
checkLocaleNumeral('ar-td', 234567890.1, /^٢٣٤٬?٥٦٧٬?٨٩٠٫١/) &&
checkLocaleNumeral('cz', NaN, /^Není.číslo/) &&
Copy link
Owner

Choose a reason for hiding this comment

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

I'm trying this in IE 11 on Windows 7, but NaN.toLocaleString('cz') results in 'NaN'.

src/lib/features.js Outdated Show resolved Hide resolved
Comment on lines 586 to 589
checkLocaleNumeral('lt', Infinity, /^begalybė/) &&
checkLocaleNumeral('lv', Infinity, /^bezgalība/) &&
checkLocaleNumeral('pl', Infinity, /^\+nieskończoność/) &&
checkLocaleNumeral('ru', Infinity, /^бесконечность/) &&
Copy link
Owner

Choose a reason for hiding this comment

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

Infinity.toLocaleString('lv') and Infinity.toLocaleString('ru') are giving "∞" on Windows 8.

@Clayblockunova Clayblockunova changed the title New Feature LOCALE_NUMERALS_IE11_WIN7, a lot of related new definitions New Features about Number.prototype.toLocaleString() for IE11 in win7 and 8, a lot of related new definitions Aug 19, 2024
@Clayblockunova
Copy link
Contributor Author

what should I do now?

Comment on lines +386 to +388
var LOCALE_NUMERALS_IE11_WIN7 = Feature.LOCALE_NUMERALS_IE11_WIN7;
var LOCALE_NUMERALS_IE11_WIN7_8 = Feature.LOCALE_NUMERALS_IE11_WIN7_8;
var LOCALE_NUMERALS_IE11_WIN8 = Feature.LOCALE_NUMERALS_IE11_WIN8;
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure if this division makes sense. It seems that these features depend on the version of certain DLLs used by IE, and these DLLs could be updated independently from the operating system, for example with Windows updates. This is especially true for Windows 7 and 8 where IE was integrated with other system components.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so, what should I do? should I just remove Latvian and Russian related definition?

Copy link
Owner

Choose a reason for hiding this comment

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

I don't know yet. I will have to do some more analysis.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is your analysis finished now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and, what should I do to Latvian and Russian related definition?

@Clayblockunova
Copy link
Contributor Author

Sorry for closing this PR since I have something else to add.

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

Successfully merging this pull request may close these issues.

2 participants