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 258491 e10ddd2bc05f
parent 258490 58f203b17be2
child 258492 7b296a71b115
push id4679
push userryanvm@gmail.com
push date2015-04-15 17:06 +0000
treeherdermozilla-beta@6c7e8d9f955c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswesj, sledru
bugs1148390, 1122752
milestone38.0
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;
@@ -272,21 +274,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
@@ -204,13 +204,17 @@
     <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>
+
     <!-- Use android attr in dimens, via http://stackoverflow.com/a/19043952 -->
     <dimen name="custom_match_parent">-2px</dimen>
     <dimen name="custom_wrap_content">-1px</dimen>
 
 </resources>