Sprints: Base64_encoding_and_decoding page needs better code samples?

[mburszley]

Request type

• [ ] Please close this issue, I accidentally submitted it without adding any details
• [ ] New documentation
• [x] Correction or update

Details

The code samples in this documentation are extremely dated. Between the nested ternaries, useless names for functions, and hungarian notation, it borders on unreadable.

[chrisdavidmills]

Page in question is https://developer.mozilla.org/en-US/docs/Web/API/WindowBase64/Base64_encoding_and_decoding

[wbamberg]

Yes, I agree that these code blocks are not readable.

But, they are not really code examples. They are more like mini-libraries. The difference is that:

• a code example/code sample shows someone how to use some API. It's designed to be read and understood, and then potentially adapted for a particular use case. So it should be kept short enough that someone can read it and understand it relatively easily, and should favour readability over efficiency (within reason).

• a library is designed to be used. Outside its maintainers, most people don't expect to read the Lodash source, for instance. The maintainers can choose whatever code style suits them. Users of the library expect to drop it into their programs, and just trust that it does the thing it's supposed to. As far as I can tell, that's the intended purpose of code blocks like https://developer.mozilla.org/en-US/docs/Web/API/WindowBase64/Base64_encoding_and_decoding#Solution_1_%E2%80%93_JavaScript's_UTF-16_%3E_base64.

I don't think MDN should host libraries (just as we shouldn't host polyfills). One of the main reasons for this is that we aren't really able to verify that longish, complicated chunks of code actually do what they are intended to do, and don't contain critical bugs or, worse, vulnerabilities. People will trust that they can just drop this code into their program and it will work safely and reliably, and we're not really in a position to give that assurance.

But I'm not sure I understand the use case here. It's true that passing certain JS strings into btoa() will cause exceptions, and that it's possible to re-encode the string in such a way that this won't happen. But AFAIK this is not a problem if the string does actually contain binary data. And AFAIK the purpose of btoa() is to represent binary data as ASCII.

So... what is the use case for trying to pass Unicode text into btoa(), that could give rise to this "Unicode problem"? I feel like it would be better for us to say "don't do this, it's a misuse of the API and probably not what you want" rather than provide 5 different quite complex ways to do it. But if it is a legitimate thing that people need to do, then we need to help people find a good way to do it.

[mburszley]

For reference, @wbamberg , I believe it was probably a SO answer that pointed me to the snippet in the first place as the wonky UTF8 v. UTF16 & Base64 thing bit me.

[wbamberg]

Thanks @TheIncorrigible1 , I'm very glad you got back to us, it's really helpful. If you don't mind, I had some other questions. What was your use case for base64-encoding text? What were you trying to do? And how did you end up resolving it? Did you use any of the code snippets in that page, and were you worried about relying on hard-to-read code?

[mburszley]

@wbamberg I do a lot of API work, so it probably had to do with serialization issues between whatever I was using at the time and javascript transforming html inputs. I don't really have a more accurate answer as three months ago is an eternity for me.

[wbamberg]

@TheIncorrigible1 , thanks for getting back to me.

@madmurphy, I saw that you contributed the code snippets under https://developer.mozilla.org/en-US/docs/Web/API/WindowBase64/Base64_encoding_and_decoding#The_Unicode_Problem, so thought you should know about this issue and the proposed resolution.

I appreciate your contributions here, and agree that it's important to highlight the limitations of btoa(), but "library functions" like this are not in scope for MDN. It's not practical for us to guarantee the correctness of code like this, especially given that we don't have any tool support for it in the platform (e.g. linting or CI) or even any barriers to people editing the code at any time. For a similar case, see the discussion about polyfills on MDN. So I'm intending to remove these code snippets, and instead describe the intended usage and limitations of the btoa() API.

[madmurphy]

@wbamberg

Thank you for contacting me. I understand the points raised here. My suggestion is to keep the § Solution #3 as an immediate possible solution to the “Unicode Problem” (since this is a very small and concise snippet), and move all the other solutions in a sub-page, possibly WindowBase64/Base64_encoding_and_decoding/The_unicode_problem – something similar exists for Object.defineProperty(), which has a sub-page named “Additional examples” under Global_Objects/Object/defineProperty/Additional_examples.

The point is that _there is_ a “unicode problem” with btoa(), as this accepts only code units smaller than 255, while JavaScript strings per se allow code units up to 65535 – and binary strings are not really a thing defined in any standard.

Without creating libraries, which I agree are impossible to maintain in a wiki, I think MDN should indicate a possible solution through small snippets (maybe only one), and then provide a more complete guide in a sub-page for whoever wants to deepen the topic.

--madmurphy

[wbamberg]

Thanks for getting back to us @madmurphy !

