1261041 - Remove feature11Plus - feature15Plus flags

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

All code is 15+ now. Those flags shouldn't be needed anymore:
https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/AppConstants.java.in#50-54

Comment 1

[Tushar Saini (:shatur)]

Attachment: Raw.patch

Hi Sebastian

  I have some queries related to this bug. Like, removing constant flag feature11Plus, do I have to remove all related code or do I have to change its occurrences[everywhere] to feature15Plus[as feature11Plus also includes feature15]?
  I have uploaded a raw patch. Please let me know, if I am on right track.

Thanks!!

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

Hey shatur! :)

Our minSdkVersion is set to 15. So all checks for feature11Plus, ... feature15Plus will return true always. So you can remove the checks and the constants. No need to do any checks for this anymore.

Comment 3

[Tushar Saini (:shatur)]

Attachment: Bug 1261041 - Remove feature11Plus-feature15Plus flags.

Review commit: https://reviewboard.mozilla.org/r/80448/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/80448/

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8793828 [details]
Bug 1261041 - Remove feature11Plus-feature15Plus flags.

https://reviewboard.mozilla.org/r/80448/#review80660

Thanks for the patch, Shatur. And sorry for delay reviewing it.

The patch is looking very good already. There are some things we need to clean up and then I think it's ready to land. :)

::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:3365
(Diff revision 1)
>              findInPage.setEnabled(false);
>  
>              // NOTE: Use MenuUtils.safeSetEnabled because some actions might
>              // be on the BrowserToolbar context menu.
> -            if (Versions.feature11Plus) {
> -                // There is no page menu prior to v11 resources.
> +            // There is no page menu prior to v11 resources.

We can remove this comment too now.

::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:3447
(Diff revision 1)
> -        }
>          MenuUtils.safeSetEnabled(aMenu, R.id.subscribe, tab.hasFeeds());
>          MenuUtils.safeSetEnabled(aMenu, R.id.add_search_engine, tab.hasOpenSearch());
>          MenuUtils.safeSetEnabled(aMenu, R.id.add_to_launcher, !isAboutHome(tab));
>  
>          // Action providers are available only ICS+.

This comment is now outdated too.

::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:3583
(Diff revision 1)
>                  if (item.isChecked()) {
>                      Telemetry.sendUIEvent(TelemetryContract.Event.UNSAVE, TelemetryContract.Method.MENU, extra);
>                      tab.removeBookmark();
>                      item.setTitle(resolveBookmarkTitleID(false));
> -                    if (Versions.feature11Plus) {
> -                        // We don't use icons on GB builds so not resolving icons might conserve resources.
> +                    // We don't use icons on GB builds so not resolving icons might conserve resources.

Outdated comment

::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:3589
(Diff revision 1)
>                  } else {
>                      Telemetry.sendUIEvent(TelemetryContract.Event.SAVE, TelemetryContract.Method.MENU, extra);
>                      tab.addBookmark();
>                      item.setTitle(resolveBookmarkTitleID(true));
> -                    if (Versions.feature11Plus) {
> -                        // We don't use icons on GB builds so not resolving icons might conserve resources.
> +                    // We don't use icons on GB builds so not resolving icons might conserve resources.

Outdated comment

::: mobile/android/base/java/org/mozilla/gecko/DoorHangerPopup.java:326
(Diff revision 1)
>          // Make the popup focusable for accessibility. This gets done here
>          // so the node can be accessibility focused, but on pre-ICS devices this
>          // causes crashes, so it is done after the popup is shown.

Outdated comment

::: mobile/android/base/java/org/mozilla/gecko/db/AbstractTransactionalProvider.java:97
(Diff revision 1)
>      /**
>       * Return true if OS version and database parallelism support indicates
>       * that this provider should bundle writes into transactions.
>       */
>      @SuppressWarnings("static-method")
>      protected boolean shouldUseTransactions() {

This method is now obsolete (It only returns true after all). We could remove it. Maybe not in this bug but we could file a new one for it.

::: mobile/android/base/java/org/mozilla/gecko/tabqueue/TabQueueHelper.java:40
(Diff revision 1)
>  import java.util.List;
>  
>  public class TabQueueHelper {
>      private static final String LOGTAG = "Gecko" + TabQueueHelper.class.getSimpleName();
>  
>      // Disable Tab Queue for API level 10 (GB) - Bug 1206055

Outdated comment - This flag is now obsolete. We coulkd file a follow-up bug to remove it.

::: mobile/android/base/java/org/mozilla/gecko/widget/DateTimePicker.java:104
(Diff revision 1)
>      public class OnValueChangeListener implements NumberPicker.OnValueChangeListener {
>          @Override
>          public void onValueChange(NumberPicker picker, int oldVal, int newVal) {
>              updateInputState();
>              mTempDate.setTimeInMillis(mCurrentDate.getTimeInMillis());
> -            final boolean newBehavior = Versions.feature11Plus;
> +            final boolean newBehavior = true;

newBehavior is now unneeded and you can remove more of the code.

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/Clipboard.java:66
(Diff revision 1)
>  
>      @WrapForJNI(calledFrom = "gecko")
>      public static void setText(final CharSequence text) {
>          ThreadUtils.postToBackgroundThread(new Runnable() {
>              @Override
>              @SuppressWarnings("deprecation")

This annotation can now be removed after we removed the deprecated code, I guess.

Comment 5

[Tushar Saini (:shatur)]

Comment on attachment 8793828 [details]
Bug 1261041 - Remove feature11Plus-feature15Plus flags.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/80448/diff/1-2/

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8793828 [details]
Bug 1261041 - Remove feature11Plus-feature15Plus flags.

https://reviewboard.mozilla.org/r/80448/#review81060

This is looking good! Thanks for this patch. I'll trigger a try run from reviewboard.

Comment 7

[Sebastian Kaspari (:sebastian; :pocmo)]

There's only one unrelated failure on try. This can land.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Please close out the two open issues in MozReview so that this can autoland.

Comment 9

[Tushar Saini (:shatur)]

Closed the issues in mozreview.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7914b9afee68
Remove feature11Plus-feature15Plus flags. r=sebastian

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7914b9afee68