1346850 - guid in FormHistory notification "formhistory-remove" is always null

Status: RESOLVED DUPLICATE of bug 1395427

(Toolkit :: Form Manager, defect, P3)

Description

[Frank-Rainer Grahl (:frg)]

+++ This bug was initially created as a clone of Bug #876002 +++

In Bug 878677 the notification "formhistory-remove" was changed to have a guid as a parameter:

https://hg.mozilla.org/mozilla-central/annotate/9126675b827a/toolkit/components/satchel/FormHistory.jsm#l646

Unfortunately the guid is always null in Desktop Firefox as far as I can see. So after removal of the sync formhistory api its inpossible for add-ons or other code to know what specific form entry was removed.

In FormHistory.jsm the actual remove function makeRemoveStatement will fall back to use fieldname and value if the provided guid is null/undefined so no error is reported. Tests seem not to use or cover the returned guid either. 

If this is confirmed and can't be fixed easily maybe fieldname, value and guid should be returned by the notification. The sync api nsIFormHistory2 did rjust do this in a "removeEntry" Notification.

Comment 1

[Frank-Rainer Grahl (:frg)]

Attachment: formhistoryguildtest.patch

Patch to print the guild before and after removal.

In the Browser console I just get:

> FormHistory: updateFormHistoryWrite  1
> FormHistory: Remove from form history  [object Object]
> FormHistory: Remove from form history before guid is null!
> FormHistory: removeEntries
> FormHistory: Creating new statement for query: DELETE FROM moz_formhistory WHERE 
>              fieldname = :fieldname AND value = :value
> FormHistory: Remove from form history after guid is null!

Comment 2

[Mark Hammond [:markh] [:mhammond]]

It's relatively easy to get the GUID for items deleted from an auto-complete list - however, in the more general case, there's alot of code that explicitly calls FormHistory.update() with a change that looks like, say, {op: "remove", name: "field name", value: "field value"}, which ends up being turned into a SQL statement like |DELETE FROM moz_formhistory WHERE name = "field name" AND value = "field value"| - or sometimes just specifying a date field for when clearing recent history. In these cases the GUID isn't known, so to get this right we'd probably need to turn that into a query for GUIDs, then issue the delete against the resulting set - but this is likely to be fairly inefficient in many cases.

The end result of all this is that Sync can't sanely sync such removals from other devices.

Comment 3

[Mark Hammond [:markh] [:mhammond]]

Attachment: Patch that sends a GUID on deletion from the popup

FWIW, this patch works only when pressing the "del" key in a popup, but not in other common cases. The test change fails for this reason.