702306 - The selected Awesomescreen tab's text color is black instead of white

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P4)

Description

[Cristian Nicolae (:xti)]

Attachment: screenshot

Mozilla/5.0 (Android;Linux armv7l;rv:10.0a1)Gecko/20111113
Firefox/10.0a1 Fennec/10.0a1
Devices: Motorola Droid 2
OS: Android 2.3.3

Steps to reproduce:
1. Open Fennec App
2. Tap on URL bar
3. Tap on Bookmarks tab

Expected result:
The selected tab's text color is white.

Actual result:
The selected tab's text color is black as you can see in the attached screenshot.

Comment 1

[Lucas Rocha (:lucasr)]

Attachment: Ensure we use sane text colours in AwesomeBar tabs

Comment 2

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

Comment on attachment 574620 [details] [diff] [review]
Ensure we use sane text colours in AwesomeBar tabs

Can we put this file in resources/values ? I see that we have some color values in colors.xml already, even if they are not used in Fennec Native. Could we move this file into that folder? Or does it make more sense to put all the file types (themes, styles, etc) in separate folders?

Comment 3

[Lucas Rocha (:lucasr)]

(In reply to Mark Finkle (:mfinkle) from comment #2)
> Comment on attachment 574620 [details] [diff] [review] [diff] [details] [review]
> Ensure we use sane text colours in AwesomeBar tabs
> 
> Can we put this file in resources/values ? I see that we have some color
> values in colors.xml already, even if they are not used in Fennec Native.
> Could we move this file into that folder? Or does it make more sense to put
> all the file types (themes, styles, etc) in separate folders?

The general Android standard is to have lists of individual values unders the "values" directory and resource files into different directories (i.e. images and selectors under "drawables*", color selectors under "color", etc). So, I think we should follow that.

Comment 4

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

(In reply to Lucas Rocha (:lucasr) from comment #3)
> (In reply to Mark Finkle (:mfinkle) from comment #2)
> > Comment on attachment 574620 [details] [diff] [review] [diff] [details] [review] [diff] [details] [review]
> > Ensure we use sane text colours in AwesomeBar tabs
> > 
> > Can we put this file in resources/values ? I see that we have some color
> > values in colors.xml already, even if they are not used in Fennec Native.
> > Could we move this file into that folder? Or does it make more sense to put
> > all the file types (themes, styles, etc) in separate folders?
> 
> The general Android standard is to have lists of individual values unders
> the "values" directory and resource files into different directories (i.e.
> images and selectors under "drawables*", color selectors under "color",
> etc). So, I think we should follow that.

OK. Can you also remove the old values/colors.xml since I think we do not use it anymore.

Comment 5

[Lucas Rocha (:lucasr)]

(In reply to Mark Finkle (:mfinkle) from comment #4)
> (In reply to Lucas Rocha (:lucasr) from comment #3)
> > (In reply to Mark Finkle (:mfinkle) from comment #2)
> > > Comment on attachment 574620 [details] [diff] [review] [diff] [details] [review] [diff] [details] [review] [diff] [details] [review]
> > > Ensure we use sane text colours in AwesomeBar tabs
> > > 
> > > Can we put this file in resources/values ? I see that we have some color
> > > values in colors.xml already, even if they are not used in Fennec Native.
> > > Could we move this file into that folder? Or does it make more sense to put
> > > all the file types (themes, styles, etc) in separate folders?
> > 
> > The general Android standard is to have lists of individual values unders
> > the "values" directory and resource files into different directories (i.e.
> > images and selectors under "drawables*", color selectors under "color",
> > etc). So, I think we should follow that.
> 
> OK. Can you also remove the old values/colors.xml since I think we do not
> use it anymore.

Maybe I wasn't clear. The values/colors.xml is still needed to define individual colours. The file that I added in this patch (color/awesomebar_tab_text.xml) is a color selector which has to be defined in a separate file. The standard location for this type of file is under the "color" directory.

Comment 6

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

(In reply to Lucas Rocha (:lucasr) from comment #5)

> Maybe I wasn't clear. The values/colors.xml is still needed to define
> individual colours. The file that I added in this patch
> (color/awesomebar_tab_text.xml) is a color selector which has to be defined
> in a separate file. The standard location for this type of file is under the
> "color" directory.

Indeed. I was confused. OK, still r+.

Comment 7

[Lucas Rocha (:lucasr)]

Pushed: http://hg.mozilla.org/projects/birch/rev/644b8a0b7f9d

Comment 8

[Catalin Suciu [:csuciu]]

Verified fixed on:
Mozilla/5.0 (Android;Linux armv7l;rv:11.0a1)Gecko/20111117 Firefox/11.0a1 Fennec/11.0a1