Closed Bug 745425 Opened 9 years ago Closed 9 years ago

Move Utils.encodeBase64url to common Utils

Categories

(Firefox :: Sync, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: anant, Assigned: anant)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file, 2 obsolete files)

The encodeBase64url function currently provided by sync/utils.js is useful to code outside of sync and should be moved to the common Utils module.
Assignee: nobody → anant
Blocks: 744257
Status: NEW → ASSIGNED
All existing occurrences of Utils.encodeBase64url have also been changed to use the common version.
Attachment #615014 - Flags: review?(gps)
Comment on attachment 615014 [details] [diff] [review]
Move encodeBase64url to CommonUtils - v1

It's good enough as-is. If you want to fix a nit, rename to encodeBase64URL (capitalize URL).
Attachment #615014 - Flags: review?(gps) → review+
Fixes nit (url -> URL). Attaching the patch so I can push to try.
Whiteboard: [autoland-try:615067]
Whiteboard: [autoland-try:615067] → [autoland-in-queue]
Autoland Patchset:
	Patches: 615067
	Branch: mozilla-central => try
	Destination: http://hg.mozilla.org/try/pushloghtml?changeset=2074b82a0cf5
Try run started, revision 2074b82a0cf5. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=2074b82a0cf5
Try run for 2074b82a0cf5 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=2074b82a0cf5
Results (out of 15 total builds):
    success: 15
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-2074b82a0cf5
Whiteboard: [autoland-in-queue]
Anant: if you are going to run try builds, please be sure to run the xpcshell tests. You are highly unlikely to break the build by merely modifying JS files.
Comment on attachment 615067 [details] [diff] [review]
Move encodeBase64url to CommonUtils - v1.1

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

OK to land. I would prefer the nit be addressed before landing. No need to request another review for just the nit fix.

::: services/common/utils.js
@@ +58,5 @@
>    /**
> +   * Encode byte string as base64URL (RFC 4648).
> +   */
> +  encodeBase64url: function encodeBase64URL(bytes) {
> +    return btoa(bytes).replace('+', '-', 'g').replace('/', '_', 'g');

Nit: use double quotes.

I know it was like this in the source file. For small things like this, we try to adjust coding styles when you move to the new location. You are breaking blame, so you might as well make yourself look good in the process!
Attachment #615067 - Flags: review+
Rebase patch to s-c.
Attachment #615014 - Attachment is obsolete: true
Attachment #615067 - Attachment is obsolete: true
Attachment #615922 - Flags: review?(gps)
Comment on attachment 615922 [details]
Move encodeBase64url to CommonUtils - v1.2

Already reviewed.
Attachment #615922 - Flags: review?(gps)
[qa-] but I will be smoketesting Sync on s-c to be certain nothing is broken.
Whiteboard: [fixed in services] → [fixed in services][qa-]
https://hg.mozilla.org/mozilla-central/rev/8fc61cdd2a71
https://hg.mozilla.org/mozilla-central/rev/6700372d36d4
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in services][qa-] → [qa-]
Target Milestone: --- → mozilla15
Component: Firefox Sync: Backend → Sync
Product: Cloud Services → Firefox
You need to log in before you can comment on or make changes to this bug.