434818 - Sync saved form data

Status: RESOLVED FIXED

(Cloud Services :: General, defect, P1)

Description

[Dan Mills [:thunder]]

Weave should sync saved form data.

Comment 1

[Anant Narayanan [:anant]]

This *may* need some extra work in order to retrieve all keys/values stored in formhistory.sqlite since the nsIHistory interface doesn't expose a getAllEntries() method like nsILoginManager. It does. however, have a AutoCompleteSearch method, maybe giving wild-cards for both the key and value fields returns the whole set...

Comment 2

[Anant Narayanan [:anant]]

s/nsIHistory/nsIFormHistory2 (comment #1)

Comment 4

[Anant Narayanan [:anant]]

Attachment: Add form data (and search bar history) data type

The same database (formhistory.sqlite) is used to store form data as well as search-bar values, so we get both these with the same data type. Do we still want the user to have the option of syncing form data but not search bar history or vice-versa?

Comment 5

[Justin Dolske [:Dolske]]

Comment on attachment 322131 [details] [diff] [review]
Add form data (and search bar history) data type

>+    while (stmnt.executeStep()) {
>+      var nam = stmnt.getUTF8String(1);
>+      var val = stmnt.getUTF8String(2);
>+      var key = Utils.sha1(nam + val);

Hmm, not really sure hashing makes sense here. It might make more sense to use the field name as the key, and the values an array of history items stored for that name? Eg, sync some data structure like:

{
  "somefield" : [
                   "lol cats",
                   "kittens",
                   "lions"
                ],

  "otherfield" : [
                   "beer"
                 ],

  ...
}

This would also require supporting edit commands, but syncs would be a bit more efficient as a result?

Comment 6

[Anant Narayanan [:anant]]

I'm not sure if that would really be efficient, because most forms have same field names. Especially in the case of search-history, there's only one key, but everytime a small change is made, you'd have to transmit the whole array inside an edit command, instead of just an add or remove.

Basically it's a trade-off between processing time (SHA1) and bulkiness of data (many small adds/removes instead of one huge edit) on every sync.

Also, if we move to a single-object-addressable model in the future, the current behavior would be preferred. Dan?

Comment 7

[Dan Mills [:thunder]]

First, just a nitpick - weave objects (each thing referenced by a guid) should be a non-array object.  So it would really look like:

{ 'foo': { values: ["str1", "str2", ...]}}

Nevertheless, the problem that Anant described is true, this structure will get us edits for the entire array (with the whole array repeated) every time there's an addition/substraction from it.

Since there is no other metadata (unlike in the case of bookmarks & tags, for example), we can flatten out the structure some more and have each object represent a key-value pair.  This way we'll mostly get adds and deletes, just for the data we're adding/deleting.

Comment 8

[Dan Mills [:thunder]]

Comment on attachment 322131 [details] [diff] [review]
Add form data (and search bar history) data type

Looks good.  I'm not sure if you should throw in the edit command:

+    throw "Form syncs are expected to only be create/remove!";

Maybe only this._log.error() ?  Throwing will cause sync to stop for that engine, and I'm not sure that's what we want.

Comment 9

[Dan Mills [:thunder]]

Comment on attachment 322131 [details] [diff] [review]
Add form data (and search bar history) data type

Or, maybe even warn().
r+ otherwise.

Comment 10

[Anant Narayanan [:anant]]

Checked-in revision 357.

Comment 11

[Ayan Shah[:ashah]]

Investigating this bug for being potential crossweave automation test case
candidate.