Bug 1148390 - Dynamically add padding to share icon on GB devices. r=wesj
authorMichael Comella <michael.l.comella@gmail.com>
Mon, 13 Apr 2015 16:27:30 -0700
changeset 270493 73daa8f0f5f0f2b057ebddd89af6f76132d5e16f
parent 270492 cdebac12ce27874f392ce8f1cde77f975606b8ca
child 270494 3c1b43d1b703746a212b9050ddbad6dd0f5e7579
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswesj
bugs1148390, 1122752
milestone40.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1148390 - Dynamically add padding to share icon on GB devices. r=wesj 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
@@ -208,16 +208,20 @@
     <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>
+
     <!-- This is a 4:7 ratio (as per UX decision). -->
     <item name="thumbnail_aspect_ratio" format="float" type="dimen">0.571</item>
 
     <!-- http://blog.danlew.net/2015/01/06/handling-android-resources-with-non-standard-formats/ -->
     <item name="match_parent" type="dimen">-1</item>
     <item name="wrap_content" type="dimen">-2</item>
 
 </resources>