Closed
Bug 950237
Opened 11 years ago
Closed 11 years ago
Fennec's geolocation "stumbling" code needs to use the new JSON report format
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox27 wontfix, firefox28 wontfix, firefox29 fixed, firefox30 fixed)
RESOLVED
FIXED
Firefox 30
People
(Reporter: cpeterson, Assigned: cpeterson)
References
Details
Attachments
(1 file)
(deleted),
patch
|
blassey
:
review+
hschlichting
:
feedback+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
In particular, reports include BSSIDs instead of hashes:
https://mozilla-ichnaea.readthedocs.org/en/latest/api/submit.html
rtilder said he was working on this. :)
Comment 1•11 years ago
|
||
This only blocks the general MLS bug, but not the privacy text UI change.
Updated•11 years ago
|
Assignee: rtilder → nobody
Comment 2•11 years ago
|
||
It was on the list but my hopes to do work on the MozStumbler client have disappeared in the face of other higher priority items.
Status: ASSIGNED → NEW
Updated•11 years ago
|
Assignee: nobody → cpeterson
Assignee | ||
Comment 3•11 years ago
|
||
Fennec has been reporting location data that is incompatible with the current server format. Migrate reporter changes from MozStumbler to Fennec:
https://github.com/mozilla/MozStumbler/blob/master/src/org/mozilla/mozstumbler/Reporter.java
• Do not omit ad hoc SSIDs (because they are not necessarily mobile devices and the server already has to detect other moving access points).
• Round lat/long measurements to 6 decimal places (for consistency and to avoid the illusion that these Android devices' GPS hardware has millimeter precision).
• Report the current date instead of a timestamp in seconds to reduce PII that might be used to track a user. The server does not need one-second precision to scrub stale measurements from the database months from now.
• Report BSSIDs instead of SHA1(BSSID + SSID) hashes.
• Workaround an HTTP connection pooling bug in ICS and JB that causes many report uploads to fail.
Attachment #8372081 -
Flags: review?(blassey.bugs)
Updated•11 years ago
|
Attachment #8372081 -
Flags: feedback+
Comment 4•11 years ago
|
||
Comment on attachment 8372081 [details] [diff] [review]
950237.patch
Review of attachment 8372081 [details] [diff] [review]:
-----------------------------------------------------------------
::: mobile/android/base/GeckoApp.java
@@ +2459,5 @@
> }
>
> + // Round lat/lon to 6 decimal places.
> + locInfo.put("lat", Math.floor(location.getLatitude() * 1.0E6) / 1.0E6);
> + locInfo.put("lon", Math.floor(location.getLongitude() * 1.0E6) / 1.0E6);
use a Formatter (formatter.format("%.2f", location.getLatitude()));
Attachment #8372081 -
Flags: review?(blassey.bugs) → review+
Assignee | ||
Comment 5•11 years ago
|
||
(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #4)
> use a Formatter (formatter.format("%.2f", location.getLatitude()));
Using a Formatter seems a bit heavyweight for this operation. A Formatter will produce a formatted string, which JSONObject will emit as a quoted string property (e.g. "lat":"37.871936") instead of a number (e.g. "lat":37.871936), so we would need to parse that formatted string to obtain a number again. And the Formatter can't be reused, so I would need two:
> locInfo.put("lat", Double.parseDouble(new Formatter().format("%.6f", location.getLatitude()).toString()));
> locInfo.put("long", Double.parseDouble(new Formatter().format("%.6f", location.getLongitude()).toString()));
I can just leave the old code without rounding. The server will drop the extra bits of pseudo-precision anyways. :)
Assignee | ||
Comment 6•11 years ago
|
||
Status: NEW → ASSIGNED
status-firefox27:
--- → affected
status-firefox28:
--- → wontfix
status-firefox29:
--- → affected
status-firefox30:
--- → fixed
Comment 7•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Assignee | ||
Comment 8•11 years ago
|
||
Comment on attachment 8372081 [details] [diff] [review]
950237.patch
[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: Fennec's geolocation data collection has been broken for a few months, using the wrong report format so the server has been receiving useless data.
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): Low risk because this code is already broken. This code is preffed to only be enabled on Nightly and Aurora.
String or IDL/UUID changes made by this patch: N/A
Attachment #8372081 -
Flags: approval-mozilla-aurora?
Comment 9•11 years ago
|
||
Comment on attachment 8372081 [details] [diff] [review]
950237.patch
It is not too late for 28 if needed.
Attachment #8372081 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 10•11 years ago
|
||
Assignee | ||
Comment 11•11 years ago
|
||
• Ryan: thanks for uplifting this fix.
• Sylvestre: we won't need to uplift this fix to Beta 28 because this code is preffed off in the Beta and Release channels (atm).
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•