Closed Bug 902783 Opened 11 years ago Closed 11 years ago

Fx24 Beta: back out Bug 866957 and Bug 877725

Categories

(Firefox for Android Graveyard :: General, defect)

24 Branch
All
Android
defect
Not set
normal

Tracking

(firefox24+ verified)

RESOLVED FIXED
Tracking Status
firefox24 + verified

People

(Reporter: elan, Assigned: mfinkle)

References

Details

Attachments

(1 file)

We'll need to back out Bug 877725 of Nightly Fx26 and Aurora Fx25, too
Bug 902791 tracks the UI only back-out for Nightly and Aurora.
Attached patch backout-wifi-cell-tracking (deleted) — Splinter Review
I am testing a build with this backout patch. I am basically backing out: * bug 866957, the main bug (3 csets) * bug 877725, the setting ui (1 cset) * bug 882959, a crash fix (1 cset) * bug 886921, a crash fix (1 cset) The patch leaves the strings alone. They won't hurt anything and will reduce l10n churn.
Comment on attachment 787329 [details] [diff] [review] backout-wifi-cell-tracking The beta build went fine. I tested the build. It launched fine. It displayed normal geolocation webpages fine. The settings for wifi tracking were removed, but otherwise things continued to work fine. Ready for review
Attachment #787329 - Flags: review?(blassey.bugs)
Comment on attachment 787329 [details] [diff] [review] backout-wifi-cell-tracking Preemptive approval request [Approval Request Comment] Bug caused by (feature/regressing bug #): User impact if declined: Needed for Product reasons Testing completed (on m-c, etc.): None. This is a straight to beta patch Risk to taking this patch (and alternatives if risky): Some risk. We'll need to do some general testing. String or IDL/UUID changes made by this patch: I left the strings alone
Attachment #787329 - Flags: review?(liuche)
Attachment #787329 - Flags: approval-mozilla-beta?
Here is my build in case anyone else can test: http://people.mozilla.com/~mfinkle/fennec/fennec-24.0.en-US.android-arm.backout.apk It is a mozilla-beta build using Beta branding. You might need to remove any Beta or Release builds from your device if you have issues with installing.
Note to testers: The backout affects the Settings UI and code fired during a geolocation reading. Of course, the app needs to load correctly, and without any new errors appearing in logcat. In general: 1. Check that Settings still functions correctly for the rest of the settings. 2. Check that geo-location test pages still work and request permissions. 3. Check that no logcat errors appear while testing #1 or #2
Another thing to look for is permissions. Compare the patched build (Fx 24 Beta) to the release version (Fx 23). You can find the permissions by going to the Android Settings > Apps > Firefox (or Firefox Beta) The only new permissions should be: * record audio * control Near Field Communication
The unpatched Beta would also have: * connect and disconnect from Wi-Fi * view Wi-Fi connections
OS: Mac OS X → Android
Hardware: x86 → All
Tested on Samsung Galaxy Tab 2 (Android 4.1.1) and HTC Desire HD (Android 2.3.5): * The permissions: "connect and disconnect from Wi-Fi" and "view Wi-Fi connections" are not present in the test build * I do not see any errors in logcat related to data reporting * There is no crash when enabling location share * Location share works over wifi and the share request doorhanger is displayed on maps.google.com and http://mozqa.com/data/firefox/geolocation/wifi.html
Comment on attachment 787329 [details] [diff] [review] backout-wifi-cell-tracking Review of attachment 787329 [details] [diff] [review]: ----------------------------------------------------------------- The UI stuff definitely looks good to me, they pretty much exactly match the patch I added the UI in, and I see you're intentionally leaving in the strings (from an earlier comment). I looked at the GeckoApp.java changes, and those seem fine, but should also be formally reviewed by blassey. Thanks for doing this backout, mfinkle, especially at whatever hellish time zone you're in!
Attachment #787329 - Flags: review?(liuche) → review+
why do we need to back this out?
Comment on attachment 787329 [details] [diff] [review] backout-wifi-cell-tracking Review of attachment 787329 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/GeckoApp.java @@ -2355,5 @@ > public void onLocationChanged(Location location) { > // No logging here: user-identifying information. > GeckoAppShell.sendEventToGecko(GeckoEvent.createLocationEvent(location)); > - if (mShouldReportGeoData) > - collectAndReportLocInfo(location); If we want a lower-touch patch we can just remove these two lines and the permissions from the manifest.
Attachment #787329 - Flags: review?(blassey.bugs) → review+
> -import java.io.InputStreamReader; Was that an unused import? (In reply to Mark Finkle (:mfinkle) from comment #7) > Another thing to look for is permissions. Compare the patched build (Fx 24 > Beta) to the release version (Fx 23). You can find the permissions by going > to the Android Settings > Apps > Firefox (or Firefox Beta) > > The only new permissions should be: > * record audio > * control Near Field Communication Confirmed.
(In reply to Aaron Train [:aaronmt] from comment #13) > > -import java.io.InputStreamReader; > > Was that an unused import? It was added here: https://hg.mozilla.org/mozilla-central/rev/92178edfeb6e (in bug 866957) Must be unused or Java would yell at me.
Comment on attachment 787329 [details] [diff] [review] backout-wifi-cell-tracking Approving for landing directly to the beta branch for a 24.0beta1 (build #3) respin.
Attachment #787329 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: