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

string.format() doesn't work under IE 11 #148

Open
Egor-Skriptunoff opened this issue Feb 26, 2019 · 11 comments
Open

string.format() doesn't work under IE 11 #148

Egor-Skriptunoff opened this issue Feb 26, 2019 · 11 comments

Comments

@Egor-Skriptunoff
Copy link

Fengari is said to be "Verified to work in Microsoft IE 11".
But unfortunately an error is raised under IE 11:

print(("%02X"):format(0))
TypeError: Object doesn't support property or method 'repeat'

fengari_ie11

It seems that the sprintf.js which implements string.format is not IE11-compatible.
The problem is in the single line of code:

pad = ph.width ? (pad_length > 0 ? pad_character.repeat(pad_length) : '') : ''

Unfortunately, repeat is not implemented on all IE, including IE11

BTW .repeat occurs also in lstrlib.js.

Support of IE11 actually means support of Windows 7 users.
There are a lot of them nowadays.

All what is needed for happiness is to implement String.prototype.repeat manually in Fengari.

@daurnimator
Copy link
Member

@daurnimator
Copy link
Member

Filed alexei/sprintf.js#182
Also see #149

@Egor-Skriptunoff
Copy link
Author

I can't modify the global environment

But why?
Defining standard well-known (but undefined on IE) function looks very innocent.

@daurnimator
Copy link
Member

I've worked in environments before where the rule is never modify the global JS environment. I want fengari to be allowed in such environments.

Every modification you make is potentially conflicting with some other piece of JS running on a site. The easiest solution is to just opt out.

@Egor-Skriptunoff
Copy link
Author

Egor-Skriptunoff commented Mar 12, 2019

Probably, this issue should be migrated from fengari to fengari-web, because it affects only fengari running inside browser.

@Egor-Skriptunoff
Copy link
Author

The bug hasn't been solved in three months.
Maybe, the documentation should honestly claim that Fengari does require a polyfill for IE11?

@daurnimator
Copy link
Member

I've been hoping that alexei/sprintf.js#183 will get merged.
Otherwise I may just remove that dependency entirely and implement our own sprintf-alike.

@daurnimator
Copy link
Member

I haven't had the time to implement out own sprintf, so I'm marking this issue as "help-wanted". If anyone would like to implement it, please send a PR!

@catwell
Copy link

catwell commented Aug 16, 2019

Would it be fine just to fork + vendor sprintf.js with your fix?

@daurnimator
Copy link
Member

Would it be fine just to fork + vendor sprintf.js with your fix?

I guess. But there is plenty of code in there we don't use. And the verification code in lstrlib.js sort of duplicates some of the work anyway.
A rewrite with a bit of "inspiration" is probably the best choice.

@ashwalk33r
Copy link

ashwalk33r commented Apr 2, 2020

I can't modify the global environment

But why?
Defining standard well-known (but undefined on IE) function looks very innocent.

You pollute users namespace. It's considered a bad practice. It's much easier to point users that it's not working on this and that browser and offer a link to a polyfill - if user wants then can include polyfil from separate repo - but then it's extra request or extra job to bake it into users code.

There is a way to create a copy of String prototype and assign it to new class - OurString and decorate it encapsulated only in module scope. @daurnimator what do you think?


or alexei/sprintf.js#193

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

No branches or pull requests

4 participants