Thank you for contacting me. I understand the points raised here. My suggestion is to keep the § Solution #3 as an immediate possible solution to the “Unicode Problem” (since this is a very small and concise snippet), and move all the other solutions in a sub-page, possibly WindowBase64/Base64_encoding_and_decoding/The_unicode_problem – something similar exists for Object.defineProperty(), which has a sub-page named “Additional examples” under Global_Objects/Object/defineProperty/Additional_examples.

I agree that keeping solution 3 is OK - it's short and simple enough for people to be able to understand the implications of using the code.

But I don't think moving the other snippets into a subpage at all addresses the issue with keeping this sort of "library content" on MDN. And the fact that other pages do this does not affect the issue either.

The point is that _there is_ a “unicode problem” with btoa(), as this accepts only code units smaller than 255, while JavaScript strings per se allow code units up to 65535 – and binary strings are not really a thing defined in any standard.

I do understand the issue with btoa(), and agree that it should be clearly stated what the problem is and when it can happen, and that including short comprehensible code snippets, (especially ones that heavily leverage browser code, instead of reinventing) is also worth doing. But I do not think including code snippets like https://developer.mozilla.org/en-US/docs/Web/API/WindowBase64/Base64_encoding_and_decoding#Solution_1_%E2%80%93_JavaScript's_UTF-16_%3E_base64 is the right thing for MDN to be doing.

[madmurphy]

@wbamberg

But I do not think including code snippets like https://developer.mozilla.org/en-US/docs/Web/API/WindowBase64/Base64_encoding_and_decoding#Solution_1_%E2%80%93_JavaScript's_UTF-16_%3E_base64 is the right thing for MDN to be doing.

I do agree with this in principle. The only problem is that § Solution #3 encodes UTF-16 strings, while the “standard” with base64 is to use UTF-8 (as this produces identical output for ASCII-only texts). And since JavaScript has no UTF-8 facilities, the only solution is to implement a UTF-8 library – this is why § Solution #2 is so long (it does contain a mini-library indeed).

For facing all this mess, long ago I had created a modern general library for MDN, StringView, able to address different Unicode encodings, even UTF-32 if needed. My username was fusionchess back then.

For a while StringView has been a strawman proposal for ES2015 (I discovered it after the discussion had already taken place), and it had even been included in the source code of Google Chrome, but then it slowly ended up into the obsolete content of MDN, although btoa() was still problematic.

So, my ideal solution would be to leave the § Solution #3 snippet and restore the StringView library out of the obsolete content for whoever wants more flexibility (maybe by relying on a GitHub repository). But in any case we should warn that § Solution #3 produces UTF-16 strings, and this might be a non-wanted result.

--madmurphy

[wbamberg]

OK, I finally got around to updating these pages.

I have:

• removed all the solutions except Solution 3, which is relatively short, simple, and uses the native btoa/atob functions rather than reimplementing them. But I've also reworked the code a bit to make it simpler and composable with btoa/atob. I hope I didn't break it in the process!

• moved this version of Solution 3 into the docs page for btoa

• removed some content from the "base64 encoding" page which didn't seem particularly helpful (like linking to Wikipedia). and moved it into the Glossary.

There were some more changes I could have made in here but they seemed out of scope for this issue, and you've got to draw the line somewhere.

@chrisdavidmills , please let me know if you think this covers it, and @madmurphy please let me know if I have broken your code.

[chrisdavidmills]

Looks pretty good to me, @wbamberg, thanks for all your work on this.

A couple of bits I'd like to see:

• Your example also covers atob(), so should we repeat the example on the atob() page too? That currently doesn't have an example.
• Can we get rid of the polyfills too?

[wbamberg]

Can we get rid of the polyfills too?

Well... I decided that was out of scope. The last time I removed a polyfill from a page, someone told me I was vandalising MDN, and I don't feel like getting more abuse like that. Also note that @SphinxKnight already removed it, and someone already reverted their edit: https://wiki.developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/btoa$history?page=2.

So I think if we do remove polyfills (and we should) it should be in the context of a specific project with that goal, that provides links to a better replacement, with an overall rationale that we can point to when people complain. Also probably it should be done after we are in GitHub, so we don't get into edit wars.

Your example also covers atob(), so should we repeat the example on the atob() page too? That currently doesn't have an example.

atob() does have an example: https://wiki.developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/atob#Example. It doesn't repeat the example of encoding Unicode, mostly because this isn't really an issue with atob(): the error is for encoding text. Of course if you choose an approach like the one given, then you do have to reverse-convert after decoding, but I have linked to the description of this issue from the atob() page, and that seemed good enough.

[chrisdavidmills]

@wbamberg OK, all works for me. I'm happy with this.

[wbamberg]

Thanks Chris!