Closed
Bug 804745
Opened 12 years ago
Closed 12 years ago
Add info bar notification for Firefox Health Report
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 21
Tracking | Status | |
---|---|---|
firefox20 | --- | fixed |
People
(Reporter: gps, Assigned: gps)
References
(Depends on 1 open bug)
Details
Attachments
(4 files, 8 obsolete files)
12.88 KB,
image/png
|
Details | |
689.45 KB,
image/png
|
Details | |
20.15 KB,
patch
|
Gavin
:
review+
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
3.27 KB,
patch
|
rnewman
:
review+
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
We need to add an info bar to Firefox to notify users about data collection practices for Firefox Health Report.
Product Requirements:
* Info bar is displayed on every tab on every window
* Info bar is displayed until user action is taken.
* Info bar does not need to persist across application restarts
Exact content of info bar is TBD (probably by Privacy Tom). I'd like to land this ASAP so we get it out of the way and so l10n folks can have a go at it.
Assignee | ||
Comment 1•12 years ago
|
||
Tom/Privacy needs to provide content for info bar.
Flags: needinfo?(tom)
Comment 2•12 years ago
|
||
Content! I think this is the right ballpark, but others should feel free to chime in. "Learn more." is meant to be a button.
$PRODUCTNAME automatically sends some info to Mozilla so that we can improve Firefox. You can opt out. [Learn More.]
Flags: needinfo?(tom)
Comment 3•12 years ago
|
||
(In reply to Tom Lowenthal [:StrangeCharm] from comment #2)
> $PRODUCTNAME automatically sends some info to Mozilla so that we can improve
> Firefox. You can opt out. [Learn More.]
Looks good to me, but two questions:
• Static "Firefox" should be parameterized, no?
• Do we want an opt-out function right there in the bar? ("No, thanks"?)
… and it might be time to start thinking about how to present this on Android, too. Don't want to get spammed with doorhangers. Tom and Ian, start your engines! :D
Sorry but, I think the state of opt out should be thought over. The user should be offered an option to participate, as in opt in not opt out. Same as what we do for telemetry to day.
If for no other reason then politeness toward the user.
Comment 5•12 years ago
|
||
(In reply to Cork from comment #4)
> Sorry but, I think the state of opt out should be thought over.
Thanks for your input. Please take discussion to the mailing lists, forums, or IRC; these bugs are to track implementation concerns, not to discuss whether features should be implemented at all.
You might want to read
<https://blog.lizardwrangler.com/2012/09/21/firefox-health-report/>
Thanks a lot!
Updated•12 years ago
|
Assignee: nobody → mconnor
Assignee | ||
Comment 6•12 years ago
|
||
This kinda sorta works. Submitting for feedback so Gavin, et al can steer me in the proper direction.
Still need buttons, proper l10n, tests, etc. But, the basic behavior of multiple windows seems to work! And, I didn't even need to involve nsBrowserGlue.js!
Assignee | ||
Comment 7•12 years ago
|
||
This version sucks just a little bit less. But only a little bit.
Attachment #681226 -
Attachment is obsolete: true
Attachment #681226 -
Flags: feedback?(gavin.sharp)
Attachment #681256 -
Flags: feedback?(gavin.sharp)
Comment 8•12 years ago
|
||
Comment on attachment 681256 [details] [diff] [review]
Health report info bar notification, v2
Sorry for the response delay, but I assumed you'd poke if this was blocking you. This looks good for a one-time notification kind of deal - nice to avoid having a permanent browser.xul element for that kind of thing.
I think one of the addObservers in init() should be a removeObserver? :)
You could use swithToTabHavingURI rather than addTab() to open about:healthreport.
_currentPolicyRequest is never cleared, presumably it should be? Though I'm not sure why a reference to it is needed, seems to only be used to avoid displaying multiple notifications, and I think _currentPolicyNotification could be used for that.
Seems like onNotifyDataPolicy could just be inlined in the observer to make the code a bit easier to follow (ymmv).
Presumably the notificationbox "removed" callback fired when close() is called on the notification box - won't that result in a cascade of "healthreport:notify-data-policy:close" events when closing the notification box with multiple notifications/windows open? They'll only be handled once per window, but it still seems inefficient. Setting _currentPolicyNotification to null before calling close() on it in _clearPolicyNotification should help mitigate that, I think (because we then won't notify observers in onEvent).
Attachment #681256 -
Flags: feedback?(gavin.sharp) → feedback+
Comment 9•12 years ago
|
||
This is the UI that we're suggesting: https://firefox-ux.etherpad.mozilla.org/fhroptout?
Comment 10•12 years ago
|
||
Hi all,
FHR is tied directly to a product feature. The wording of this notice should reflect that.
Here's a suggestion - open to more:
$PRODUCTNAME automatically sends some info to Mozilla to provide you with feedback on how your copy of Firefox is running and help us make Firefox better. [Learn More or disable.]
Comment 11•12 years ago
|
||
At the moment, this info bar is intended to be a general notification about the data that Mozilla collects. Tapping on "Choose What I Share" opens the pref pane in bug 809094. The info bar is dismissed automatically if the user doesn't interact with it.
Comment 12•12 years ago
|
||
After _much_ discussion we're combining the user notifications for FHR/Telemetry and linking to the new Data Choices tab we're adding in bug 809094 which exposes (and explains) the various mechanisms Firefox has for collecting information from users. This means we're replacing the current Telemetry notification, so most of this patch is actually code removal.
Assignee: gps → mconnor
Attachment #681256 -
Attachment is obsolete: true
Attachment #697768 -
Attachment is obsolete: true
Attachment #698510 -
Flags: review?(dolske)
Attachment #698510 -
Flags: feedback?(gps)
Updated•12 years ago
|
Attachment #697768 -
Attachment is obsolete: false
Assignee | ||
Comment 13•12 years ago
|
||
Comment on attachment 698510 [details] [diff] [review]
v1.0
Review of attachment 698510 [details] [diff] [review]:
-----------------------------------------------------------------
I would be shocked if this passes all tests. Surely we have mochitest coverage of these notifications. Speaking of which, where are the new tests?
::: browser/components/nsBrowserGlue.js
@@ +9,5 @@
> +#ifdef MOZ_TELEMETRY_REPORTING
> +#define MOZ_REPORTING_ENABLED
> +#elifdef MOZ_SERVICES_HEALTHREPORT
> +#define MOZ_REPORTING_ENABLED
> +#endif
We could expose this in the build system if you'd prefer.
@@ +447,5 @@
> if (this._shouldShowRights()) {
> this._showRightsNotification();
> +#ifdef MOZ_REPORTING_ENABLED
> + } else if (this._shouldShowReporting()) {
> + // once about:rights has been shown, show reporting notification on second run
Nit: use punctuation that would satisfy an rnewman review.
@@ +878,5 @@
> + try {
> + return !Services.prefs.getBoolPref("browser.reporting." + currentVersion + ".shown");
> + } catch (e) { }
> +
> + // We haven't shown the notification before, so do so now.
Nit: rnewman
@@ +882,5 @@
> + // We haven't shown the notification before, so do so now.
> + return true;
> + },
> +
> + _showReportingNotification: function BG__showReportingNotification() {
You no longer need to name functions. The stacks automagically contain the name of the variable/property they were assigned to if the name is omitted.
@@ +897,4 @@
>
> + var message = browserBundle.formatStringFromName("dataReportingNotification.message",
> + [appName, vendorName], 2);
> + var buttons =
"let" for all new code, please.
Nit: trailing whitespace.
::: browser/locales/en-US/chrome/browser/browser.properties
@@ +350,5 @@
> # but addons sync is not. %S will be replaced by syncBrandShortName.
> # The final space separates this text from the Learn More link.
> syncPromoNotification.addons-sync-disabled.description=You can use your %S account to synchronize add-ons across multiple devices.\u0020
>
> +# Mozilla data reporting notification (Telemetry, FHR, etc)
You might want to spell out "Firefox Health Report" to help the translators.
Nit: trailing whitespace.
Attachment #698510 -
Flags: feedback?(gps) → feedback+
Comment 14•12 years ago
|
||
gps' feedback addressed, except for rnewman's insistence on the Queen's English.
Automated tests will follow at some point, but there doesn't appear to be any test coverage for any of the first run notifications, and perhaps not anything in nsBrowserGlue at all, so I'm not really anxious to solve the generic problem there right at this moment.
Attachment #698510 -
Attachment is obsolete: true
Attachment #698510 -
Flags: review?(dolske)
Attachment #698537 -
Flags: review?(gavin.sharp)
Assignee | ||
Comment 15•12 years ago
|
||
So, I didn't look at this patch fully the first time through. I apologize.
We have code in policy.jsm (https://hg.mozilla.org/services/services-central/file/default/services/healthreport/policy.jsm) that essentially drives the prompting/legal requirements as well as data submission logic. It's essentially the decision maker for Health Report. All the legal requirements are codified there and the tests are extremely robust (https://hg.mozilla.org/services/services-central/file/634951119fef/services/healthreport/tests/xpcshell/test_policy.js).
The rules are jotted down at https://hg.mozilla.org/services/services-central/file/default/services/healthreport/policy.jsm#l197. Essentially, a day or so after application first run, the policy generates a new "NotifyPolicyRequest." See https://hg.mozilla.org/services/services-central/file/default/services/healthreport/policy.jsm#l24. This object is propagated to the world via the "healthreport:notify-data-policy:request" notification.
When the user is notified of the data submission requirements, a callback on that object is called. When the user accepts the policy, a callback is called. If the user rejects the policy, a callback is called. policy.jsm receives the signal for everything that occurs and records it.
If the data policy is accepted, FHR proceeds with upload. Otherwise, the service sits idle in the background for all of time (still collecting information because we agreed that collection would always occur so people could gain the benefits of FHR if they decided to opt back in).
Anyway, policy.jsm was written a while ago. I know there have been new requirements since then. If we need to modify the logic in the policy, we should do that. And, I quite like the model of having a central object defining the data submission requirements for the application. It's easier to test and wrap your head around. If nothing else, it's less code in nsBrowserGlue.js and browser.js :)
What I'm trying to say is that the requirements for prompting and data submission are rather complex. There are legal ramifications if we get it wrong. I think we should be consolidating all the code driving decision making into a central location. We have that now in policy.jsm. Let's morph policy.jsm to work for {HealthReport, Telemetry, Crash Reports, ...}.
Comment 16•12 years ago
|
||
Comment on attachment 698537 [details] [diff] [review]
v1.0.1
Review of attachment 698537 [details] [diff] [review]:
-----------------------------------------------------------------
Just a couple of brief comments. I'm not really familiar with the current state of the rights/telemetry/fhr stuff, so if you're looking for feedback on that I'm not your guy. The mechanical changes here seem ok. Except for comment 15, I'd be inclined to rs+.
::: browser/components/nsBrowserGlue.js
@@ +869,5 @@
> + _shouldShowReporting: function () {
> + // Look for an unconditional override pref. If set, do what it says.
> + // (true --> never show, false --> always show)
> + try {
> + return !Services.prefs.getBoolPref("browser.reporting.override");
Nit: this pref isn't very self-descriptive. .showPromptOverride or something?
::: browser/locales/en-US/chrome/browser/browser.properties
@@ +350,5 @@
> # but addons sync is not. %S will be replaced by syncBrandShortName.
> # The final space separates this text from the Learn More link.
> syncPromoNotification.addons-sync-disabled.description=You can use your %S account to synchronize add-ons across multiple devices.\u0020
>
> +# Mozilla data reporting notification (Telemetry, Firefox Health Report, etc)
Trailing whitespace. r-.
@@ +351,5 @@
> # The final space separates this text from the Learn More link.
> syncPromoNotification.addons-sync-disabled.description=You can use your %S account to synchronize add-ons across multiple devices.\u0020
>
> +# Mozilla data reporting notification (Telemetry, Firefox Health Report, etc)
> +dataReportingNotification.message = %1$S automatically sends some data to %2$S so that we can improve your experience
You'll be wanting a LOCALIZATION NOTE here.
Assignee | ||
Comment 18•12 years ago
|
||
We are back to having policy.jsm drive display of the info bar. Things seem to be working well.
Privacy told us they would like for the info bar to be displayed on all tabs on all windows. I solved the multiple window problem by moving the code from nsBrowserGlue.js to browser.js. I'm not sure if multiple tabs is possible with "getNotificationBox()." I'm still trying to wrap my head around where exactly that <notificationbox> exists. In early versions of this patch I was creating a new <notificationbox> and was inserting it into the DOM manually.
As I look over the source code, I think we should definitely come up with a more formal API for notification display, complete with options to control multiple windows/tabs, timeouts, etc. I predict we'd eliminate many LoC if we had a decent API here. Anyway, that's for another bug.
The other remaining open issue is gating on about:rights display. Previously, we could just check whether about:rights had been displayed in nsBrowserGlue.js. We no longer have access to this function in browser.js. So, I need to figure out a way to call it. Mossop suggested adding it to nsIBrowserGlue. I threw out the idea of moving it to a JSM. I dunno. I just don't want to copy code - nsBrowserGlue.js and browser.js are convoluted enough as it is and this is code we need to get right, so centralization is a requirement, IMO.
Finally, this patch requires others patches to be applied in order to work.
Attachment #698537 -
Attachment is obsolete: true
Attachment #698537 -
Flags: review?(gavin.sharp)
Assignee | ||
Comment 19•12 years ago
|
||
Linking to bug 699806 since we will be merging the Telemetry and FHR info bars.
If anyone from Telemetry looks at the patch, please don't fret out if not all your logic has been preserved - I haven't yet put much effort to porting the old logic to the new patch.
Depends on: 699806
Comment 20•12 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #18)
> I'm not sure if multiple tabs is possible
> with "getNotificationBox()." I'm still trying to wrap my head around where
> exactly that <notificationbox> exists.
It literally lives in tabbrowser.xml (in initial <content>, as well as in addTab()). It's part of the per-tab (tabpanel) content; if you have 5 tabs there are 5 <notificationbox> elements floating around in memory. And so making a global notification bar work via existing bits would be rather ugly, eg you'd need to observe opening/switching tabs and manually add a new bar to each.
IIRC, the original Sync landing wanted a global notification bar too, and ran into this exact issue. But sidestepped it by adding a new <notificationbox> outside the tabpanel, so that there was 1 per window. Which is much easier to deal with than 1-per-tab.
I don't see this code offhand; maybe it got removed? I certainly remember it being annoying when Sync was sensitive about temporary network glitches and was showing the damn thing all the time. :)
> As I look over the source code, I think we should definitely come up with a
> more formal API for notification display, complete with options to control
> multiple windows/tabs, timeouts, etc. I predict we'd eliminate many LoC if
> we had a decent API here. Anyway, that's for another bug.
Obligatory grumble here about us needing a better way to allow the browser to talk to the user, in general. A global notification bar is really sucky UI to have (sucky enough that I'm not thrilled about shipping such a thing -- will have to ponder that). But, yes, another bug. :)
> The other remaining open issue is gating on about:rights display.
I'm not quite sure what you mean here?
Assignee | ||
Comment 21•12 years ago
|
||
Here's the deal.
Privacy wants the notification displayed on all windows on all tabs. The simplest technical solution means placing the notification bar on the bottom of the window. The reason is that the space between the URL bar (below the tab strip) and the main content window is part of the tab browser. If we were to put the notification bar here, we would have to iterate over all tabs and insert a separate notification bar on each. We would need to listen for new tabs and insert the notification when they are created. Not super difficult code, but more complex and something I'd like to avoid if necessary.
mconnor: Please confirm an info bar on the bottom is acceptable from stakeholders. If it isn't, is landing on Nightly on bottom with a follow-up to fix acceptable?
I only assigned to mconnor because I'm not sure who owns this decision. If you do, please step in.
Attachment #700722 -
Flags: feedback?(mconnor)
Assignee | ||
Comment 22•12 years ago
|
||
The info bar part of this patch is very similar to one I submitted many weeks ago. It attained f+ from Gavin.
The refactoring of the Telemetry prompting code is from mconnor's patch. It may not be complete.
This patch still needs tests and thorough review that it is setting the appropriate Telemetry prefs.
Behavior is thus:
If the Gecko app has any forms of data submission enabled (currently FHR, Telemetry, or Crash Reports), we set up a listener in browser.js waiting for a background notification. The newly-introduced "metrics collection service" (MCS) periodically sees if it needs to perform any actions. One of those actions is initiate prompting of the acceptance of data reporting policy.
The MCS broadcasts a notification to display the data reporting policy. Every browser window will receive that notification and will display a notification box at the *bottom* of the window (top is technically harder - see previous comment).
When the notification box is displayed, the browser window tells the MCS that the notification has been presented. At this point, the MCS effectively starts an "implicit acceptance" timer. If no explicit user action is performed on the notification for a configured time span (currently 5 minutes), the MCS infers implicit acceptance of the policy and accepts the default upload choice for that data set (ON for FHR and Crash Reports, OFF for Telemetry - on Release and Beta channels).
If a new tab is opened, the notification persists at the bottom of the window for the new tab.
If the "X" in the notification box is clicked, this constitutes acknowledgement of the data reporting notice and the default upload choice for that data set is made (same as implicit acceptance). The notification boxes disappear from all windows (by way of observer notifications).
If the button in the notification box is clicked, the "Data Choices" tab in the "advanced" privacy pane is opened. The default upload choices for the data sets are made. The notification boxes on all windows disappear. The user then has the choice to change the default choices.
Once explicit user activity is performed on the notification box or once implicit acceptance has occurred, the MCS may kick off a data upload. This could occur within seconds. I can't speak for Telemetry, but for FHR, if the checkbox in the privacy pane is unchecked, we will initiate the remote data delete procedure if remote data is currently stored. The MCS will continue to try to delete server data until it is able to. These attempts are preserved across application restarts.
Tom: Please confirm this implementation is to Privacy's liking.
Attachment #700574 -
Attachment is obsolete: true
Attachment #700746 -
Flags: feedback?(tom)
Assignee | ||
Comment 23•12 years ago
|
||
Gavin: Please treat as a review. Things lacking:
* Need to verify Telemetry prefs are carried over appropriately (I'm 95% sure we're missing something).
* Need tests (although maybe not a landing requirement, but certainly an immediate follow-up).
Attachment #700746 -
Attachment is obsolete: true
Attachment #700746 -
Flags: feedback?(tom)
Attachment #701343 -
Flags: feedback?(gavin.sharp)
Assignee | ||
Comment 24•12 years ago
|
||
I added a setPref for Telemetry which I /think/ is required either by core Telemetry code or by the Telemetry prefs pane. This pref is redundant with the new info bar prompting logic (they share the same data reporting policy so we no longer need to track complex state in Telemetry prefs).
We need a follow-up bug to remove traces of the legacy preferences from the tree.
Attachment #701343 -
Attachment is obsolete: true
Attachment #701343 -
Flags: feedback?(gavin.sharp)
Attachment #701363 -
Flags: review?(gavin.sharp)
Comment 25•12 years ago
|
||
Comment on attachment 701363 [details] [diff] [review]
Data policy info bar, v6
Overall, this makes opting in to telemetry a bit more difficult, so we should probably make sure that is communicated to everyone that cares about telemetry.
>diff --git a/browser/base/content/browser-data-submission-info-bar.js b/browser/base/content/browser-data-submission-info-bar.js
>+Cu.import("resource://services-common/log4moz.js");
Looks like this will pollute the global window scope, which is probably not a great idea. You can use:
let log4moz = Cu.import("resource://services-common/log4moz.js", {}).Log4Moz;
in the DataNotificationInfoBar constructor.
>+DataNotificationInfoBar.prototype = {
>+#ifdef MOZ_TELEMETRY_ON_BY_DEFAULT
>+ _PREF_TELEMETRY_DISPLAYED: "toolkit.telementry.notifiedOptOut",
teleme_n_try?
>+ init: function() {
>+ window.addEventListener("unload", function onUnload() {
>+ for (let o of this._OBSERVERS) {
>+ Services.obs.addObserver(this, o, true);
Per comment 8, this guy should probably be a call to "removeObserver" :)
>+ _displayDataPolicyInfoBar: function (request) {
>+ if (this._currentPolicyRequest) {
>+ return;
Rather than caching _currentPolicyRequest, this check could just be:
if (this._notificationBox.getNotificationWithValue("data-reporting"))
return;
(after this._ensureNotificationBox())
>+ let buttons = [{
>+ callback: function () {
>+ policy.recordUserAcceptance("info-bar-button-pressed");
This should be request.onUserAccept, per our IRL discussion.
>+ let notification = this._notificationBox.appendNotification(
>+ function onEvent(event) {
>+ if (event == "removed") {
>+ if (!this._currentPolicyNotification) {
>+ return;
I'm not sure how this can ever be true? It shouldn't be possible for the "removed" handler to be called twice.
>+ this._currentPolicyRequest = request;
>+ this._currentPolicyNotification = notification;
I think you can get rid of _currentPolicyNotification in favor of this._notificationBox.getNotificationWithValue("data-reporting") in _clearPolicyNotification.
>+ // This likely isn't needed in a world where Telemetry and FHR share a
>+ // notification and data reporting policy. It is preserved until traces
>+ // of this pref are wiped from the code base.
>+ Services.prefs.setIntPref(this._PREF_TELEMETRY_DISPLAYED,
>+ this._TELEMETRY_DISPLAY_REV);
Is there any point to this?
>diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js
>+let gDataNotificationInfoBar = new DataNotificationInfoBar();
Seems like this could just be a singleton object.
>diff --git a/browser/components/nsBrowserGlue.js b/browser/components/nsBrowserGlue.js
>- const TELEMETRY_DISPLAY_REV = @MOZ_TELEMETRY_DISPLAY_REV@;
>- telemetryDisplayed = Services.prefs.getIntPref(PREF_TELEMETRY_DISPLAYED);
>- telemetryRejected = Services.prefs.getBoolPref(PREF_TELEMETRY_REJECTED);
Seems like we'll need more cleanup related to this prefs. They're still set in the preferences pane, apparently, and that seems pointless now.
>- var url = Services.urlFormatter.formatURLPref("app.support.baseURL");
>- url += "how-can-i-help-submitting-performance-data";
Is there a bug tracking updating the SUMO documentation for all of this stuff?
>diff --git a/browser/locales/en-US/chrome/browser/browser.properties b/browser/locales/en-US/chrome/browser/browser.properties
>+dataReportingNotification.message = %1$S automatically sends some data to %2$S so that we can improve your experience.
I assume this string has been signed off on by everyone etc.
>+dataReportingNotification.button.label = Choose What I Share
Should this be capitalized? I don't know.
With these fixed, I think this is good to go.
Attachment #701363 -
Flags: review?(gavin.sharp)
Assignee | ||
Comment 26•12 years ago
|
||
Incorporated feedback.
Attachment #701363 -
Attachment is obsolete: true
Attachment #701392 -
Flags: review?(gavin.sharp)
Comment 27•12 years ago
|
||
One other issue I forgot to mention: need to have a way to make sure the notification doesn't show up during test runs.
Updated•12 years ago
|
Attachment #701392 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 28•12 years ago
|
||
Whiteboard: [fixed in services]
Assignee | ||
Comment 29•12 years ago
|
||
Comment on attachment 701392 [details] [diff] [review]
Data policy info bar, v7
[Approval Request Comment]
Bug caused by (feature/regressing bug #): 718066
User impact if declined: Lack of localization in Fx20
Testing completed (on m-c, etc.): Manual testing. Waiting for tests to go green in s-c before merging to s-c.
Risk to taking this patch (and alternatives if risky): Slight chance it might break Telemetry. If so, we'll figure it out and uplift. We need to get strings in 20.
String or UUID changes made by this patch: Added some new strings for FHR.
Attachment #701392 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 30•12 years ago
|
||
Trivial bustage fix r+ by rnewman over IRC. Not sure how this one got past all the local runs I performed :/
https://hg.mozilla.org/services/services-central/rev/f290fba9d3a7
Assignee | ||
Comment 31•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2e571455bcd4
https://hg.mozilla.org/mozilla-central/rev/f290fba9d3a7
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in services]
Target Milestone: --- → Firefox 21
Comment 32•12 years ago
|
||
>+ this._notificationBox.getNotificationWithValue("date-reporting").close();
"datE-reporting"? Sounds like a typo, no?
Comment 33•12 years ago
|
||
Trivial fix, implemented as a refactoring to avoid duplication. Tested by running all browser mochitests, and running manually. Info bar displayed and dismissed without error.
Attachment #701539 -
Flags: review+
Comment 34•12 years ago
|
||
Comment on attachment 700722 [details]
Screenshot of info bar on bottom
Clearing stale feedback flag.
Attachment #700722 -
Flags: feedback?(mconnor)
Comment 35•12 years ago
|
||
I landed this fix on s-c to get coverage before landing on m-c.
https://hg.mozilla.org/services/services-central/rev/ef620df3403d
Comment 36•12 years ago
|
||
Comment 37•12 years ago
|
||
Comment on attachment 701392 [details] [diff] [review]
Data policy info bar, v7
[Triage Comment]
Approving for Aurora (temporarily disabled) since this feature has been targeted for a couple of Firefox releases now and only missed the merge by a couple of days. l10n drivers have been notified of this uplift.
Attachment #701392 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 38•12 years ago
|
||
I forgot to note that we're confident the current strings and design are final, and that only minor changes will be necessary in-product in preparation for Firefox 20's ship.
Comment 39•12 years ago
|
||
Comment on attachment 701539 [details] [diff] [review]
Fix for typo. v1
This'll need to catch up; landed it on m-c around 2am!
Attachment #701539 -
Flags: approval-mozilla-aurora?
Updated•12 years ago
|
Attachment #701539 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 40•12 years ago
|
||
Pushed to Aurora:
http://hg.mozilla.org/releases/mozilla-aurora/rev/b0f512680a8d
status-firefox20:
--- → fixed
Comment 41•12 years ago
|
||
Comment 42•12 years ago
|
||
Is there a way to force this infobar to appear?
There could be something wrong with the accesskey: it displayed "Choose What I Share" with a "(U)" accesskey on an older build (around 2013-01-15), even if the ak was set to "C" from the beginning.
Comment 43•12 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #42)
> Is there a way to force this infobar to appear?
>
> There could be something wrong with the accesskey: it displayed "Choose What
> I Share" with a "(U)" accesskey on an older build (around 2013-01-15), even
> if the ak was set to "C" from the beginning.
It should open a short while after launch.
If it doesn't, and you haven't seen it yet, the following in a privileged console should do the trick:
const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
let reporter = Cc["@mozilla.org/datareporting/service;1"]
.getService(Ci.nsISupports)
.wrappedJSObject
.healthReporter;
let policy = reporter._policy;
policy.ensureNotifyResponse(new Date());
Assignee | ||
Comment 44•12 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #42)
> Is there a way to force this infobar to appear?
>
> There could be something wrong with the accesskey: it displayed "Choose What
> I Share" with a "(U)" accesskey on an older build (around 2013-01-15), even
> if the ak was set to "C" from the beginning.
1) New profile
2) about:config
3) datareporting.policy.firstRunTime -= 20000000000
4) datareporting.policy.nextDataSubmissionTime -= 20000000000
(Subtract 2 from the 3rd from front digit.)
Comment 45•12 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #44)
> 1) New profile
> 2) about:config
> 3) datareporting.policy.firstRunTime -= 20000000000
> 4) datareporting.policy.nextDataSubmissionTime -= 20000000000
>
> (Subtract 2 from the 3rd from front digit.)
Thanks a lot :-)
Tested and now working as expected. Not sure where that "U" came from, considering that another person had the same problem.
Comment 46•12 years ago
|
||
The U was bug 830090, should be well fixed now.
You need to log in
before you can comment on or make changes to this bug.
Description
•