Introduce kinto #241

glasserc · 2017-09-19T20:00:22Z

By popular request, an attempt to introduce kinto.js offline-first syncing to try to simplify conflict handling. This just uses the bog standard IndexedDB "storage adapter", which works fine in web extensions, and happens to have a cleaner implementation than browser.storage.local anyhow.

This PR is a bit of a mess since I just plopped a bunch of code into place, but some additional information is in the commit messages. I tried to at least mark the dangerous places with FIXMEs.

Localization authors: - Cristian Silaghi <[email protected]>

Localization authors: - Hyeonseok Shin <[email protected]>

Localization authors: - Amanpreet Alam <[email protected]>

Localization authors: - mansimarkaur.mks <[email protected]> - Amanpreet Alam <[email protected]>

vladikoff · 2017-09-19T20:09:46Z

awwww yessss

pdehaan · 2017-09-19T20:47:29Z

src/background.js

@@ -12,7 +12,7 @@ const timeouts = {};

 // Kinto sync and encryption

-const client = new KintoClient(KINTO_SERVER);
+const client = new Kinto({remote: KINTO_SERVER, bucket: "default"});


Should the bucket name be configurable?

Also, I think we need to convert from double quotes to single quotes and declare Kinto as a global to appease the ESLint gods:

/home/travis/build/mozilla/notes/src/background.js 15:20 error 'Kinto' is not defined no-undef 15:57 error Strings must use singlequote quotes

UPDATE: You can fix the undefined Kinto global variable by adding it to ./eslintrc.yml (possibly replacing the KintoClient entry):

notes/.eslintrc.yml

Lines 9 to 14 in dca2e9c

globals:

KintoClient: false

Quill: false

Jose: false

JoseJWE: false

TestPilotGA: false

I don't think the bucket should be configurable. The default bucket is "API sugar" to allow each user to get their own bucket while still referring to them in a uniform way. It's the only one which behaves like this.

pdehaan · 2017-09-19T20:50:48Z

src/sync.js

