Bug 1148390 - Dynamically add padding to share icon on GB devices. r=wesj, a=sledru
authorMichael Comella <michael.l.comella@gmail.com>
Mon, 13 Apr 2015 16:27:30 -0700
changeset 267075 098c9a1c1390a9863bc17e5668b042dc7deb8d8d
parent 267074 51ef02cbd50bc1cdf3eef33904e78e5f57a24c2e
child 267076 adf4eb3666ddb7677cbc72f2a2b15d859bcabda8
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswesj, sledru
bugs1148390, 1122752
milestone39.0a2
Bug 1148390 - Dynamically add padding to share icon on GB devices. r=wesj, a=sledru The ideal solution is to remove the whitespace from the action bar drawables (bug 1122752) but since I'm not sure where else these images might be used and how removing the padding will affect all of the different devices and configurations, it would be easier to uplift a solution where we *only* change the padding on the share icon and *only* on the affected devices. However, this solution is dangerous because we hard-code a filename: if we rename ab_share, things will break. Also, unnecessary special casing makes it ugly.
mobile/android/base/TextSelection.java
mobile/android/base/resources/values/dimens.xml
--- a/mobile/android/base/TextSelection.java
+++ b/mobile/android/base/TextSelection.java
@@ -1,26 +1,28 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko;
 
+import android.content.res.Resources;
 import org.mozilla.gecko.gfx.BitmapUtils;
 import org.mozilla.gecko.gfx.BitmapUtils.BitmapLoader;
 import org.mozilla.gecko.gfx.Layer;
 import org.mozilla.gecko.gfx.LayerView;
 import org.mozilla.gecko.gfx.LayerView.DrawListener;
 import org.mozilla.gecko.menu.GeckoMenu;
 import org.mozilla.gecko.menu.GeckoMenuItem;
 import org.mozilla.gecko.EventDispatcher;
 import org.mozilla.gecko.util.FloatUtils;
 import org.mozilla.gecko.util.GeckoEventListener;
 import org.mozilla.gecko.util.ThreadUtils;
 import org.mozilla.gecko.ActionModeCompat.Callback;
+import org.mozilla.gecko.AppConstants.Versions;
 
 import android.content.Context;
 import android.app.Activity;
 import android.graphics.drawable.Drawable;
 import android.view.Menu;
 import android.view.MenuItem;
 
 import org.json.JSONArray;
@@ -274,21 +276,33 @@ class TextSelection extends Layer implem
             int length = mItems.length();
             for (int i = 0; i < length; i++) {
                 try {
                     final JSONObject obj = mItems.getJSONObject(i);
                     final GeckoMenuItem menuitem = (GeckoMenuItem) menu.add(0, i, 0, obj.optString("label"));
                     final int actionEnum = obj.optBoolean("showAsAction") ? GeckoMenuItem.SHOW_AS_ACTION_ALWAYS : GeckoMenuItem.SHOW_AS_ACTION_NEVER;
                     menuitem.setShowAsAction(actionEnum, R.attr.menuItemActionModeStyle);
 
-                    BitmapUtils.getDrawable(anchorHandle.getContext(), obj.optString("icon"), new BitmapLoader() {
+                    final String iconString = obj.optString("icon");
+                    BitmapUtils.getDrawable(anchorHandle.getContext(), iconString, new BitmapLoader() {
                         @Override
                         public void onBitmapFound(Drawable d) {
                             if (d != null) {
                                 menuitem.setIcon(d);
+
+                                // Dynamically add padding to align the share icon on GB devices.
+                                // To be removed in bug 1122752.
+                                if (Versions.preHC && "drawable://ic_menu_share".equals(iconString)) {
+                                    final View view = menuitem.getActionView();
+
+                                    final Resources res = view.getContext().getResources();
+                                    final int padding = res.getDimensionPixelSize(R.dimen.ab_share_padding);
+
+                                    view.setPadding(padding, padding, padding, padding);
+                                }
                             }
                         }
                     });
                 } catch(Exception ex) {
                     Log.i(LOGTAG, "Exception building menu", ex);
                 }
             }
             return true;
--- a/mobile/android/base/resources/values/dimens.xml
+++ b/mobile/android/base/resources/values/dimens.xml
@@ -202,9 +202,13 @@
     <dimen name="find_in_page_text_margin_left">5dip</dimen>
     <dimen name="find_in_page_text_margin_right">12dip</dimen>
     <dimen name="find_in_page_text_padding_left">10dip</dimen>
     <dimen name="find_in_page_text_padding_right">10dip</dimen>
     <dimen name="find_in_page_status_margin_right">10dip</dimen>
     <dimen name="find_in_page_matchcase_padding">10dip</dimen>
     <dimen name="find_in_page_control_margin_top">2dip</dimen>
 
+    <!-- The share icon asset has no padding while the other action bar items do
+         so we dynamically add padding to compensate. To be removed in bug 1122752. -->
+    <dimen name="ab_share_padding">12dp</dimen>
+
 </resources>