Closed
Bug 838877
Opened 12 years ago
Closed 12 years ago
Change policy handling to permit direct pref modification
Categories
(Firefox Health Report Graveyard :: Client: Desktop, defect)
Firefox Health Report Graveyard
Client: Desktop
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 21
People
(Reporter: rnewman, Assigned: rnewman)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
gps
:
review+
|
Details | Diff | Splinter Review |
We recently changed this so that enabling or disabling went through a function call, so that we can schedule a remote deletion.
This isn't well suited for Android: we don't want to ship the JS Bagheera client, the Gecko side of FHR won't even know the document ID, and our life is easier if we can mirror the pref directly.
We'd like to switch to using pref observers, which also buys us correct behavior if a user pokes around in about:config.
The function call can stay for convenience access.
Assignee | ||
Comment 2•12 years ago
|
||
Watch the upload enablement pref, deleting accordingly.
I think this is the only pref we change from the UI that requires observation. Might find more as we go.
Assignee | ||
Comment 3•12 years ago
|
||
Note that we don't have an end-of-lifecycle component for the policy object, so it doesn't unhook its observer.
Assignee | ||
Comment 4•12 years ago
|
||
The previous version broke a single test, which was expecting pref changes to not cause a deletion -- an outstanding deletion will cause updates to be delayed.
Attachment #712640 -
Attachment is obsolete: true
Attachment #712640 -
Flags: review?(gps)
Attachment #713156 -
Flags: review?(gps)
Comment 5•12 years ago
|
||
Comment on attachment 713156 [details] [diff] [review]
Proposed patch. v2
Review of attachment 713156 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good to me.
::: services/datareporting/policy.jsm
@@ +710,5 @@
> + let result = null;
> + if (!flag) {
> + result = this.deleteRemoteData(reason);
> + }
> +
If you are going to change style only, please don't introduce leading whitespace :P
Attachment #713156 -
Flags: review?(gps) → review+
Assignee | ||
Comment 6•12 years ago
|
||
https://hg.mozilla.org/services/services-central/rev/430b8f9868f8
There might be more once Chenxia's done with UX.
Whiteboard: [fixed in services]
Assignee | ||
Comment 7•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in services]
Target Milestone: --- → mozilla21
Updated•12 years ago
|
Component: Metrics and Firefox Health Report → Client: Desktop
Product: Mozilla Services → Firefox Health Report
Target Milestone: mozilla21 → Firefox 21
Updated•6 years ago
|
Product: Firefox Health Report → Firefox Health Report Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•