850946 - Dark holo theme with Sync UI on Nexus 7/TF201

Status: VERIFIED FIXED

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

Description

[Aaron Train [:aaronmt]]

Attachment: Nexus 7 (Nightly (03/13) - Sync UI)

For whatever reason the Sync UI on the Nexus 7 (Android 4.2.2) is using a dark holo theme, but the same Android version (stock Android) on my Nexus 4 (Android 4.2.2) is using the light holo theme.

What?

See screenshot.

--
Nexus 7 (Android 4.2.2)
Nightly (03/13)

Comment 1

[Adrian Tamas (:AdrianT)]

This is also an issue on Firefox Mobile 21 beta 1 on the Samsung Galaxy Nexus (Android 4.1.1) - although the button Nexus is visible - on the Asus EEE Transformer TF101 (Android 4.0.4) and Samsung Galaxy Tab 2 (Android 4.1.1)

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Setting fx21 as affected based on comment 1

Sriram - What do we need to do to fix this?

Comment 3

[Sriram Ramasubramanian [:sriram]]

The Dialog is using a Holo (Dark) theme. The title on top uses a pre-defined style that takes care of Holo theme. However, the bottom button uses a Holo.Light theme. Is there a reason for this?

Comment 4

[Wesley Johnston (:wesj)]

These are using Gecko.Dialog as their theme:

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/values/themes.xml#13

We need to update that to inherit from something like Theme.Holo.Light.Dialog? or maybe better Theme.Light.Panel?

Comment 5

[Sriram Ramasubramanian [:sriram]]

(In reply to Wesley Johnston (:wesj) from comment #4)
> These are using Gecko.Dialog as their theme:
> 
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/
> values/themes.xml#13
> 
> We need to update that to inherit from something like
> Theme.Holo.Light.Dialog? or maybe better Theme.Light.Panel?

I saw that too. But then, the dialogs ok. The title has a blue line as intented and hence didn't comment about it.

Comment 6

[Wesley Johnston (:wesj)]

Attachment: Patch

Simple fix

Comment 7

[Wesley Johnston (:wesj)]

Attachment: Screnshot on S3

Had to hack to make us use this style on a phone, but works.

Comment 8

[Sriram Ramasubramanian [:sriram]]

(In reply to Wesley Johnston (:wesj) from comment #6)
> Created attachment 732462 [details] [diff] [review]
> Patch
> 
> Simple fix

I believe this should done in Sync's git clone and then moved in here. Doing directly in central is not something that Sync likes (am I right?)

Comment 9

[Nick Alexander :nalexander [he/him]]

(In reply to Sriram Ramasubramanian [:sriram] from comment #8)
> (In reply to Wesley Johnston (:wesj) from comment #6)
> > Created attachment 732462 [details] [diff] [review]
> > Patch
> > 
> > Simple fix
> 
> I believe this should done in Sync's git clone and then moved in here. Doing
> directly in central is not something that Sync likes (am I right?)

You are correct.  I will take a look and test this fix this afternoon.

Comment 10

[Nick Alexander :nalexander [he/him]]

Thanks for thinking of Sync, sriram, but in this case Fennec owns values/themes.xml and values-v11/themes.xml.  And this looks sensible to me, but I don't have a device to test on.

I've pushed

https://github.com/mozilla-services/android-sync/commit/a3afbf1124d534217cfd50e2fe65a5f64176a950

to harmonize Android Sync's non-authoritative copies of themes.xml with Fennec's, including this change.  So please land it soon, so I don't have to roll back :)

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 732462 [details] [diff] [review]
Patch

Adding a reviewer. Nick wants this reviewed and landed soon :)

Comment 12

[Nick Alexander :nalexander [he/him]]

This is ready to land on trunk.

Comment 13

[Wesley Johnston (:wesj)]

Unbitrotted and landed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1d30583cd42

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/05730b9af975
https://hg.mozilla.org/mozilla-central/rev/1d30583cd42f

Comment 15

[Aaron Train [:aaronmt]]

TF201 affected along with other Acer Tablets running ICS. Any chance to get this uplifted?

Comment 16

[Adrian Tamas (:AdrianT)]

This is also an issue on the Asus EEE Transformer TF101 (Android 4.0.4) and the Samsung Galaxy Tab 2 (Android 4.1.1) on Firefox Mobile 22 beta 3

Comment 17

[Alex Keybl [:akeybl]]

No need to track, but we would accept a low risk uplift.