Closed Bug 1258289 Opened 4 years ago Closed 4 years ago

Remove code guarded by AppConstants.Versions.preHC

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 48
Tracking Status
firefox48 --- fixed

People

(Reporter: sebastian, Assigned: sebastian)

References

Details

Attachments

(1 file)

Now that we do not support Gingerbread anymore we can start to remove code that is guarded by AppConstants.Versions.preHC.
Comment on attachment 8732772 [details]
MozReview Request: Bug 1258289 - Remove code guarded by Versions.preHC. r=ahunt,grisha

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41353/diff/1-2/
Comment on attachment 8732772 [details]
MozReview Request: Bug 1258289 - Remove code guarded by Versions.preHC. r=ahunt,grisha

https://reviewboard.mozilla.org/r/41353/#review38037

nice! quick look through dxr suggests that's most (all?) of places where preHC stuff is mentioned; not sure if there are more subtle hacks for gingerbread in the codebase though :)

::: mobile/android/base/java/org/mozilla/gecko/animation/AnimatorProxy.java
(Diff revision 2)
> -
>          // If the view's animation proxy has been overridden from somewhere else, we need to
>          // create a new AnimatorProxy for the view.
> -        if (proxy == null || (needsAnimationProxy && proxy.mImpl != view.getAnimation())) {
> -            AnimatorProxyImpl impl = (needsAnimationProxy ? new AnimatorProxyPreHC(view) :
> +        if (proxy == null) {
> +            AnimatorProxyImpl impl = (new AnimatorProxyPostHC(view));
> -                                                            new AnimatorProxyPostHC(view));

might as well rename the remaining proxy class then, too?

::: mobile/android/base/java/org/mozilla/gecko/preferences/SearchPreferenceCategory.java:25
(Diff revision 2)
>  import org.mozilla.gecko.TelemetryContract;
>  import org.mozilla.gecko.TelemetryContract.Method;
>  import org.mozilla.gecko.util.GeckoEventListener;
>  import org.mozilla.gecko.util.ThreadUtils;
>  
> +import java.util.Random;

Why was this added?
Attachment #8732772 - Flags: review?(gkruglov)
Comment on attachment 8732772 [details]
MozReview Request: Bug 1258289 - Remove code guarded by Versions.preHC. r=ahunt,grisha

https://reviewboard.mozilla.org/r/41353/#review38045
Attachment #8732772 - Flags: review+
https://reviewboard.mozilla.org/r/41353/#review38037

> might as well rename the remaining proxy class then, too?

Yeah, I took a not of that for filing a follow-up bug. We might be able to move the implementation to AnimatorProxy entirely now that there are no multiple implementations anymore.

> Why was this added?

Oops. This is from some code I added for testing. This should not have been added to the patch.
Comment on attachment 8732772 [details]
MozReview Request: Bug 1258289 - Remove code guarded by Versions.preHC. r=ahunt,grisha

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41353/diff/2-3/
Comment on attachment 8732772 [details]
MozReview Request: Bug 1258289 - Remove code guarded by Versions.preHC. r=ahunt,grisha

https://reviewboard.mozilla.org/r/41353/#review38177

Looks good to me!

Do we have separate bugs for cleaning up/merging menus/layouts/etc? (I think those might be more complicated since we'll need to move the 11+ resources into the underlying base resource?)
Attachment #8732772 - Flags: review?(ahunt) → review+
(In reply to Andrzej Hunt :ahunt from comment #7)
> Do we have separate bugs for cleaning up/merging menus/layouts/etc? (I think
> those might be more complicated since we'll need to move the 11+ resources
> into the underlying base resource?)

Yeah, that's on my list too. As far as I know there are no bugs for that filed yet. Basically everything that's *-v11 can go into the base folder and duplicates should be replaced.
https://hg.mozilla.org/mozilla-central/rev/5a706e4cc1f0
https://hg.mozilla.org/mozilla-central/rev/edf13a877da4
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
You need to log in before you can comment on or make changes to this bug.