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

Fix exceptions in IE #1786

Merged
merged 9 commits into from
Jun 16, 2018
Merged
Show file tree
Hide file tree
Changes from 6 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
3 changes: 2 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ module.exports = {
'no-irregular-whitespace': 1,
'no-self-assign': 1,
'new-cap': 1,
'no-undefined': 1
'no-undefined': 1,
'no-global-assign': 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

if this is a critical issue, you shouldn't make it a warning, but an error (2).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah thanks. Will address

}
};
11 changes: 5 additions & 6 deletions src/crypt/decrypter.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@ import Event from '../events';
import { getSelfScope } from '../utils/get-self-scope';

// see https://stackoverflow.com/a/11237259/589493
/* eslint-disable-next-line no-undef */
const window = getSelfScope(); // safeguard for code that might run both on worker and main thread

const { performance, crypto } = window;
const global = getSelfScope(); // safeguard for code that might run both on worker and main thread
Copy link
Collaborator

Choose a reason for hiding this comment

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

if this is a problem on IE11, can we add a linter setting that forbids the variable name window? it would be the way to avoid regression in this case.

Copy link
Collaborator

Choose a reason for hiding this comment

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

or this is what no-global-assign does?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah no-global-assign does that


class Decrypter {
constructor (observer, config, { removePKCS7Padding = true } = {}) {
Expand All @@ -24,8 +21,10 @@ class Decrypter {
// built in decryptor expects PKCS7 padding
if (removePKCS7Padding) {
try {
const browserCrypto = crypto || window.crypto;
this.subtle = browserCrypto.subtle || browserCrypto.webkitSubtle;
const browserCrypto = global.crypto;
if (browserCrypto) {
this.subtle = browserCrypto.subtle || browserCrypto.webkitSubtle;
}
} catch (e) {}
}
this.disableWebCrypto = !this.subtle;
Expand Down
6 changes: 2 additions & 4 deletions src/demux/demuxer-inline.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,8 @@ import PassThroughRemuxer from '../remux/passthrough-remuxer';
import { getSelfScope } from '../utils/get-self-scope';

// see https://stackoverflow.com/a/11237259/589493
/* eslint-disable-next-line no-undef */
const window = getSelfScope(); // safeguard for code that might run both on worker and main thread

const { performance } = window;
const global = getSelfScope(); // safeguard for code that might run both on worker and main thread
const performance = global;

class DemuxerInline {
constructor (observer, typeSupported, config, vendor) {
Expand Down
8 changes: 3 additions & 5 deletions src/demux/demuxer.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ import { getMediaSource } from '../utils/mediasource-helper';
import { getSelfScope } from '../utils/get-self-scope';

// see https://stackoverflow.com/a/11237259/589493
/* eslint-disable-next-line no-undef */
const window = getSelfScope(); // safeguard for code that might run both on worker and main thread

const global = getSelfScope(); // safeguard for code that might run both on worker and main thread
const MediaSource = getMediaSource();

class Demuxer {
Expand Down Expand Up @@ -69,7 +67,7 @@ class Demuxer {
logger.error('error while initializing DemuxerWorker, fallback on DemuxerInline');
if (w) {
// revoke the Object URL that was used to create demuxer worker, so as not to leak it
window.URL.revokeObjectURL(w.objectURL);
global.URL.revokeObjectURL(w.objectURL);
}
this.demuxer = new DemuxerInline(observer, typeSupported, config, vendor);
this.w = undefined;
Expand Down Expand Up @@ -134,7 +132,7 @@ class Demuxer {
switch (data.event) {
case 'init':
// revoke the Object URL that was used to create demuxer worker, so as not to leak it
window.URL.revokeObjectURL(this.w.objectURL);
global.URL.revokeObjectURL(this.w.objectURL);
break;
// special case for FRAG_PARSING_DATA: data1 and data2 are transferable objects
case Event.FRAG_PARSING_DATA:
Expand Down
16 changes: 13 additions & 3 deletions src/remux/mp4-generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
*/

// import Hex from '../utils/hex';
Copy link
Collaborator

Choose a reason for hiding this comment

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

if this import isn't needed we should remove it

import { makeArrayFromArrayLike } from '../utils/make-array-from-array-like';
Copy link
Collaborator

@tchakabam tchakabam Jun 15, 2018

Choose a reason for hiding this comment

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

if you wanted to remove it, we should have deleted the module as well. but imo we should definitely keep it.


const UINT32_MAX = Math.pow(2, 32) - 1;

Expand Down Expand Up @@ -335,7 +334,13 @@ class MP4 {
len = data.byteLength;
sps.push((len >>> 8) & 0xFF);
sps.push((len & 0xFF));
sps = sps.concat(makeArrayFromArrayLike(data)); // SPS

// SPS
if (typeof Array.from === 'function') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

not sure what is broken in there, but: why don't you fix the util function (makeArrayFromArrayLike)?

the way you did it here, we will have more code-size if this functionnality is needed somewhere else.

Copy link
Collaborator

Choose a reason for hiding this comment

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

update: see my proposed fix further down

sps = sps.concat(Array.from(data));
} else {
sps = sps.concat(Array.prototype.slice.call(data));
}
}

// assemble the PPSs
Expand All @@ -344,7 +349,12 @@ class MP4 {
len = data.byteLength;
pps.push((len >>> 8) & 0xFF);
pps.push((len & 0xFF));
pps = pps.concat(makeArrayFromArrayLike(data));

if (typeof Array.from === 'function') {
Copy link
Collaborator

@tchakabam tchakabam Jun 15, 2018

Choose a reason for hiding this comment

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

like this is actually redundant code really growing the package, isn't it?

it seems more intuitive to rather fix the issue here: https://github.com/video-dev/hls.js/blob/master/src/utils/make-array-from-array-like.js#L1

if the current state doesn't work...I guess you need to change it to something like:

export function makeArrayFromArrayLike(arrayLike) { 
  return (typeof Array.from === 'function' ? Array.from(arrayLike) : Array.prototype.slice.call(arrayLike));
}

and you can use it without changes the exact same way.

IE11 seems to have a bug in exporting native prototype members :)

pps = pps.concat(Array.from(data));
} else {
pps = pps.concat(Array.prototype.slice.call(data));
}
}

let avcc = MP4.box(MP4.types.avcC, new Uint8Array([
Expand Down
7 changes: 3 additions & 4 deletions src/utils/logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,17 @@ function formatMsg (type, msg) {
return msg;
}

/* eslint-disable-next-line no-undef */
const window = getSelfScope();
const global = getSelfScope();

function consolePrintFn (type) {
const func = window.console[type];
const func = global.console[type];
if (func) {
return function (...args) {
if (args[0]) {
args[0] = formatMsg(type, args[0]);
}

func.apply(window.console, args);
func.apply(global.console, args);
};
}
return noop;
Expand Down