btoa and atob declarations break native browser implementations for other libraries

[thomaspeeters]

I'm currently trying to set up a video.js player using dash.js, videojs-contrib-dash and videojs-contrib-hls.

videojs-contrib-hls uses window.btoa several times and it seems this part of dash.debug.js overrides this native implementation of btoa.

if (undefined === btoa) {
    var btoa = BASE64.encode;
}

if (undefined === atob) {
    var atob = BASE64.decode;
}

This isn't a problem in Firefox and Chrome, but breaks playback of HLS streams in IE (tested in IE11 on Win7).

I did some testing and following changes fix the problem:

• Removing the var keyword

if (undefined === btoa) {
    btoa = BASE64.encode;
}

if (undefined === atob) {
    atob = BASE64.decode;
}

• Changing to encodeASCII and decodeASCII as per atob/btoa got patched on chrome #419

if (undefined === btoa) {
    var btoa = BASE64.encodeASCII;
}

if (undefined === atob) {
    var atob = BASE64.decodeASCII;
}

• Or just removing the declarations altogether, because they don't seem to be used anywhere anymore. See commit fabeba3 where all references to btoa and atob seem to have been removed.

[cagen]

@thomaspeeters +1
I encountered this problem too. And thanks for pointing out the source of this problem.
My test environment:
Mac OSX El Captian 10.11.1
Chrome 46.0.2490.86 (64-bit)
dashjs: 1.5.0
videojs-contrib-hls: 1.2.1

And I find it will break Chrome if you force to play video in Flash mode.
It seems the videojs-contrib-hls is using window.btoa() to encode data and then pass it to swfObject. Maybe this is a different implementation to the native btoa() and cause the calculation of buffered content have wrong timerange.

IMHO, this implementation may trying to make btoa() work for legacy IE that don't support it. But the usage of var break that purpose because of hoist

Perhaps removing the var keyword may be the right way? And it will solve the conflict problem.
But it will still have the different implementation in legacy IE, right? I don't know whether they should be the same or not...

[thomaspeeters]

I saw there is a pull request related to this issue: #902

Did you see the note about pull requests and v2.0? It was added on October 30th: 2b78bd5

I'm guessing this issue will be resolved after - or perhaps during - the refactor to v2.0.

[cagen]

Thanks for the reply.
Grad to hear that the v2.0 is coming out. Hope this issue will be fixed then.
Great job of the Dash.js team!

[dsparacio]

I think we are going to put this in a temp 1.5.2 patch release until we can get the 2.0 version out. We will need to merge these changes in one by one manually into 2.0 so trying to overload this small release.

[thomaspeeters]

Sounds good to me. On a slightly unrelated note, do you have an ETA for 2.0?

[dsparacio]

Not on 2.0 just yet most likely mid Jan 2016. But the 1.5.2 should be out next week sometime.

[thomaspeeters]

Alright, thanks for the update. From the note in 2b78bd5, I was under the impression 2.0 would have been ready on the development branch by the end of November.

[dsparacio]

We will have something in Dev most likely right after the Dec 9th Face to face meeting. At this point we can all work on bug fixes / enhancements in the refactored branch. This will involve moving all the 1.5.2 changes that are applicable.

[thomaspeeters]

Gotcha, thanks for the info.

[dsparacio]

Another issue that was not pulled in from 1.6.0 to 2.0. It will be added to dev right away and released with 2.1

[dsparacio]

Please implement changes from MobiTV-AB@73486d3 in dev

[dsparacio]

missed code was added back in and push directly to dev without a PR.