Skip to content

Polyfill TypedArray.slice() in older browsers. fixes #11008 #11011

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/runtime_init_memory.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,18 @@ HEAP32[DYNAMICTOP_PTR>>2] = DYNAMIC_BASE;
#if USE_PTHREADS
}
#endif

#if MIN_CHROME_VERSION < 45 || MIN_EDGE_VERSION < 14 || MIN_FIREFOX_VERSION < 38 || MIN_IE_VERSION != TARGET_NOT_SUPPORTED
// Older browsers may lack TypedArray.slice()
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/slice#Browser_compatibility
[Int8Array, Uint8Array, Int16Array, Uint16Array, Int32Array, Uint32Array,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Note that all of those actually have a super-class TypedArray that is not directly exposed. That is, instead of adding a method to each and every class, you can get a TypedArray reference via

var typedArrayProto = Int8Array.prototype.__proto__;

and then polyfill typedArrayProto.slice once.

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting, thanks, I didn't know that.

Do you know if that's well-supported in all legacy browsers?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't, unfortunately, and can't test it.

Choose a reason for hiding this comment

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

Still relevant. In fact, we need these polyfills.

Tested this polyfill:

var typedArrayProto = Object.getPrototypeOf(Int8Array.prototype);
if (typedArrayProto && !typedArrayProto.slice) {
    typedArrayProto.slice = function (begin, end) {
        return new this.constructor(this.subarray(begin, end));
    }
}

webOS 1.2 (WebKit)
Polyfill works correctly.

Object.getPrototypeOf(Int8Array.prototype):     [object ArrayBufferViewPrototype]
Int8Array.prototype:                            [object Int8ArrayPrototype]
Object.getPrototypeOf(Int8Array):               [object Object]
Int8Array:                                      [object Int8ArrayConstructor]

Object.getPrototypeOf(Int32Array.prototype):    [object ArrayBufferViewPrototype]
Int32Array.prototype:                           [object Int32ArrayPrototype]
Object.getPrototypeOf(Int32Array):              [object Object]
Int32Array:                                     [object Int32ArrayConstructor]

Object.getPrototypeOf(Int8Array.prototype) === Object.getPrototypeOf(Int32Array.prototype): true
Object.getPrototypeOf(Int8Array.prototype) === Object.getPrototypeOf({}): false

webOS 3 (Chrome 38)
Polyfill works, but {} gets slice. So it is probably better to leave as it is.

Object.getPrototypeOf(Int8Array.prototype):     [object Object]
Int8Array.prototype:                            [object Object]
Object.getPrototypeOf(Int8Array):               function Empty() {}
Int8Array:                                      function Int8Array() { [nativecode] }

Object.getPrototypeOf(Int32Array.prototype):    [object Object]
Int32Array.prototype:                           [object Object]
Object.getPrototypeOf(Int32Array):              function Empty() {}
Int32Array:                                     function Int32Array() { [nativecode] }

Object.getPrototypeOf(Int8Array.prototype) === Object.getPrototypeOf(Int32Array.prototype): true
Object.getPrototypeOf(Int8Array.prototype) === Object.getPrototypeOf({}): true

Copy link
Member Author

Choose a reason for hiding this comment

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

@dmitrylyzo thanks for the info. Would you like to open a PR that takes this code and updates it to use the right polyfills?

Choose a reason for hiding this comment

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

I tested your polyfill - works fine. As I said, I think it is better (for legacy browsers) than the super-class polyfill, because in webOS 3 super-class polyfill adds slice to every object due to a common prototype (but it could just be a glitch of the emulator).

As for the other polyfills, they are for the libraries used in that project. Wouldn't their inclusion bloat the output file?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can I ask what targets do you have that run webOS 3 (chrome 38). That seems like a rather old version of the browser to target.

Choose a reason for hiding this comment

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

Mostly old TVs (https://webostv.developer.lge.com/discover/specifications/web-engine/). Jellyfin is targeting Chrome 27 as a rough approximation of webOS 1.2 and 2.0 (both WebKit).

Float32Array, Float64Array].forEach(function(typedArray) {
if (!typedArray.prototype.slice) {
Object.defineProperty(typedArray.prototype, 'slice', {
value: function(begin, end) {
return new typedArray(this.subarray(begin, end));
}
});
}
});
#endif
2 changes: 0 additions & 2 deletions tests/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -3436,8 +3436,6 @@ def test(expected, opts=[]):
# when legacy is needed, we show an error indicating so
test('build with LEGACY_VM_SUPPORT')
# legacy + disabling wasm works
if self.is_wasm_backend():
return
test('hello, world!', ['-s', 'LEGACY_VM_SUPPORT=1', '-s', 'WASM=0'])

def test_on_abort(self):
Expand Down