Move Utils.encodeBase64url to common Utils

RESOLVED FIXED in mozilla15

Status

Cloud Services
Firefox Sync: Backend
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: anant, Assigned: anant)

Tracking

unspecified
mozilla15
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 years ago
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)

Updated

6 years ago
Assignee: nobody → anant
Blocks: 744257
Status: NEW → ASSIGNED
(Assignee)

Comment 1

6 years ago
Created attachment 615014 [details] [diff] [review]
Move encodeBase64url to CommonUtils - v1

All existing occurrences of Utils.encodeBase64url have also been changed to use the common version.
Attachment #615014 - Flags: review?(gps)

Comment 2

6 years ago
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+
(Assignee)

Comment 3

6 years ago
Created attachment 615067 [details] [diff] [review]
Move encodeBase64url to CommonUtils - v1.1

Fixes nit (url -> URL). Attaching the patch so I can push to try.
(Assignee)

Updated

6 years ago
Whiteboard: [autoland-try:615067]

Updated

6 years ago
Whiteboard: [autoland-try:615067] → [autoland-in-queue]

Comment 4

6 years ago
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

Comment 5

6 years ago
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

Updated

6 years ago
Whiteboard: [autoland-in-queue]

Comment 6

6 years ago
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 7

6 years ago
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+
(Assignee)

Comment 8

6 years ago
Created attachment 615922 [details]
Move encodeBase64url to CommonUtils - v1.2

Rebase patch to s-c.
Attachment #615014 - Attachment is obsolete: true
Attachment #615067 - Attachment is obsolete: true
Attachment #615922 - Flags: review?(gps)
https://hg.mozilla.org/services/services-central/rev/6700372d36d4

Simple typo in test_prefs_tracker.js fixed.
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
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in services][qa-] → [qa-]
Target Milestone: --- → mozilla15
You need to log in before you can comment on or make changes to this bug.