1071536 - Don't ship a JS polyfill of the Encoding Standard as part of Loop; use the built-in impl.

Status: RESOLVED INCOMPLETE

(Hello (Loop) :: Client, defect, P4)

Description

[Henri Sivonen (:hsivonen)]

It appears that the Loop client includes, as part of a library, a JS polyfill of the Encoding Standard:
http://mxr.mozilla.org/mozilla-central/source/browser/components/loop/content/shared/libs/sdk.js#9210

In the interest of avoiding both download and runtime bloat, Loop should use the built-in TextDecoder/TextEncoder API instead.

(Also, since Loop is a new thing, hopefully it doesn't need to handle encodings other than UTF-8.)

Comment 1

[Anne (:annevk)]

If Loop does need to handle encodings other than utf-8, I would love to see that explained somewhere.

Comment 2

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

(In reply to Henri Sivonen (:hsivonen) from comment #0)
> It appears that the Loop client includes, as part of a library, a JS
> polyfill of the Encoding Standard:
> http://mxr.mozilla.org/mozilla-central/source/browser/components/loop/
> content/shared/libs/sdk.js#9210

This is part of the Tokbox SDK, which is third party code shipping along with Firefox. They most likely use a polyfill because this SDK should be useable on other platforms than Firefox. Options here are:

- Patch the library code and maintain it over releases;
- Get in touch with TB to see if we could have special builds with this polyfill not included;
- Leave it that way.

While 2 might be the smartest option, /me votes for option 3 because it's simple. Note that both could be made concurrently.

Comment 3

[Mark Banner (:standard8)]

(In reply to Nicolas Perriault (:NiKo`) — needinfo me if you need my attention from comment #2)
> (In reply to Henri Sivonen (:hsivonen) from comment #0)
> - Get in touch with TB to see if we could have special builds with this
> polyfill not included;

There's hidden consequences here. This means we'd need to be supplied and manage two versions of the library - one for our standalone code (which has to run on any browser), and one for desktop.

Additionally, (correct me if I am wrong) TextEncoder/TextDecoder aren't available to content-space web pages, so we'd have to punch a whole through MozLoopAPI to be able to use them.

Comment 4

[Anne (:annevk)]

You're wrong.

Comment 5

[Henri Sivonen (:hsivonen)]

(In reply to Nicolas Perriault (:NiKo`) — needinfo me if you need my attention from comment #2)
> They most likely use a polyfill because this SDK should be
> useable on other platforms than Firefox.

What browsers support WebRTC but don't support TextEncoder/TextDecoder?

(In reply to Mark Banner (:standard8) from comment #3)
> Additionally, (correct me if I am wrong) TextEncoder/TextDecoder aren't
> available to content-space web pages, so we'd have to punch a whole through
> MozLoopAPI to be able to use them.

(Anne was replying to this. They are available to Web content.)

Comment 6

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

(In reply to Henri Sivonen (:hsivonen) from comment #5)
> What browsers support WebRTC but don't support TextEncoder/TextDecoder?

I don't know, this is something to ask to the Tokbox frontend engineering team, as it's their library. NI Songz here.

Comment 7

[song]

1. The polyfill is only used when native implementation is not available so runtime bloat should be minimal.

2. The polyfill is used for backwards compatibility with IE ie browsers.

2. The polyfill will probably be removed in the near future because we plan to remove our binary rumor protocol. 

I think niko's 3rd suggestion is best for the time being (leave it that way) until we remove our binary rumor protocol.

Comment 8

[Henri Sivonen (:hsivonen)]

(In reply to song from comment #7)
> 1. The polyfill is only used when native implementation is not available so
> runtime bloat should be minimal.

Do you use the non-UTF-8 parts? If so, why? If not, seems rather wasteful to include them.

> I think niko's 3rd suggestion is best for the time being (leave it that way)
> until we remove our binary rumor protocol.

Not knowing the time frame for that, I guess all I can say is that as a developer, it's quite demotivating to spend time removing dead code from Gecko only to see dead code shipped loosely in the JS parts of Firefox and, assuming that Loop client code ends up on Firefox OS, as a Firefox OS user, it's pretty demotivating to see the keyboard layout for my language excluded from builds over size concerns and then dead code is included loosely elsewhere.

Comment 9

[song]

We did strip out the other encodings, the definitions are still in there. We could strip those out as well and save a couple of hundred lines of code or so. Again, we also plan to remove this library completely in the near future...

Comment 10

[Mark Banner (:standard8)]

Support for Hello/Loop has been discontinued.

https://support.mozilla.org/kb/hello-status

Hence closing the old bugs. Thank you for your support.