Closed
Bug 1226754
Opened 8 years ago
Closed 8 years ago
sdk/base64 fails to encode long strings
Categories
(Add-on SDK Graveyard :: General, defect)
Add-on SDK Graveyard
General
Tracking
(firefox48 fixed)
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: robwu, Assigned: robwu)
Details
Attachments
(1 file, 2 obsolete files)
3.05 KB,
patch
|
robwu
:
review+
|
Details | Diff | Splinter Review |
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
Assignee | ||
Comment 1•8 years ago
|
||
Assignee | ||
Comment 2•8 years ago
|
||
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)
Comment 3•8 years ago
|
||
I suck I don't know how to change the r? to be jsantell, so cheating and using ni.
Flags: needinfo?(jsantell)
Comment 4•8 years ago
|
||
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+
Updated•8 years ago
|
Flags: needinfo?(jsantell)
Comment 6•8 years ago
|
||
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
Assignee | ||
Comment 7•8 years ago
|
||
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+
Comment 8•8 years ago
|
||
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
Comment 10•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d8a10fd5b3f6
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•