STR: Navigate to site with a form field (I have been using www.ryerson.ca/JavaScript/lectures/forms/textinput.html). Enter data. Refresh page; observer form history correctly remembered locally. Sync. Witness first log showing form history propagating to server. Either clear form history in preferences or delete form history item using https://addons.mozilla.org/en-US/firefox/addon/form-history-control/. Observe form history data correctly forgotten locally. Sync. Witness second log showing no form history changes propagating to server. (I have also checked with in-development extensions to Android Sync and see no form history records marked as deleted being downloaded.)

Just as with Bug 578694, we don't act on the appropriate notifications. case "satchel-storage-changed": if (data == "addEntry" || data == "before-removeEntry") { vs. removeAllEntries : function () { this.log("removeAllEntries"); this.sendNotification("before-removeAllEntries", null); and removeEntriesForName : function (name) { this.log("removeEntriesForName with name=" + name); this.sendStringNotification("before-removeEntriesForName", name);

Nick, please enable trace logging for the forms engine and retry your experiment. You should see the FormsTracker tracking the form history entries as you remove them, and then sync them. Might also be a good idea to add some more logging information here: https://mxr.mozilla.org/mozilla-central/source/services/sync/modules/engines/forms.js#321

I see nothing in the trace logs about FormsTracker (maybe I'm looking in the wrong place) and I see no deletes propagating.

(In reply to Philipp von Weitershausen [:philikon] from comment #4) > Nick, please enable trace logging for the forms engine and retry your > experiment. You should see the FormsTracker tracking the form history > entries as you remove them, and then sync them. nsFormHistory.removeAllEntries does not notify before-removeEntry for each item. It sends before-removeAllEntries, then clears the DB, then notifies removeAllEntries. Sync isn't watching for bulk deletes.

This handles removeAllEntries the hard way. There is a much more efficient approach. And I only stubbed out the other possible deletion pathways. But this seems to work. 1331697607251 Sync.Tracker.Forms DEBUG Notified of removeAllEntries. ... 1331697607687 Sync.Engine.Forms INFO 1 outgoing items pre-reconciliation 1331697607688 Sync.Engine.Forms INFO Records: 0 applied, 0 successfully, 0 failed to apply, 0 newly failed to apply, 0 reconciled. 1331697607692 Sync.Engine.Forms INFO Uploading all of 1 records 1331697607693 Sync.Collection DEBUG POST Length: 253

fyi on STR's you can clear any single form history item by highlighting it in the actual form field drop down list, then click option(alt) + shift + delete. Will dupe bug I filed on this nearly 2 years ago.

Comment on attachment 605648 [details] [diff] [review] WIP. v1 Review of attachment 605648 [details] [diff] [review]: ----------------------------------------------------------------- I'm cancelling the feedback because 1) there is lots more to do (including possibly a more proper solution per your comment) and 2) I don't fully grok the API yet. I'll happily review something that's more complete. ::: services/sync/modules/engines/forms.js @@ +308,5 @@ > this.score += SCORE_INCREMENT_MEDIUM; > }, > > _enabled: false, > observe: function observe(subject, topic, data) { Uncaught exceptions in observers scare me. @@ +364,5 @@ > + if (data == "before-removeEntriesByTimeframe") { > + subject = subject.QueryInterface(Ci.nsIArray); > + > + // Timestamps in microseconds. > + let begin = subject.queryElementAt(0, Ci.nsISupportsPRInt64).data; I'm not entirely sure if queryElementAt will throw (like QueryInterface does) if the QI is not valid. I would assume so. However, https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIArray#queryElementAt%28%29 also says that the return may be null. Do we need to guard against null access?

To try and ensure this gets some attention, I'm blocking our generic "sync moar things" bug...

Thanks, Mark. One could argue that this is a bit of a security risk as it's easy to mistype a password in a username field. With this bug, you can't get rid of it. If someone is standing over your shoulder, they can see the password briefly in the dropdown of previously entered values.

Note that single deletions (eg, using the "delete" key on the popup) is being done in bug 1395427, so this bug remains for the various "bulk" operations.

I think we're going to go with a different approach in bug 1404427, storing tombstones for cleared entries (or, alternatively, date ranges) in a separate SQL table instead of relying on observers.