Closed Bug 1071091 Opened 10 years ago Closed 10 years ago

System download manager needs a build time pref

Categories

(Firefox Build System :: Android Studio and Gradle Integration, defect)

All
Android
defect
Not set
normal

Tracking

(firefox35 fixed, fennec35+)

RESOLVED FIXED
mozilla35
Tracking Status
firefox35 --- fixed
fennec 35+ ---

People

(Reporter: wesj, Assigned: wesj)

References

Details

Attachments

(1 file)

We want a build time pref so that we don't accidentally ship the Android permission request.
Blocks: 816318
tracking-fennec: --- → ?
Component: General → Build Config & IDE Support
OS: Linux → Android
Hardware: x86_64 → All
Attached patch PatchSplinter Review
I had to add some stuff in configure.in that's not in your blog post nick. I'm not sure why?
Attachment #8495001 - Flags: review?(nalexander)
Comment on attachment 8495001 [details] [diff] [review]
Patch

Review of attachment 8495001 [details] [diff] [review]:
-----------------------------------------------------------------

Nits and try builds both ways (as-is and with confvars.sh set to off), but it looks fine to me!

::: configure.in
@@ +3844,5 @@
>  MOZ_WEBSMS_BACKEND=
>  MOZ_ANDROID_BEAM=
>  MOZ_LOCALE_SWITCHER=
>  MOZ_ANDROID_SEARCH_ACTIVITY=
> +MOZ_ANDROID_DOWNLOADS_INTEGRATION=

You'll also want a AC_SUBST(MOZ_ANDROID_*) line below.  See the other MOZ_ANDROID.  (I was trying to avoid this at the time I blogged, but I think we're just rolling with it.)

::: mobile/android/base/GeckoAppShell.java
@@ +1806,5 @@
>              }
>          }
>  
>          final File f = new File(aFile);
> +        if (AppConstants.Versions.feature12Plus && AppConstants.ANDROID_DOWNLOADS_INTEGRATION) {

Would it be worth folding the version check into the constant define?  Might make it harder to use incorrectly in future.

::: mobile/android/confvars.sh
@@ +92,5 @@
>  else
>    MOZ_ANDROID_MLS_STUMBLER=
>  fi
> +
> +# Enable the Mozilla Location Service stumbler in Nightly.

Copy-pasta comment, but you've changed the flag below.  You want Aurora and Nightly?
Attachment #8495001 - Flags: review?(nalexander) → review+
Assignee: nobody → wjohnston
tracking-fennec: ? → 35+
https://hg.mozilla.org/mozilla-central/rev/0c8a202cc44f
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
(In reply to Nick Alexander :nalexander from comment #2)

> Would it be worth folding the version check into the constant define?  Might
> make it harder to use incorrectly in future.

For future reference, this is the pattern I introduced for MOZ_ANDROID_BEAM when we futzed with version constants, and I think it works.

Thanks for spotting this, and thanks for making the change, Wes!
(In reply to Richard Newman [:rnewman] from comment #5)
> (In reply to Nick Alexander :nalexander from comment #2)
> 
> > Would it be worth folding the version check into the constant define?  Might
> > make it harder to use incorrectly in future.
> 
> For future reference, this is the pattern I introduced for MOZ_ANDROID_BEAM
> when we futzed with version constants, and I think it works.
> 
> Thanks for spotting this, and thanks for making the change, Wes!

Great minds and all that.  Worth a short mail to mobile-firefox-dev, I think.  (Which I will send.)
Flags: qe-verify-
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 35 → mozilla35
You need to log in before you can comment on or make changes to this bug.