+    this.key = key;
+  }
+
+  async encode(record) {


I think ESLint is choking here and failing to parse the entire file... Should this be async function encode(record) {?

/home/travis/build/mozilla/notes/src/sync.js 63:9 error Parsing error: Unexpected token encode

Ah, is it maybe the case that you guys stick to ES2015 on this project? Or which standard do you support? Per http://kangax.github.io/compat-table/es2016plus/#test-async_functions_async_methods,_classes, this syntax is supported in FF 52 ESR and greater, which is why it's used in the storage.sync implementation, which is why I copied and pasted it here 😊

pdehaan · 2017-09-19T20:51:45Z

src/sync.js

+    return encryptedResult;
+  }
+
+  async decode(record) {


Same here.. should this be async decode(record) {} or async function decode(record) {}?

pdehaan · 2017-09-19T20:53:42Z

src/sync.js

+const notesIdSchema = {
+  // FIXME: Maybe this should generate IDs?
+  generate() {
+    throw new Error("cannot generate IDs");


Once the encode/decode issues are resolved below, I believe ESLint will start complaining about use of double quotes instead of single quotes here. Sorry.

pdehaan · 2017-09-19T20:54:41Z

src/sync.js

+    // headers (If-Match, If-None-Match) correctly.
+    // DON'T copy over "deleted" status, because then we'd leak
+    // plaintext deletes.
+    const status = record._status && (record._status == "deleted" ? "updated" : record._status);


Nit: should use === instead.

pdehaan · 2017-09-19T20:56:07Z

src/sync.js

+      return syncResult;
+    })
+    .catch(error => {
+      if (error.response && error.response.status == 401) {


Nit: use === here.

vladikoff · 2017-09-20T13:30:59Z

@glasserc @Natim Getting result is not defined on an existing account:

Will try a new account now

vladikoff · 2017-09-20T13:40:34Z

Seems to be working for a new account!!!!

vladikoff · 2017-09-20T13:47:04Z

@Natim Seems to working well, i think the biggest thing we need is handling merge conflicts.

There is some minor stuff such as the last sync time not updating:

I synced at :44 on the hour, but still said :41:

But that's small fish

vladikoff · 2017-09-20T14:49:27Z

@Natim seems like after I close 2 profiles that I tested sync on and opened a 3rd profile I didn't get my notes back (got invalid date):

Menu implementation for fixing #208. This commit adds the material-design-lite component library used for creating the menu and it's menu items.

Localization authors: - Théo Chevalier <[email protected]>

Localization authors: - Rok Žerdin <[email protected]>

Localization authors: - Kohei Yoshino <[email protected]>

Localization authors: - Michael Wolf <[email protected]>

Localization authors: - Juan Esteban Ajsivinac Sián <[email protected]>

Localization authors: - Juraj Cigáň <[email protected]>

Localization authors: - Michael Köhler <[email protected]>

Localization authors: - Michael Wolf <[email protected]>

Localization authors: - Balázs Meskó <[email protected]>

Localization authors: - Selim Şumlu <[email protected]>

Localization authors: - manxmensch <[email protected]>

Natim · 2017-10-10T12:06:58Z

First alpha version of the branch: addon-2.0.0a1.zip

Natim · 2017-10-10T12:25:48Z

New version with QuillJS 1.3.3 addon-2.0.0a2.zip

Natim · 2017-10-10T14:09:08Z

src/sync.js

@@ -91,6 +91,9 @@ class JWETransformer {
      throw new Error('No ciphertext: nothing to decrypt?');
    }

+    // FIXME: this is hack to work around a bug with FxA keys, which
+    // seem to have timestamps embedded in them that are affected by
+    // TZ offsets. Take out the substr calls once vladikoff says we can.


Revert "Use mocha spec reporter.", moving from the karma spec reporter back to the karma mocha reporter, just because it seems better supported and is part of the example-webextension standard we're trying to follow. Try to achieve the same effect using the built-in Karma option reportSlowerThan -- set it to a generous 40ms. This reverts commit 54299d2.

vladikoff · 2017-10-10T22:41:09Z

src/background.js

@@ -6,7 +6,7 @@ const TRACKING_ID = 'UA-35433268-79';
 const KINTO_SERVER = 'https://kinto.dev.mozaws.net/v1';
 // XXX: Read this from Kinto fxa-params
 const FXA_CLIENT_ID = 'c6d74070a481bc10';
-const FXA_OAUTH_SERVER = 'https://oauth-scoped-keys.dev.lcip.org/v1';
+const FXA_OAUTH_SERVER = 'https://oauth-scoped-keys-oct10.dev.lcip.org/v1';


@Natim @glasserc please update the kinto dev server to use this now. If this server is not working for you while I'm on PTO please rollback this commit :)

This has the fixes to the kid and other stuff

Natim · 2017-10-12T15:50:39Z

Since this PR is against sync-v4, I am going to land it and then we can keep improving sync-v4 branch until it is ready.

sn-o-w and others added 7 commits September 7, 2017 19:51

Pontoon: Update Romanian (ro) localization of Test Pilot: Notes

cf6acc9

Localization authors: - Cristian Silaghi <[email protected]>

Pontoon: Update Romanian (ro) localization of Test Pilot: Notes

c83c6f1

Localization authors: - Cristian Silaghi <[email protected]>

Pontoon: Update Korean (ko) localization of Test Pilot: Notes

5dd5e9d

Localization authors: - Hyeonseok Shin <[email protected]>

Pontoon: Update Korean (ko) localization of Test Pilot: Notes

dac61da

Localization authors: - Hyeonseok Shin <[email protected]>

Pontoon: Update Punjabi (pa-IN) localization of Test Pilot: Notes

a3c02be

Localization authors: - Amanpreet Alam <[email protected]>

Pontoon: Update Punjabi (pa-IN) localization of Test Pilot: Notes

dca2e9c

Localization authors: - mansimarkaur.mks <[email protected]> - Amanpreet Alam <[email protected]>

Add kinto.js to build

251dd65

pdehaan reviewed Sep 19, 2017

View reviewed changes

cedricium and others added 12 commits September 21, 2017 11:01

Context Menu for Footer (#240) r=vladikoff

fce284b

Menu implementation for fixing #208. This commit adds the material-design-lite component library used for creating the menu and it's menu items.

Pontoon: Update French (fr) localization of Test Pilot: Notes

c511d89

Localization authors: - Théo Chevalier <[email protected]>

Pontoon: Update Slovenian (sl) localization of Test Pilot: Notes

c164456

Localization authors: - Rok Žerdin <[email protected]>

Pontoon: Update Japanese (ja) localization of Test Pilot: Notes

ac8d755

Localization authors: - Kohei Yoshino <[email protected]>

Pontoon: Update Sorbian, Upper (hsb) localization of Test Pilot: Notes

f035430

Localization authors: - Michael Wolf <[email protected]>

Pontoon: Update Kaqchikel (cak) localization of Test Pilot: Notes

581c544

Localization authors: - Juan Esteban Ajsivinac Sián <[email protected]>

Pontoon: Update Slovak (sk) localization of Test Pilot: Notes

245088f

Localization authors: - Juraj Cigáň <[email protected]>

Pontoon: Update German (de) localization of Test Pilot: Notes

1d2b060

Localization authors: - Michael Köhler <[email protected]>

Pontoon: Update Sorbian, Lower (dsb) localization of Test Pilot: Notes

91cfbfc

Localization authors: - Michael Wolf <[email protected]>

Pontoon: Update Hungarian (hu) localization of Test Pilot: Notes

fe2c83b

Localization authors: - Balázs Meskó <[email protected]>

Pontoon: Update Turkish (tr) localization of Test Pilot: Notes

c5d72a8

Localization authors: - Selim Şumlu <[email protected]>

Pontoon: Update Malay (ms) localization of Test Pilot: Notes

c725254

Localization authors: - manxmensch <[email protected]>

Natim mentioned this pull request Oct 10, 2017

Bring back FxA login using the crypto relier library #141

Closed

1 task

Natim added this to the Sprint Dos milestone Oct 10, 2017

Upgrade Quill to 1.3.3

b03c9e0

Natim mentioned this pull request Oct 10, 2017

"Undo" option doesn't work properly on previously formatted text #260

Closed

Natim mentioned this pull request Oct 10, 2017

Upgrade to Quill 1.3.0 #158

Closed

1 task

glasserc and others added 2 commits October 10, 2017 09:14

Document why we put this in here

5020d2b

Fix menu wording.

a597663

Natim reviewed Oct 10, 2017

View reviewed changes

Rémy HUBSCHER and others added 5 commits October 10, 2017 16:41

Upgrade test timeout.

16207cf

Use mocha spec reporter.

54299d2

Update readme

865b947

BREAKING CHANGE: change servers to oauth-scoped-keys-oct10.dev.lcip.org

38f0643

vladikoff reviewed Oct 10, 2017

View reviewed changes

glasserc added 3 commits October 12, 2017 11:27

Failing test demonstrating how conflicts should be handled

15a4445

Handle conflicts

37e0470

No longer need to hack around tz offset bug

deda478

Natim merged commit 38f26b9 into mozilla:sync-v4 Oct 12, 2017

glasserc deleted the introduce-kinto branch October 12, 2017 15:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce kinto #241

Introduce kinto #241

glasserc commented Sep 19, 2017

vladikoff commented Sep 19, 2017

pdehaan Sep 19, 2017

pdehaan Sep 19, 2017 •

edited

Loading

glasserc Sep 20, 2017

pdehaan Sep 19, 2017 •

edited

Loading

glasserc Sep 20, 2017

pdehaan Sep 19, 2017

pdehaan Sep 19, 2017

pdehaan Sep 19, 2017

pdehaan Sep 19, 2017

vladikoff commented Sep 20, 2017

vladikoff commented Sep 20, 2017

vladikoff commented Sep 20, 2017

vladikoff commented Sep 20, 2017

Natim commented Oct 10, 2017

Natim commented Oct 10, 2017

Natim Oct 10, 2017

vladikoff Oct 10, 2017

vladikoff Oct 10, 2017

Natim commented Oct 12, 2017

	globals:
	KintoClient: false
	Quill: false
	Jose: false
	JoseJWE: false
	TestPilotGA: false

Introduce kinto #241

Introduce kinto #241

Conversation

glasserc commented Sep 19, 2017

vladikoff commented Sep 19, 2017

Choose a reason for hiding this comment

pdehaan Sep 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pdehaan Sep 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vladikoff commented Sep 20, 2017

vladikoff commented Sep 20, 2017

vladikoff commented Sep 20, 2017

vladikoff commented Sep 20, 2017

Natim commented Oct 10, 2017

Natim commented Oct 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Natim commented Oct 12, 2017

pdehaan Sep 19, 2017 •

edited

Loading

pdehaan Sep 19, 2017 •

edited

Loading