Likely a nice touch. Might look a bit weird without any clear separation between statusbar and the tabs tray header though.

I'm still trying to decide if I like these or not, but they definitely feel like a nice touch on first glance! Holding onto that feedback button for now..

I'm a huge fan. I say we land the patch and get feedback, if it's negative, we can always pull it before the train reaches release.

Comment on attachment 8476573 [details] Screenshot with tinted status bar I like this - I'm ok with putting it in and seeing how it feels IRL :P

Actually, another plus for this fading bar is that we can then pad our tabs right up against the status bar and save us some vertical space (more applicable for Tablet's UI). Since, one of the reasons for not doing that right now is that it looks awkward since our background matte is a grey and the statusbar is a black, we needed some room for the top of the tabs to breath. This would probably help that quite a bit... Thoughts?

(In reply to Anthony Lam (:antlam) from comment #6) > Actually, another plus for this fading bar is that we can then pad our tabs > right up against the status bar and save us some vertical space (more > applicable for Tablet's UI). > > Since, one of the reasons for not doing that right now is that it looks > awkward since our background matte is a grey and the statusbar is a black, > we needed some room for the top of the tabs to breath. This would probably > help that quite a bit... Thoughts? Good point but keep in mind that status bar tinting is only available in KitKat and later Android releases. We'll have to add top padding in the tab strip on pre-KitKat.

Comment on attachment 8495321 [details] [diff] [review] Merge picasso and nineoldandroids jars into one (r=nalexander) I think it's fine for search activity to link with Picasso too (even though it's not really using it) as we're not planning to ship it in a separate APK anyway.

Comment on attachment 8495322 [details] [diff] [review] Tint the status bar on KitKat+ (r=mfinkle) I assume that Proguard would remove the unused Java code during it's optimization run, so we don't need build flags in the moz.build file.

Forgot the say it: the reason I'm merging these libraries into a single jar is that I'll be integrating some smaller libraries soon and we don't really need to have separate jars for each 3rd-party library. We can split them up if we end up needing more granularity in the future.

(In reply to Mark Finkle (:mfinkle) from comment #11) > Comment on attachment 8495322 [details] [diff] [review] > Tint the status bar on KitKat+ (r=mfinkle) > > I assume that Proguard would remove the unused Java code during it's > optimization run, so we don't need build flags in the moz.build file. I'm assuming proguard is able to remove unreachable code like the stuff in setupSystemUITinting(). rnewman?

Comment on attachment 8495321 [details] [diff] [review] Merge picasso and nineoldandroids jars into one (r=nalexander) Review of attachment 8495321 [details] [diff] [review]: ----------------------------------------------------------------- lgtm. For bonus points, include a comment at the declaration of gecko-thirdparty explaining that this is a good place to put small independent libraries.

(In reply to Lucas Rocha (:lucasr) from comment #13) > I'm assuming proguard is able to remove unreachable code like the stuff in > setupSystemUITinting(). rnewman? javac itself will remove the unreachable code if Versions.feature19Plus is known to be false during the build. ProGuard should do it if javac didn't know for sure. ProGuard might or might not eliminate the unused class (it should, but it's public…); this is something I'd verify experimentally rather than trying pure reason :D If you want to test this, make a backup of your .mozconfig, change the object directory in there, and add ac_add_options --with-android-min-sdk=9 ac_add_options --with-android-max-sdk=9 You can then do a build and take a look at the output in the new object directory, see which classes make it through, and use javap to see what bytecode javac produces. You can also look at $objdir/mobile/android/base/jars-proguarded to see the post-ProGuard output. jar tf $objdir/mobile/android/base/jars-proguarded/gecko-browser.jar | fgrep MyClass will tell you if ProGuard decided to eliminate the redundant class.

Tried using --with-android-min-sdk=9 & --with-android-max-sdk=9 and found out that the new tablet UI is breaking the build with these flags :-/ We'll have to add alias/stubs for a bunch of resources. Filed bug 1073474 to track this. I'll double-check that the tint manager is not present once I fix bug 1073474. I'll just land it now as it's just a single class anyway.

(In reply to Nick Alexander :nalexander from comment #14) > Comment on attachment 8495321 [details] [diff] [review] > Merge picasso and nineoldandroids jars into one (r=nalexander) > > Review of attachment 8495321 [details] [diff] [review]: > ----------------------------------------------------------------- > > lgtm. For bonus points, include a comment at the declaration of > gecko-thirdparty explaining that this is a good place to put small > independent libraries. Done.

https://hg.mozilla.org/integration/fx-team/rev/3fb6805983a0 https://hg.mozilla.org/integration/fx-team/rev/c56275d516ec

https://hg.mozilla.org/mozilla-central/rev/3fb6805983a0 https://hg.mozilla.org/mozilla-central/rev/c56275d516ec

It does not work as expected with the HTC One, see the screenshot.

(In reply to Sören Hentzschel from comment #20) > Created attachment 8496429 [details] > android.png > > It does not work as expected with the HTC One, see the screenshot. I filed a bug, bug 1073845

This has caused quite a few regressions (see the depedent bugs) and, in particular, makes our fullscreen experience pretty unpolished. I propose we back it out and come up with a more refined solution. We are currently using the SystemBarTintManager [1] which does not gracefully handle toggling fullscreen modes (causing many graphical glitches) so we'd probably have to roll our own solution (that can better integrate with our fullscreen mode). In the mean time, it's built into the system on Lollipop so we can get easy wins there. What do you think, Mark? We currently ship the tint in 35 and 36, but can backout starting with 37. This also has the benefit of simplifying the uplifts I'd have to make to fix some of the other fullscreen bugs tracking 37 that I'm assigned to. [1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/thirdparty/com/readystatesoftware/systembartint/SystemBarTintManager.java

(In reply to Michael Comella (:mcomella) from comment #22) > What do you think, Mark? I agree. A backout seems like the best option for now.

/r/4279 - Bug 1056002 - Backout changeset c56275d516ec. r=mfinkle Pull down this commit: hg pull review -r 5448050404c59576081a8e16ebbe3725fe41836c

Comment on attachment 8568867 [details] MozReview Request: bz://1056002/mcomella https://reviewboard.mozilla.org/r/4277/#review3443 Ship It!

Comment on attachment 8568867 [details] MozReview Request: bz://1056002/mcomella Backout of this bug: note the fix is already in release (35 & 36) so we'll be taking away the toolbar tint from users. Approval Request Comment [Feature/regressing bug #]: This one. [User impact if declined]: All of the unclosed dependent bugs remain broken - these are largely fullscreen mode polish issues. Toolbar tint will remain. [Describe test coverage new/current, TreeHerder]: None [Risks and why]: Low - backing out a feature that interfaces through an instance that we no longer access. The risk is slightly higher because I had to back out both bug 1074924 and bug 1076692, but I did it by hand in order to make the compile work because I didn't realize the deps. [String/UUID change made/needed]: None

(backout is comment 26)

Actually, this is a lot of extra work for little gain (i.e. this only works in KitKat) - I want to call WONTFIX. Anthony, thoughts?

Hm, this actually helped our toolbar feel less cramped in many ways and was a nice touch I felt. That being said, I'm ok with this proposal for it's impact on full screen mode and complexity. But, for the record can I get the effects in a screenshot here? Also, what (solid color im guessing) are you proposing we switch this to?

(In reply to Anthony Lam (:antlam) from comment #31) > But, for the record can I get the effects in a screenshot here? Also, what > (solid color im guessing) are you proposing we switch this to? Here's the breakdown: * Pre KitKat - no control over the status bar color * KitKat - gradient status bar color possible, with some effort * Lollipop - status bar color is simple, gradient status bar color possible with same efforts. Note that a gradient status bar would break Android convention This may change in the future if Google backports some of their Lollipop APIs to earlier versions, where a solid status bar color is possible (but perhaps unlikely). The reason I don't think it's worth the effort is that I imagine many (most?) KitKat devices will get the bump to Lollipop. In Lollipop, we're adding the tint in bug 1137240 [2], which is tabs tray grey. Note that I also found a blog post [1] that makes doing the gradient color easy on KitKat only, but for an ugly perf hit. [1]: https://gauntface.com/blog/2014/01/10/translucent-theme-in-android/ [2]: https://bug1137240.bugzilla.mozilla.org/attachment.cgi?id=8570225

Fixing status: the merge was a backout, not a resolution. FWIW, the status bar turning grey or red in some apps is one of the things I hate about Lollipop. The color never matches (deliberately!), and notification bar icons typically don't work well with every color. I don't like the idea of spending time trying to get this working on KK and earlier, especially when previous attempts are known to cause regressions.

I'm ok with just using a solid "Text and Tabs tray grey" as a color for the status bar.

(In reply to Anthony Lam (:antlam) from comment #34) > I'm ok with just using a solid "Text and Tabs tray grey" as a color for the > status bar. For now, that's only on L and up.

Comment on attachment 8568867 [details] MozReview Request: bz://1056002/mcomella Approving backout - this is nice and early in Beta so hopefully we will catch anything missed in the backout.

Note that bug 1137240 should be uplifted along with this (it doesn't cause crashes so it doesn't have to be simultaneous).

Glad to hear that since bug 1137240 hasn't even landed or had approval requests yet.

(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #38) > Glad to hear that since bug 1137240 hasn't even landed or had approval > requests yet. It's not a tight dependency so I'm not too concerned - bug 1137240 exists because this bug was backed out but that being said, getting this backout landed is more important than getting them landed together.