1226754 - sdk/base64 fails to encode long strings

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect)

Description

[Rob Wu [:robwu]]

Trying to encode a long string, e.g. using

    require('sdk/base64').encode('f'.repeat(1e6))

results in the following error:

    Message: RangeError: too many function arguments

Comment 1

[Rob Wu [:robwu]]

Attachment: Fix + test

Comment 2

[Rob Wu [:robwu]]

Attachment: Enable base64 encoding of long strings

Hi Andy, I moved the patch from Github as requested.

Could you review the patch, or select a responsive reviewer? The one I selected for my previous patch never replied.

Comment 3

[Andy McKay]

I suck I don't know how to change the r? to be jsantell, so cheating and using ni.

Comment 4

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8728634 [details] [diff] [review]
Enable base64 encoding of long strings

Review of attachment 8728634 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM!

Comment 5

[Andy McKay]

Thanks jsantell, rob.

Comment 6

[Carsten Book [:Tomcat]]

has problems to apply:

renamed 1226754 -> 0001-Bug-1226754-Enable-base64-encoding-of-long-strings.patch
applying 0001-Bug-1226754-Enable-base64-encoding-of-long-strings.patch
unable to find 'lib/sdk/base64.js' for patching
2 out of 2 hunks FAILED -- saving rejects to file lib/sdk/base64.js.rej
unable to find 'test/test-base64.js' for patching
2 out of 2 hunks FAILED -- saving rejects to file test/test-base64.js.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh 0001-Bug-1226754-Enable-base64-encoding-of-long-strings.patch

Comment 7

[Rob Wu [:robwu]]

Attachment: Enable base64 encoding of long strings

I reformatted the patch. Previously it was an export from the addon-sdk repo on Github, now it is based on the mozilla-central repo:

$ cd addon-sdk/source
$ git apply 0001-Bug-1226754-Enable-base64-encoding-of-long-strings.patch
$ hg com -m 'Bug 1226754 - Enable base64-encoding of long strings'
$ hg export -o Bug-1226754-Enable-base64-encoding-of-long-strings-hg.patch

Comment 8

[Luca Greco [:rpl] [:luca] [:lgreco]]

I pushed this patch to try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ac462058213

the mochitest-jetpack tests are green, so I'm applying the checkin-needed keyword.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/d8a10fd5b3f6

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d8a10fd5b3f6