907594 - Pref-change observer isn't being called.

Status: RESOLVED FIXED

(Toolkit :: Password Manager, defect)

Description

[Justin Dolske [:Dolske]]

Attachment: Patch v.1

Found while debugging bug 355063 comment 62. From IRC discussion it seems this is likely a GC issue -- the object returned by getBranch() gets GC'd and takes the observer with it. I was assuming it was just exposing some underlying structure in the native libpref code. :(

Fix is simple, will probably want to uplift this.

Comment 1

[Justin Dolske [:Dolske]]

https://hg.mozilla.org/integration/fx-team/rev/e0f58fd2d9ba

Comment 2

[Justin Dolske [:Dolske]]

Comment on attachment 793360 [details] [diff] [review]
Patch v.1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 839961 introduced this code
User impact if declined: Main impact is that the "remember passwords" UI checkbox is no longer live, changes to it wouldn't take effect until restarting the browser. Also notable is that the pref to enable debug logging is also affected in the same way, which is slightly annoying for getting data from bug reporters.
Testing completed (on m-c, etc.):  Manually in the course of debugging the problem, otherwise password manager is well covered by tests.
Risk to taking this patch (and alternatives if risky): Extremely low. It's trivial, and restores functionality that was working before.
String or IDL/UUID changes made by this patch: None

Comment 3

[Ed Morley [:emorley]]

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

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bc01472dd9b5
https://hg.mozilla.org/releases/mozilla-beta/rev/ecb5878a1717

Comment 5

[u279076]

Assuming no QA needed here. Please remove [qa-] from the whiteboard and add the verifyme keyword if this needs QA.