1429976 - formautofill-storage-changed observer notification doesn't indicate which collection was changed

Status: RESOLVED FIXED

(Toolkit :: Form Autofill, enhancement)

Description

[Matthew N. [:MattN]]

Consumers can't easily distinguish between address vs. payment card changes which may lead to unnecessary works.

For example, if a credit card is changed, Sync may also think it should sync addresses even though they're not dirty.

This also affects UI that add observers, like about:preferences and the PaymentRequest dialog.

We should probably include the collection name in the topic or include it in the subject object.

Comment 1

[Ray Lin[:ralin]]

Take the bug. I think including the collection name in subject might be easier.

Comment 2

[Ray Lin[:ralin]]

Hi Thom,

I find FormautofillTracker is currently the only place use subject in observe callback, would adding additional property on it affect any part of sync? Thanks :D

Comment 3

[Ray Lin[:ralin]]

Attachment: Bug 1429976 - Include collectionName in formautofill-storage-changed notify payload.

Review commit: https://reviewboard.mozilla.org/r/213050/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/213050/

Comment 4

[Ray Lin[:ralin]]

Comment on attachment 8942804 [details]
Bug 1429976 - Include collectionName in formautofill-storage-changed notify payload.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/213050/diff/1-2/

Comment 5

[Ray Lin[:ralin]]

Comment on attachment 8942804 [details]
Bug 1429976 - Include collectionName in formautofill-storage-changed notify payload.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/213050/diff/2-3/

Comment 6

[Thom Chiovoloni [:tcsc] (ex-moco)]

> I find FormautofillTracker is currently the only place use subject in observe callback, would adding additional property on it affect any part of sync?

It shouldn't. Eventually we might want to use it to avoid bumping e.g. the address tracker's score if credit cards change, but that could be done as a followup.

Comment 7

[Ray Lin[:ralin]]

That's good to know, I'll file a bug for the followup. Thanks again.

Comment 8

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8942804 [details]
Bug 1429976 - Include collectionName in formautofill-storage-changed notify payload.

https://reviewboard.mozilla.org/r/213050/#review219136

Looks good. Thanks.

Comment 9

[Ray Lin[:ralin]]

Thank you!

I don't see any failure on TH: https://treeherder.mozilla.org/#/jobs?repo=try&revision=733fe255aff4f7c894a3fb94aa42c982413c073b, let's land it :D

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/df2421b0dbe5
Include collectionName in formautofill-storage-changed notify payload. r=lchang

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/df2421b0dbe5