1213223 - Theme: accentColor is not used everywhere anymore after switching to Theme.AppCompat

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Theme and Visual Design, defect)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

Bug 1208705 introduced an orange accentColor for Android 5+. Now after swichting to Theme.AppCompat in bug 1201206 the color is not used everywhere anymore.

See attached screenshots: Preference headlines use the accentColor but checkboxes and the selected section on the left don't.

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: accentColor-previously.png

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: accentColor-now.png

Comment 3

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Sebastian Kaspari (:sebastian) from comment #2)
> Created attachment 8671813 [details]
> accentColor-now.png

Oh. It looks like the ActionBar color is wrong too.

Looking at v21/themes.xml we probably should drop the "android:" prefix from at least the colors so that components controlled by AppCompat will get the appropriate styling.

http://android-developers.blogspot.de/2014/10/appcompat-v21-material-design-for-pre.html

> When you set these attributes, AppCompat automatically propagates their values to the framework attributes on API 21+. 

So we do not need to duplicate any of those styles - just removing the prefix should be enough.

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Oops – I fixed in this during the rebase in my first backout, but not the second. x_x

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1213223 - Remove android prefix from accentColor. r=sebastian

Bug 1213223 - Remove android prefix from accentColor. r=sebastian

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8671975 [details]
MozReview Request: Bug 1213223 - Remove android prefix from accentColor. r=sebastian

https://reviewboard.mozilla.org/r/21661/#review19469

::: mobile/android/base/resources/values-v21/themes.xml:12
(Diff revision 1)
>          <item name="android:colorPrimary">@color/text_and_tabs_tray_grey</item>
>          <item name="android:colorPrimaryDark">@color/text_and_tabs_tray_grey</item>

I think we should do this for at least those two colors as well.

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Sebastian Kaspari (:sebastian) from comment #6)
> I think we should do this for at least those two colors as well.

I didn't want to spend too much time tweaking parameters that I wasn't sure how it might affect things so I filed bug 1213366.

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://hg.mozilla.org/integration/fx-team/rev/bcebde1a43a2b34db8c29f84cd0b9c7392cef31a
Bug 1213223 - Remove android prefix from accentColor. r=sebastian

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bcebde1a43a2