Closed Bug 1226754 Opened 4 years ago Closed 4 years ago

sdk/base64 fails to encode long strings

Categories

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

defect
Not set

Tracking

(firefox48 fixed)

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: robwu, Assigned: robwu)

Details

Attachments

(1 file, 2 obsolete files)

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
Attached file Fix + test (obsolete) —
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.
Attachment #8690271 - Attachment is obsolete: true
Attachment #8728634 - Flags: review?(amckay)
I suck I don't know how to change the r? to be jsantell, so cheating and using ni.
Flags: needinfo?(jsantell)
Comment on attachment 8728634 [details] [diff] [review]
Enable base64 encoding of long strings

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

LGTM!
Attachment #8728634 - Flags: review?(amckay) → review+
Flags: needinfo?(jsantell)
Thanks jsantell, rob.
Keywords: checkin-needed
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
Flags: needinfo?(rob)
Keywords: checkin-needed
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
Attachment #8728634 - Attachment is obsolete: true
Flags: needinfo?(rob)
Attachment #8731251 - Flags: review+
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.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/d8a10fd5b3f6
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.