UITelemetry for new search UI breaks about:telemetry, needs privacy review

RESOLVED DUPLICATE of bug 1119327

Status

()

Firefox
Search
RESOLVED DUPLICATE of bug 1119327
3 years ago
3 years ago

People

(Reporter: vladan, Unassigned)

Tracking

37 Branch
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

3 years ago
This line in BrowserUITelemetry.jsm adds an nsISearchEngine object to the Telemetry payload (indirectly):

result.currentSearchEngine = Services.search.currentEngine;

From: http://hg.mozilla.org/mozilla-central/diff/368f0748e882/browser/modules/BrowserUITelemetry.jsm


These nsISearchEngine objects can't be converted to strings with JSON.stringify:

JSON.stringify(Services.search.currentEngine);
[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIURI.userPass]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: debugger eval code :: <TOP_LEVEL> :: line 1"  data: no]

Apparently there's some issue with the "userPass" nsIURI field.


1) Can you stringify these objects before giving them to Telemetry in UITelemetry.getSimpleMeasures();
2) Additionally, I'm concerned that we're including raw URIs and potentially submitting a userPass to the Telemetry payload..
3) Also, I think we should put BrowserUITelemetry in its own top-level section of the Telemetry ping. It has become too complex for simpleMeasurements. This should be another less-urgent bug
Flags: needinfo?(bwinton)
(Reporter)

Comment 1

3 years ago
I think this would have broken Telemetry submissions as well.. Mark, is there a dip in submissions since Jan 7th?
Flags: needinfo?(mreid)
(Reporter)

Comment 2

3 years ago
[Tracking Requested - why for this release]:
Now that I think about it.. we should do a privacy review if we're going to submit user URLs to Telemetry
status-firefox37: --- → affected
tracking-firefox37: --- → ?
(Reporter)

Updated

3 years ago
Summary: UITelemetry for search bar UI breaks about:telemetry → UITelemetry for new search UI breaks about:telemetry, needs privacy review
I believe this should be fixed (both the bustage and the bug of sending the urls) by bug 1119327.

If we put BrowserUITelemetry in its own top-level section, I would advocate for making it a little simpler, since I don't think we need to track as much stuff, or at least, not in as confusing a way.  ;)  (In particular, I don't think we care about the UITour durations anymore, which should simplify the structure a fair bit.)
Flags: needinfo?(bwinton)
(Reporter)

Updated

3 years ago
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Flags: needinfo?(mreid)
Resolution: --- → DUPLICATE
Duplicate of bug: 1119327
status-firefox37: affected → ---
tracking-firefox37: ? → ---
You need to log in before you can comment on or make changes to this bug.