745425 - Move Utils.encodeBase64url to common Utils

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Anant Narayanan [:anant]]

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.

Comment 1

[Anant Narayanan [:anant]]

Attachment: Move encodeBase64url to CommonUtils - v1

All existing occurrences of Utils.encodeBase64url have also been changed to use the common version.

Comment 2

[Gregory Szorc [: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).

Comment 3

[Anant Narayanan [:anant]]

Attachment: Move encodeBase64url to CommonUtils - v1.1

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

Comment 4

[Mozilla RelEng Bot]

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

[Mozilla RelEng Bot]

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/[e-mail]-2074b82a0cf5

Comment 6

[Gregory Szorc [:gps]]

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

[Gregory Szorc [:gps]]

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!

Comment 8

[Anant Narayanan [:anant]]

Attachment: Move encodeBase64url to CommonUtils - v1.2

Rebase patch to s-c.

Comment 9

[Anant Narayanan [:anant]]

https://hg.mozilla.org/services/services-central/rev/8fc61cdd2a71

Comment 10

[Gregory Szorc [:gps]]

https://hg.mozilla.org/services/services-central/rev/6700372d36d4

Simple typo in test_prefs_tracker.js fixed.

Comment 11

[Gregory Szorc [:gps]]

Comment on attachment 615922 [details]
Move encodeBase64url to CommonUtils - v1.2

Already reviewed.

Comment 12

[Tracy Walker [:tracy]]

[qa-] but I will be smoketesting Sync on s-c to be certain nothing is broken.

Comment 13

[Gregory Szorc [:gps]]

https://hg.mozilla.org/mozilla-central/rev/8fc61cdd2a71
https://hg.mozilla.org/mozilla-central/rev/6700372d36d4