Re-implement form sync engine for 0.3

RESOLVED FIXED in Future

Status

Cloud Services
General
P3
normal
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: Jono Xia, Assigned: anant)

Tracking

unspecified
Future
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

9 years ago
What it says.  We've got an older modules/engines/forms.js that can serve as a starting point.

Updated

9 years ago
Blocks: 480694
Priority: -- → P3
Target Milestone: -- → 0.3

Comment 1

9 years ago
Punting this to Future
Target Milestone: 0.3 → Future

Updated

9 years ago
No longer blocks: 480694
(Assignee)

Comment 2

9 years ago
Done: http://hg.mozilla.org/labs/weave/rev/afc084b21a86

nsIFormSubmitObserver was made scriptable via bug #370561, so we now use it in the tracker. The tracker is still a little convoluted though because we don't have GUID support in nsIFormHistory2.

The GUID for a record is still SHA1 of the name:value pair - making the update() and changeItemID() methods without much meaning (hence unimplemented). Performance-wise, this is better than the earlier version because of the new tracker, worked fine in my tests (desktop).

Form sync is still turned off by default in the preferences, pending comments from Dan.
Assignee: nobody → anant
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Comment 3

9 years ago
Why not just use a GUID for the GUID? recordLike should handle duplicate records.

Comment 4

9 years ago
(In reply to comment #3)
> Why not just use a GUID for the GUID? recordLike should handle duplicate
> records.

Because the form db doesn't store GUIDs.  e.g., Store.update(id) needs to know how to look up 'id' in the database and modify it.

(In reply to comment #2)
> Form sync is still turned off by default in the preferences, pending comments
> from Dan.

Clearly I should've commented here, I can't remember what I said ;)

I think using hashes for IDs is very suboptimal, and potentially a privacy problem.

Comment 5

9 years ago
(In reply to comment #4)
> Because the form db doesn't store GUIDs
I wonder if weave should have its own sqlite table. We currently reuse the places db for history weave/guid with annotations. Something like.. string guid <-> string data

where history data is the place id.. form data is name,value ? json([name,value]) ?
(Assignee)

Comment 6

9 years ago
(In reply to comment #5)
> I wonder if weave should have its own sqlite table.

An alternative is to convince the FF team to add GUID support in FormHistory. We did get that done for the Password Manager and it worked well for everyone :)

Speaking of our own tables, I had an idea sometime last summer: syncing sqlite databases without the engine being aware of its contents . There are some nice DB synchronization algorithms out there... but I figured it would be too much of a departure from our existing model.

Comment 7

9 years ago
In general, it is better for the component that keeps the data to decide what makes something unique.  The risk of having Weave keep its own ID mappings is that it needs to be guarantee uniqueness over data it doesn't fully control.

For example, for passwords uniqueness would be defined by realm (if set), formURL (if set), and username.  And what if the underlying store changes over time?  It's a solvable problem, but it's an extra source of bugs.

That said, there is an advantage to having a built-in system for handling arbitrary ID-less data.  Let's keep discussing this outside of this bug :)

Updated

9 years ago
Blocks: 487543

Updated

9 years ago
Blocks: 487541
No longer blocks: 487543

Updated

9 years ago
Component: Weave → General
Product: Mozilla Labs → Weave
QA Contact: weave → general
You need to log in before you can comment on or make changes to this bug.