Is it me or is this enormous now after the patch?

Worth noting that other applications (I checked Chrome & gmail) do not put the application icon in their settings menus though they did in JB. Do you still want to include the icon, Anthony? I like it as a way of branding. I think the icon might be larger because the ActionBar height may have increased - I'm looking for a way to style the icon without writing a custom ActionBar layout.

(In reply to Michael Comella (:mcomella) from comment #1) > I think the icon might be larger because the ActionBar height may have > increased Actually, it looks like it might be the same size - the changed layout just makes it look funky. If we want to change the icon size, I think we have a few options: * Custom layout (too much work :|) * New assets (too much memory) * Inset drawable as the icon We can use android:logo, rather than android:icon, as an element that only gets put into the ActionBar.

For comparison's sake.

Anthony, do we want to keep the icon the same size in the non-Android L builds?

I added 6dp of padding in all directions on the icon. Anthony, what do you think?

Comment on attachment 8585796 [details] Post patch (6dp) WFM! what size does that make this icon? Also, what's the size of the margin between the <- and the build icon?

I'm open to keeping it for now.

(In reply to Anthony Lam (:antlam) from comment #6) > what size does that make this icon? > > Also, what's the size of the margin between the <- and the build icon? I tried digging into the source but it's non-obvious. For anyone following in my footsteps, it's somewhere around: * screen_action_bar.xml: http://androidxref.com/5.1.0_r1/xref/frameworks/base/core/res/res/layout/screen_action_bar.xml * action_bar_home.xml: http://androidxref.com/5.1.0_r1/xref/frameworks/base/core/res/res/layout/action_bar_home.xml * ActionBarView.java: http://androidxref.com/5.1.0_r1/xref/frameworks/base/core/java/com/android/internal/widget/ActionBarView.java#63 * Widget.ActionBar: http://androidxref.com/5.1.0_r1/xref/frameworks/base/core/res/res/values/styles.xml#1193

/r/6381 - Bug 1132751 - Remove redundant ActionBar home setting. r=liuche /r/6383 - Bug 1132751 - Add android:logo to fennec application. r=liuche Pull down these commits: hg pull -r 93722e1935fa363e527e230b5498339f93f2f911 https://reviewboard-hg.mozilla.org/gecko/

Comment on attachment 8586371 [details] MozReview Request: bz://1132751/mcomella https://reviewboard.mozilla.org/r/6379/#review5339

https://reviewboard.mozilla.org/r/6381/#review5335 ::: mobile/android/base/preferences/GeckoPreferences.java (Diff revision 1) > - actionBar.setHomeButtonEnabled(true); For some reason I thought this was related to some v14+ bug: https://code.google.com/p/android/issues/detail?id=58007 But this doesn't even seem to be the fix, so if the "back" button works on, say, v14 devices for going "up", feel free to remove it.

https://reviewboard.mozilla.org/r/6383/#review5337 ::: mobile/android/base/resources/drawable/logo.xml (Diff revision 1) > + android:src="@drawable/icon"/> Was the sizing of the logo fine on other devices? It looked pretty big on JB too.

https://reviewboard.mozilla.org/r/6383/#review5341 > Was the sizing of the logo fine on other devices? It looked pretty big on JB too. Anthony okayed this in comment 7.

https://reviewboard.mozilla.org/r/6381/#review5343 > For some reason I thought this was related to some v14+ bug: > https://code.google.com/p/android/issues/detail?id=58007 > > But this doesn't even seem to be the fix, so if the "back" button works on, say, v14 devices for going "up", feel free to remove it. The up button works as expected on my 4.4 N7 and my 5.0 N4 after these changes.

https://hg.mozilla.org/integration/fx-team/rev/ae9a28f9f736 https://hg.mozilla.org/integration/fx-team/rev/e9e89c1c7e41

Comment on attachment 8586371 [details] MozReview Request: bz://1132751/mcomella Approval Request Comment [Feature/regressing bug #]: bug 1097337 [User impact if declined]: Users will see an awkwardly large Firefox asset (specific to build type) in the Settings menu (i.e. polish) [Describe test coverage new/current, TreeHerder]: None [Risks and why]: Short answer: Android L users may see a smaller application logo in some places. Long answer: We add a new attribute to the base level of the application, android:logo. This appears to only be used in the ActionBar which, to my knowledge, we only use in the Settings menu. On Android L+, this icon has some extra padding over the standard application icon (i.e. the firefox logo) which means it'll appear smaller in places where android:logo is used in the place of android:icon. Again, I think this is just ActionBar but if I'm wrong, Android L+ users may see a smaller application logo in those places. [String/UUID change made/needed]: None

https://hg.mozilla.org/mozilla-central/rev/ae9a28f9f736 https://hg.mozilla.org/mozilla-central/rev/e9e89c1c7e41

https://hg.mozilla.org/releases/mozilla-aurora/rev/4ad0c7cad3bf https://hg.mozilla.org/releases/mozilla-aurora/rev/445036402091

https://hg.mozilla.org/releases/mozilla-beta/rev/8be55cae236f https://hg.mozilla.org/releases/mozilla-beta/rev/b28b502e4aca

Verified as fixed on Nexus 7 (5.0.2) on Firefox 38 Beta 2 and latest Aurora 39.0a2 , Nightly 40.0a1