Closed Bug 1137240 Opened 9 years ago Closed 9 years ago

Color android status bar with tabs tray grey on Android L

Categories

(Firefox for Android Graveyard :: General, defect)

38 Branch
ARM
Android
defect
Not set
normal

Tracking

(firefox37 unaffected, firefox38+ verified, firefox39 verified, fennec38+)

VERIFIED FIXED
Firefox 39
Tracking Status
firefox37 --- unaffected
firefox38 + verified
firefox39 --- verified
fennec 38+ ---

People

(Reporter: aaronmt, Assigned: mcomella)

References

Details

(Keywords: regression, reproducible)

Attachments

(4 files, 1 obsolete file)

Attached image screenshot.png
See screenshot.

Thinking this is a regression from backing out the semi-transparent patch.

In the screenshot, we should have a black bar on Android 5.0. With it now, the browser looks out of place on the platform.

--
Nexus 6 (Android 5.0.1)
Flags: needinfo?(michael.l.comella)
I think this is a regression from bug 1097337 (inherit from Material theme), uncovered by backing out bug 1056002 (tinting the status bar).
Assignee: nobody → michael.l.comella
Flags: needinfo?(michael.l.comella)
I can't repro on my N9.

I imagine this is related to android:colorPrimaryDark [1] not being set (and perhaps being different across Nexus devices? ew).

[1]: https://developer.android.com/training/material/theme.html#StatusBar
But then again, I don't see `WindowManager.LayoutParams.FLAG_DRAWS_SYSTEM_BAR_BACKGROUNDS` and `WindowManager.LayoutParams.FLAG_TRANSLUCENT_STATUS`, or their XML equivalents, defined anywhere (which allow us to affect the system bar colors).

Aaron, can you repro on any other devices? e.g. N5
Flags: needinfo?(aaron.train)
tracking-fennec: ? → 39+
Yeah reproducible on my Nexus 5 as well, alongside my other Android 5.0 devices.

Don't have any 4.x on me, what does it look like?
Flags: needinfo?(aaron.train)
Got my S5, I see black. Looks like an Android 5.0 specific bug to me.
Upgraded N4 to 5.0.1 and I can repro the issue.
/r/4405 - Bug 1137240 - Specify primaryColorDark in v21 themes.xml to set status bar color. r=liuche

Pull down this commit:

hg pull review -r d9029fa465ad1de6c80b89916fbdc399dbec522b
Attachment #8570222 - Flags: review?(liuche)
Solution is to apply proper tint color on Lollipop. It still doesn't work on my N9 but I'm going to guess it's an Android 5 bug. It has the dark system status bar so it doesn't look terrible.
Attached image Post patch screenshot
Status bar tint cool with you, Anthony?
Attachment #8570225 - Flags: feedback?(alam)
(In reply to Michael Comella (:mcomella) from comment #8)
> Solution is to apply proper tint color on Lollipop.

I'm assuming the reason is because the default system status bar color is that grey color (perhaps because we inherit from Material light?).
Comment on attachment 8570225 [details]
Post patch screenshot

Looks good to me! Though, this isn't a tint so much as a solid color right? or are my eyes deceiving me...
Attachment #8570225 - Flags: feedback?(alam) → feedback+
(In reply to Anthony Lam (:antlam) from comment #12)
> Looks good to me! Though, this isn't a tint so much as a solid color right?

Yes - solid colors are Android L convention. See bug 1056002 comment 32 for more details.
This regression is also in Aurora 38.  Should this be nominated for tracking-fennec:38+ and Aurora uplift?

(In reply to Michael Comella (:mcomella) from comment #9)
> Status bar tint cool with you, Anthony?

I think we should also consider going back to black for the status bar, since the status bar is still visible even when the url bar disappears, and any color besides black looks a bit weird in that case.  (It feels less "full screen.")
Version: Trunk → Firefox 38
For comparison: Chrome always uses black for the status bar.  Apps like YouTube and Play Books change the status bar to black when you enter their "content" views.
Flags: needinfo?(michael.l.comella)
(In reply to Matt Brubeck (:mbrubeck) from comment #14)
> I think we should also consider going back to black for the status bar,
> since the status bar is still visible even when the url bar disappears, and
> any color besides black looks a bit weird in that case.  (It feels less
> "full screen.")

See this screenshot for comparison - I think our tabs tray color is dark enough that the contrast to page content looks good though I'll leave the final decision to Anthony. 

(In reply to Matt Brubeck (:mbrubeck) from comment #15)
> For comparison: Chrome always uses black for the status bar.  Apps like
> YouTube and Play Books change the status bar to black when you enter their
> "content" views.

Good sleuthing - we can also try doing something to this effect.
Flags: needinfo?(michael.l.comella)
renom to track 38 (see comment 14).
tracking-fennec: 39+ → ?
Blocks: android-l
Summary: Regression: android status-bar theme colour is not adhered to platform → Color android status bar with tabs tray grey on Android L
Comment on attachment 8570222 [details]
MozReview Request: bz://1137240/mcomella

https://reviewboard.mozilla.org/r/4403/#review3803

Ship It!
Attachment #8570222 - Flags: review?(liuche) → review+
Comment on attachment 8570222 [details]
MozReview Request: bz://1137240/mcomella

Approval Request Comment
[Feature/regressing bug #]: bug 1097337
[User impact if declined]:
  Users on android L will have an inconsistent grey status bar - this patch makes it fit the system.

[Describe test coverage new/current, TreeHerder]: None

[Risks and why]:
  Extremely low - we just add a "colorPrimaryDark" attribute which is only used by the system to set the status bar color. Since we don't reference the attribute, nothing else should change.

[String/UUID change made/needed]: None
Attachment #8570222 - Flags: approval-mozilla-aurora?
Darrin, How critical from a ux perspective is this, for uplift to aurora ?

[Tracking Requested - why for this release]:  This was nominated for tracking and uplift on bug 1139237 so I'm moving the nomination here.
Flags: needinfo?(dhenein)
tracking-fennec: ? → 38+
From my perspective, its something I'd certainly like to see uplifted. Its a pretty major regression in our UI, and will be clearly noticeable.
Flags: needinfo?(dhenein)
https://hg.mozilla.org/mozilla-central/rev/97cf1b0eb9da
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
OK, thanks Darrin!  Tracking this for 38.
Comment on attachment 8570222 [details]
MozReview Request: bz://1137240/mcomella

Approving this for uplift to 38 since it's a noticeable UI regression and looks low-risk.
Attachment #8570222 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
v. Firefox for Android 39 and 38
Status: RESOLVED → VERIFIED
Attachment #8570222 - Attachment is obsolete: true
Attachment #8619602 - Flags: review+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: