mapbox / mapbox-gl-supported

A library to determine if a browser supports Mapbox GL JS
BSD 3-Clause "New" or "Revised" License
42 stars 17 forks source link

Add a check for "Uint8ClampedArray" #1

Closed lucaswoj closed 8 years ago

lucaswoj commented 8 years ago

ref https://github.com/mapbox/mapbox-gl-js/issues/2444

kachkaev commented 8 years ago

Any plans on this yet? I recently tried Mapbox GL with IE 11.0.9600.16428 and found it impossible to ask a user to update. This is because mapboxgl.supported() returns true but then a Uint8ClampedArray-related exception is thrown. Would be good to have the compatibility check fixed so that an alert on https://www.mapbox.com/mapbox-gl-js/example/check-for-support/ is shown.

lucaswoj commented 8 years ago

@kachkaev Thank you for your patience! This has been slipping on my todo list for a little while. I just pushed a PR to add this check. Looking forward to 🚢 this very soon.

kachkaev commented 8 years ago

Hi again guys! When about are you planning to roll out 1.2.0 to mapbox.com? I just opened the example in that faulty IE11 and noticed that the behaviour did not change.

lucaswoj commented 8 years ago

My apologies @kachkaev, I did not update the version in that example. Fixed in https://github.com/mapbox/mapbox-gl-js/commit/f9db8ab6157155fbeba31156d92388d20131d295 (might take a few minutes to be published on mapbox.com)

kachkaev commented 8 years ago

@lucaswoj should not it be 1.2.0 instead of 1.1.1?

lucaswoj commented 8 years ago

Yes. A couple more publishing steps in https://github.com/mapbox/mapbox-gl-js/commit/feeba68ef57878daabaae64d338f255ca1335afd

kachkaev commented 8 years ago

Hmm. Although I see 1.2.0 in the source code of https://www.mapbox.com/mapbox-gl-js/example/check-for-support/, IE11 still fires an exception in the console and shows no alert.

SCRIPT5009: 'Uint8ClampedArray' is undefined
File: mapbox-gl.js, Line: 212, Column: 2693
SCRIPT5009: 'mapboxgl' is undefined
File: check-for-support, Line: 443, Column: 5

It seems like Uint8ClampedArray is being referenced in the code before mapboxgl.supported().

Could you please check this if you have access to IE 11.0.9600.16428?

lucaswoj commented 8 years ago

Could you please check this if you have access to IE 11.0.9600.16428?

Unfortunately I don't have access to this version.

It seems like Uint8ClampedArray is being referenced in the code before mapboxgl.supported().

You are correct. You will need to defer loading GL JS until after mapboxgl.supported() returns true. Unfortunately I am prevented from making our published example work this way because some stock boilerplate is injected that includes loading GL JS.

Here is an untested prototype of this:

if (!mapboxgl.supported()) {
    alert('Your browser does not support Mapbox GL');
} else {
    var element = document.createElement('script');
    element.src = 'https://api.mapbox.com/mapbox-gl-js/v0.21.0/mapbox-gl.js';
    document.body.appendChild(element);
    element.onload = function() {
        var map = new mapboxgl.Map({
            container: 'map',
            style: 'mapbox://styles/mapbox/streets-v9',
            center: [-74.50, 40],
            zoom: 9
        });
    }
}
kachkaev commented 8 years ago

Well, it's a shame that the current design does not allow for a clean and robust way of calling mapboxgl.supported(). The idea of this function is to enable a 100% reliable customer experience in any ancient environment by letting the developers fall back to something simple.

Deferring the load of mapbox.js is not something everyone can afford. E.g. this is against a pattern suggested in npm+es6+webpack projects.

In my view this situation needs fixing. It wont cause any problems in my current small project since I can just ask a client to open the demo in Chrome. But I'm sure that some of your paid clients who use mapbox at scale will sooner or later get strange error reports from their customers and it will be extremely difficult to fight them. It's hard to estimate how much money can be wasted, especially including the lost opportunity costs.

mvanroon commented 7 years ago

The guys at three.js ran into the same problem. They fixed it by checking for Uint8ClampedArray support and use Uint8Array if it's not supported: https://github.com/mrdoob/three.js/pull/11462

andrewharvey commented 7 years ago

The guys at three.js ran into the same problem They fixed it by checking for Uint8ClampedArray support and use Uint8Array if it's not > supported: mrdoob/three.js#11462

So something like this should work if it is inserted before GL JS (credit to @mourner who suggested this):

if (!window.Uint8ClampedArray) window.Uint8ClampedArray = window.Uint8Array;

mvanroon commented 7 years ago

exactly - but it makes more sense to put this into the GL JS source code in my opinion