Closed
Bug 1352497
Opened 8 years ago
Closed 7 years ago
Remove about:healthreport
Categories
(Toolkit :: Telemetry, enhancement, P1)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
mozilla59
People
(Reporter: mheubusch, Assigned: dao)
References
(Blocks 1 open bug)
Details
(Whiteboard: [measurement:client:tracking])
Attachments
(2 files, 2 obsolete files)
Please remove the access points to the about:healthreport page. On all platforms from the HamburgerMenu>? button>Help menu and on Mac also from the top Help menu.
Comment 1•8 years ago
|
||
In the Preferences/Options,
new preferences (Nightly; after bug 1335907 landed): Updates > Enable Health Report
old preferences: Advanced > Data Choices > Enable Health Report
has as description: "Helps you understand your browser performance and shares data with Mozilla about your browser health"
Should this be shortened to "Shares data with Mozilla about your browser health"?
Furthermore, Telemetry depends on FHR on desktop in the preferences, but not on Android.
Flags: needinfo?(mheubusch)
Comment 2•8 years ago
|
||
(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #1)
> In the Preferences/Options,
> new preferences (Nightly; after bug 1335907 landed): Updates > Enable Health
> Report
> old preferences: Advanced > Data Choices > Enable Health Report
> has as description: "Helps you understand your browser performance and
> shares data with Mozilla about your browser health"
>
> Should this be shortened to "Shares data with Mozilla about your browser
> health"?
See bug 1348074 on FHR/Telemetry description concerns.
This bug is separate and only about not exposing about:healthreport directly anymore.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Updated•8 years ago
|
Assignee: nobody → aryx.bugmail
Status: NEW → ASSIGNED
Flags: needinfo?(mheubusch)
Comment 5•8 years ago
|
||
Has this gone through a legal review with Marshall? I fully support this but there have been concerns before.
Flags: needinfo?(mheubusch)
Updated•8 years ago
|
Attachment #8853948 -
Flags: review?(rnewman) → review?(s.kaspari)
Comment 6•8 years ago
|
||
mozreview-review |
Comment on attachment 8853948 [details]
Bug 1352497 - Remove link to about:healthreport from menus: android.
https://reviewboard.mozilla.org/r/125966/#review129358
LGTM
Attachment #8853948 -
Flags: review?(s.kaspari) → review+
Assignee | ||
Comment 7•8 years ago
|
||
So these patches remove the references to about:healthreport but the page still exists, basically as dead code at that point. What's the longer-term plan for about:healthreport?
Comment 8•8 years ago
|
||
Please DO NOT land this without confirmation from Michelle that this has gone through risk analysis.
If we can remove this, we can probably remove the about:healthreport mechanism altogether.
(In reply to Benjamin Smedberg [:bsmedberg] from comment #5)
> Has this gone through a legal review with Marshall? I fully support this but
> there have been concerns before.
Hello Benjamin - I filed this bug per a discussion with Mika. Since we are only removing the access points from the UI but not the page itself, she simply asked that I notify Marshall so that we had a response ready in case anyone thought the page itself was gone. He is cc'ed on the bug. I will file a separate bug and x-ref it here just to be thorough.
Flags: needinfo?(mheubusch)
Comment 10•8 years ago
|
||
There is absolutely no point in keeping the about:healthreport page if it's not in the UI.
Assignee | ||
Comment 11•8 years ago
|
||
Comment on attachment 8853947 [details]
Bug 1352497 - Remove link to about:healthreport from menus: browser/.
I haven't heard back from Mika, but I think general consensus was that we should completely remove this page.
Attachment #8853947 -
Flags: review?(dao+bmo) → review-
Updated•8 years ago
|
Whiteboard: [measurement:client:tracking]
Assignee | ||
Updated•8 years ago
|
Summary: Remove link to about:healthreport from Menu → Remove about:healthreport
Comment 12•7 years ago
|
||
Hi all. I'm visiting from bug 1346273 where we're trying, and failing, to apply modern web security principles to about:healthreport. Can we get this removal shipped in Firefox soon?
Comment 13•7 years ago
|
||
Looking at what about:healthreport does, it almost seems like this would be better served as a wrapped webextension, if we want it to continue to exist.
Comment 16•7 years ago
|
||
I spoke with Elvin about this (since Mika is now out on leave).
We will likely want to move forward with removal in v56, but Elvin is trying to coordinate some governance communication about a number of changes for telemetry (preferences) and healthreport. (that we may want to put out before the removal)
He asked that we hold off until he comes back with confirmation to proceed. I'm NI'ing him here so he can reply directly.
Flags: needinfo?(pdolanjski) → needinfo?(ellee)
Comment 17•7 years ago
|
||
Hi, confirming that we can move forward with the work to remove for 56+. How are we planning on informing users in-product what happened to FHR?
Flags: needinfo?(ellee) → needinfo?(pdolanjski)
Comment 18•7 years ago
|
||
(In reply to Elvin Lee [:ellee] (use NEEDINFO please) from comment #17)
> Hi, confirming that we can move forward with the work to remove for 56+. How
> are we planning on informing users in-product what happened to FHR?
What happened to FHR in terms of it just becoming telemetry or do you mean about:healthreport, specifically?
Flags: needinfo?(pdolanjski)
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(ellee)
Comment 19•7 years ago
|
||
What happened to about:healthreport, specifically. The concern being that few users who do go to about:healthreport will suddenly get "address not valid"
Flags: needinfo?(ellee) → needinfo?(pdolanjski)
Assignee | ||
Comment 20•7 years ago
|
||
(In reply to Elvin Lee [:ellee] (use NEEDINFO please) from comment #19)
> What happened to about:healthreport, specifically. The concern being that
> few users who do go to about:healthreport will suddenly get "address not
> valid"
I think the standard way to do this is to put in the release notes.
We could also have a stub about:healthreport page explaining that the feature doesn't exist anymore. However, I suspect for most people who ever saw or used this feature, the Help menu has been the entry point, so once that menu item is gone, leaving about:healthreport behind, even as a stub, seems pointless.
Comment 21•7 years ago
|
||
Yes, release notes. If someone goes there and it's not there, they can check the release notes. That's probably the best we can do and maybe add something in SUMO.
Elvin, does that work for you?
Joni, can you help with the SUMO piece? If we have any information on about:health report, it should be updated to say that the page has been removed.
Flags: needinfo?(pdolanjski)
Flags: needinfo?(jsavage)
Flags: needinfo?(ellee)
Comment 22•7 years ago
|
||
That's fine - probably should indicate the version it is being removed for in the SUMO article too.
Flags: needinfo?(ellee)
Assignee | ||
Comment 23•7 years ago
|
||
Sebastian, are you available to pick this up again?
Flags: needinfo?(aryx.bugmail)
Comment 24•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #23)
> Sebastian, are you available to pick this up again?
Yes, will do.
Flags: needinfo?(aryx.bugmail)
Comment 25•7 years ago
|
||
Just curious, are the underlying about:config settings being stripped out as well?
Flags: needinfo?(aryx.bugmail)
Assignee | ||
Comment 26•7 years ago
|
||
(In reply to [:jgruen] from comment #25)
> Just curious, are the underlying about:config settings being stripped out as
> well?
What prefs in particular?
Any pref that's needed only for the about:healthreport page should ideally go away here. Other prefs would be off-topic as far as this bug is concerned.
Flags: needinfo?(jgruen)
Comment 27•7 years ago
|
||
Screenshots uses `datareporting.healthreport.uploadEnabled` to determine whether or not we collect data. I've already filed an issue to move us off this pref. https://github.com/mozilla-services/screenshots/issues/3390
Flags: needinfo?(jgruen)
Assignee | ||
Comment 28•7 years ago
|
||
(In reply to [:jgruen] from comment #27)
> `datareporting.healthreport.uploadEnabled`
I don't think this has anything to do with this bug.
Comment 29•7 years ago
|
||
Fair enough, we're going to make the switch anyway since FHR seems to be going into the dustbin.
Comment 30•7 years ago
|
||
(In reply to [:jgruen] from comment #27)
> Screenshots uses `datareporting.healthreport.uploadEnabled` to determine
> whether or not we collect data. I've already filed an issue to move us off
> this pref. https://github.com/mozilla-services/screenshots/issues/3390
This pref still controls all data upload, even with Telemetry. Let's check what exactly can get cleanup.
To simplify things we could limit this bug to just remove what is clearly only used for the about:healthreport page and move anything that requires investigation or discussion to a follow-up.
I'll follow up on the GH issue.
Updated•7 years ago
|
Comment 31•7 years ago
|
||
please help us what to do with the existing sumo article at https://support.mozilla.org/kb/firefox-health-report-understand-your-browser-perf documenting fhr (bug 1400272) :-)
we can add version specific content there - in which version of firefox are you planning to totally remove the page?
Assignee | ||
Comment 32•7 years ago
|
||
At this point this will miss 57 unless we still uplift it. Sebastian, are you working on this or should somebody else take over?
Assignee | ||
Updated•7 years ago
|
Assignee: aryx.bugmail → dao+bmo
Flags: needinfo?(jsavage)
Flags: needinfo?(aryx.bugmail)
Assignee | ||
Updated•7 years ago
|
Component: Menus → Telemetry
Product: Firefox → Toolkit
Assignee | ||
Updated•7 years ago
|
Attachment #8853947 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8853948 -
Attachment is obsolete: true
Comment hidden (mozreview-request) |
Assignee | ||
Comment 34•7 years ago
|
||
Comment 35•7 years ago
|
||
mozreview-review |
Comment on attachment 8928932 [details]
Bug 1352497 - Remove about:healthreport.
https://reviewboard.mozilla.org/r/200278/#review206456
Cheers for this, i'm happy to see this go!
One thing: far as i can tell, the selfsupport service is only used for about:healthreport and can go too:
https://searchfox.org/mozilla-central/search?q=MozSelfSupport&path=
https://searchfox.org/mozilla-central/search?q=SelfSupport&case=false®exp=false&path=
Also, i think we can remove more preferences (see below).
::: testing/geckodriver/src/prefs.rs:121
(Diff revision 1)
> ("browser.warnOnQuit", Pref::new(false)),
>
> // Do not show datareporting policy notifications which can
> // interfere with tests
> - ("datareporting.healthreport.about.reportUrl", Pref::new("http://%(server)s/dummy/abouthealthreport/")),
> ("datareporting.healthreport.documentServerURI", Pref::new("http://%(server)s/dummy/healthreport/")),
I think we can remove all `datareporting.*` prefs, except:
- `datareporting.policy.dataSubmission*`
- `datareporting.healthreport.uploadEnabled`
- `datareporting.healthreport.infoURL`
We should double-check though for what Fennec uses.
::: toolkit/components/telemetry/docs/fhr/architecture.rst
(Diff revision 1)
> service.loadDelayMsec
> How long (in milliseconds) after initial application start should FHR
> wait before initializing.
>
> FHR may initialize sooner than this if the FHR service is requested.
> - This will happen if e.g. the user goes to ``about:healthreport``.
FWIW, these docs are marked as obsolete already:
https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/fhr/index.html
We already removed the FHR service before and only keep these docs for data archeology purposes.
::: toolkit/components/telemetry/docs/internals/preferences.rst:31
(Diff revision 1)
>
> ``MOZ_SERVICES_HEALTHREPORT``
>
> When Defined (which it is on most platforms):
>
> - * Builds ``about:healthreport`` and associated underpinnings (see `healthreport <../fhr/index>`)
> + * Builds healthreport underpinnings (see `healthreport <../fhr/index>`)
While we're here, could you also remove this bullet?
We already removed FHR a while ago.
Attachment #8928932 -
Flags: review?(gfritzsche)
Comment 36•7 years ago
|
||
Sebastian, do you know who can give high-level feedback on the Android removals here?
Flags: needinfo?(s.kaspari)
Updated•7 years ago
|
Priority: -- → P1
Assignee | ||
Comment 37•7 years ago
|
||
(In reply to Georg Fritzsche [:gfritzsche] from comment #35)
> One thing: far as i can tell, the selfsupport service is only used for
> about:healthreport and can go too:
> https://searchfox.org/mozilla-central/search?q=MozSelfSupport&path=
> https://searchfox.org/mozilla-central/
> search?q=SelfSupport&case=false®exp=false&path=
It's a web API, right? I can't tell from those searches where it's used. New bug please.
> ::: testing/geckodriver/src/prefs.rs:121
> (Diff revision 1)
> > ("browser.warnOnQuit", Pref::new(false)),
> >
> > // Do not show datareporting policy notifications which can
> > // interfere with tests
> > - ("datareporting.healthreport.about.reportUrl", Pref::new("http://%(server)s/dummy/abouthealthreport/")),
> > ("datareporting.healthreport.documentServerURI", Pref::new("http://%(server)s/dummy/healthreport/")),
>
> I think we can remove all `datareporting.*` prefs, except:
> - `datareporting.policy.dataSubmission*`
> - `datareporting.healthreport.uploadEnabled`
> - `datareporting.healthreport.infoURL`
Ditto, this should be a separate bug. Looks like some of these prefs are already unused today.
Comment hidden (mozreview-request) |
Comment 39•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #37)
> (In reply to Georg Fritzsche [:gfritzsche] from comment #35)
> > One thing: far as i can tell, the selfsupport service is only used for
> > about:healthreport and can go too:
> > https://searchfox.org/mozilla-central/search?q=MozSelfSupport&path=
> > https://searchfox.org/mozilla-central/
> > search?q=SelfSupport&case=false®exp=false&path=
>
> It's a web API, right? I can't tell from those searches where it's used. New
> bug please.
Sounds good.
> > ::: testing/geckodriver/src/prefs.rs:121
> > (Diff revision 1)
> > > ("browser.warnOnQuit", Pref::new(false)),
> > >
> > > // Do not show datareporting policy notifications which can
> > > // interfere with tests
> > > - ("datareporting.healthreport.about.reportUrl", Pref::new("http://%(server)s/dummy/abouthealthreport/")),
> > > ("datareporting.healthreport.documentServerURI", Pref::new("http://%(server)s/dummy/healthreport/")),
> >
> > I think we can remove all `datareporting.*` prefs, except:
> > - `datareporting.policy.dataSubmission*`
> > - `datareporting.healthreport.uploadEnabled`
> > - `datareporting.healthreport.infoURL`
>
> Ditto, this should be a separate bug. Looks like some of these prefs are
> already unused today.
Right. So, if i understand this correctly, you are just removing the one specific pref related to the about:healthreport content?
Assignee | ||
Comment 40•7 years ago
|
||
(In reply to Georg Fritzsche [:gfritzsche] from comment #39)
> Right. So, if i understand this correctly, you are just removing the one
> specific pref related to the about:healthreport content?
Yeah.
Comment 41•7 years ago
|
||
mozreview-review |
Comment on attachment 8928932 [details]
Bug 1352497 - Remove about:healthreport.
https://reviewboard.mozilla.org/r/200278/#review206970
This looks good to me overall.
Two things though:
1) I'd really want a mobile peer to look over it if the changes make sense to them.
2) Removing `mobile/android/chrome/content/healthreport-prefs.js` might have side-effects as it includes `datareporting.healthreport.uploadEnabled`.
I'm not off-hand 100% sure that this won't effect Fennec Telemetry in some way.
I'd prefer to leave the pref `uploadEnabled` in for Fennec and let us clean this up when we get to sorting the mobile Telemetry situation.
Alternatively we can trace the pref usage through the tree and verify, but that will take a bit.
Attachment #8928932 -
Flags: review?(gfritzsche)
Comment 42•7 years ago
|
||
mozreview-review |
Comment on attachment 8928932 [details]
Bug 1352497 - Remove about:healthreport.
https://reviewboard.mozilla.org/r/200278/#review206976
Attachment #8928932 -
Flags: review+
Assignee | ||
Updated•7 years ago
|
Attachment #8928932 -
Flags: review?(s.kaspari)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 44•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8928932 -
Flags: review?(s.kaspari) → review?(cnevinchen)
Comment 45•7 years ago
|
||
mozreview-review |
Comment on attachment 8928932 [details]
Bug 1352497 - Remove about:healthreport.
https://reviewboard.mozilla.org/r/200278/#review208022
The code looks goot to me. I've pulled it and tested in my local. In Fennec side it works fine. But I still don't know the reason to remove this. It'll be great if you can give us more background about this.
Attachment #8928932 -
Flags: review?(cnevinchen) → review+
Assignee | ||
Comment 46•7 years ago
|
||
(In reply to Nevin Chen [:nechen] from comment #45)
> Comment on attachment 8928932 [details]
> Bug 1352497 - Remove about:healthreport.
>
> https://reviewboard.mozilla.org/r/200278/#review208022
>
> The code looks goot to me. I've pulled it and tested in my local. In Fennec
> side it works fine. But I still don't know the reason to remove this. It'll
> be great if you can give us more background about this.
The feature is unneeded and nobody wants to maintain it.
Flags: needinfo?(s.kaspari)
Comment 47•7 years ago
|
||
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/be86ccde4f4a
Remove about:healthreport. r=gfritzsche,nechen
Comment 48•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Comment 49•7 years ago
|
||
Backed out for bc failures on Windows 7 debug with e10s at docshell/test/browser/browser_bug1309900_crossProcessHistoryNavigation.js
https://treeherder.mozilla.org/logviewer.html#?job_id=147537473&repo=mozilla-central&lineNumber=13901
https://hg.mozilla.org/mozilla-central/rev/3e14872b31a7b1b207605d09b78fbaaf21f1bba7
https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=f6f7da6085aa13abb6dce67f7eb21cdc000fd4fb&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable&selectedJob=147537473
Also would you please look at these tier 2 Linux x64 JSDCov opt failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&filter-searchStr=ba2e181e09bd32af6e2b059a56058a9b498da49c&selectedJob=147639971
https://treeherder.mozilla.org/logviewer.html#?job_id=147639971&repo=mozilla-central&lineNumber=4320
Flags: needinfo?(dao+bmo)
Comment 50•7 years ago
|
||
s/with e10s/without e10s/
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8928932 -
Flags: review?(s.kaspari)
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(dao+bmo)
Comment 53•7 years ago
|
||
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7a1a46675739
Remove about:healthreport. r=gfritzsche,nechen
Comment 54•7 years ago
|
||
bugherder |
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → FIXED
Comment 55•7 years ago
|
||
bugherder landing |
Comment 56•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #20)
> (In reply to Elvin Lee [:ellee] (use NEEDINFO please) from comment #19)
> > What happened to about:healthreport, specifically. The concern being that
> > few users who do go to about:healthreport will suddenly get "address not
> > valid"
>
> I think the standard way to do this is to put in the release notes.
>
Do you still intend to have it mentionned in the release notes? If so, please nominate this bug for release note addition. Thanks
Flags: needinfo?(dao+bmo)
Assignee | ||
Comment 57•7 years ago
|
||
Release Note Request (optional, but appreciated)
[Why is this notable]: users may have used this page to understand what data Firefox sends to Mozilla
[Affects Firefox for Android]: yes
[Suggested wording]: about:healthreport (Firefox Health Report) has been removed in favor of about:telemetry
[Links (documentation, blog post, etc)]:
relnote-firefox:
--- → ?
Flags: needinfo?(dao+bmo)
Noted for 59.0b1 as: Changed the location of Firefox Health Report from about:healthreport to about:telemetry
Comment 59•7 years ago
|
||
Note that this patch removed the
datareporting.healthreport.about.reportUrl pref from various automation
tools in the tree that targets release channels out-of-tree.
This change should not have been made without approval from a
peer of the Testing component. I’m remedying this as part of
https://bugzilla.mozilla.org/show_bug.cgi?id=1401129.
You need to log in
before you can comment on or make changes to this bug.
Description
•