fix: fallback when fetch does not supports URL scheme "data"

[pecoram]

This PR implements a fallback for cases where createImageBitmap is supported but the fetch fails because does not supports URL scheme "data" (for example when using a polyfill).

[elsassph]

This looks very unsafe:

• There can be many errors caught here,
• It seems to be blindly trying to parse src as a data URL, while this seem like a very specific case.

[pecoram]

like this is better?

[wouterlucas]

Honestly I don't think we should do fallback when we can predetermine that the createImageBitmap won't work.

In such case we should just rely on Image() and turn off the path that involves createImageBitmap.

[wouterlucas]

if fetch is a promise, why wrap it in a try/catch block?

[elsassph]

It's idiomatic nowadays to try { await } catch

[wouterlucas]

is it? that doesn't automatically mean its a good idea 🙃

[wouterlucas]

wouldn't it make more sense to do the src.startsWith('data:') check up top before or just after we do fetch? Sounds expensive to always make the fetch fail first, deal with the error chain and then try to parse a data blob.

If its only an error case you're worried about, why not:

const response = await fetch(src).catch( e => { /* .... handle error */ });

if (!response && src.startWith('data:')) {
  /* ... do alternate parsing here */
}

If this if this is a 1 in 10 or 1 in 100 time case, going through the try/catch error chain to then figure out its a data: and do an alternative path is very expensive.

[wouterlucas]

I honestly think this is a bad idea, rather keep things straight and neat instead of trying to fallback and make it work. If the feature is not available, we shouldn't try to make it work. It will cause confusion.

[pecoram]

That is a specific case where createImageBitmap exists but fetch("data:..") is not compatible.
I detected it using createImageBitmap polyfill.
I don't know in advance if the device supports it or not and the fetch only returns me a message without an errorcode or type: Fetch API cannot load data:image/png;base64,iVB... URL scheme "data" is not supported.

[pecoram]

What can I do is using a function like this:

async function supportsDataURLFetch() {
  try {
    await fetch('data:text/plain,Test');
    return true;
  } catch (error) {
    return false;
  }
}

or

function supportsDataURLFetch() {
  return fetch('data:text/plain,Test')
    .then(() => true)
    .catch(() => false);
}

and check it at startup

[wouterlucas]

two questions:

• Does polyfilling it actually helps over just turning createImageBitmap support off?
• What polyfill are you using, as this seems like limitations of the polyfill?

[pecoram]

I used the polyfill (https://gist.github.com/pseudosavant/a6d970b945ae85ef4dbc43200da94faf) because the images were not taking the correct size, but with polyfill it was giving me that exception. Then I realised that by removing width and height from the new Image everything worked correctly and then I could remove the polyfill. I added the fallback in the PR to avoid potential problems like this.

[wouterlucas]

if it didn't match, it would still return src and then fail down below in the base64toBlob?
I think we should handle the error case more strictly if we can figure it is not Base64 and bail out gracefully.

[pecoram]

OK - I'm opening the PR with just the change we need, then we'll consider whether to go ahead with this