Use our color palette for toolbar and URL bar text

RESOLVED FIXED in Firefox 48

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: antlam, Assigned: k.krish, Mentored)

Tracking

(Blocks: 2 bugs)

unspecified
Firefox 48
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 fixed)

Details

(Whiteboard: [lang=java][good next bug])

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

3 years ago
The current text colors in our URL bar don't match anything in our palette: https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/values/colors.xml?from=colors.xml#113-116

Let's clean these up - 

Normal browsing: 
TLD - #363B40 Text and tabs tray grey 
text - #AFB1B3 tabs tray icon grey

Private browsing:
TLD - #FFFFFF white
text - #777777 placeholder grey
Mentor: michael.l.comella
Whiteboard: [lang=java][good next bug]
Hi.
  I would like to work on this bug. Can you just point me to right direction by suggesting, which files I have to work on?

Thanks.
Flags: needinfo?(michael.l.comella)
(In reply to shatur from comment #1)
> Hi.
>   I would like to work on this bug. Can you just point me to right direction
> by suggesting, which files I have to work on?
> 
> Thanks.

Hey Shatur.

It looks like this bug now relies on bug 1236431 to be figured out first before we can decide to fix or wontfix this bug – perhaps you'd like to take a look at a different bug first? May I recommend bug 1144965 or bug 1130809?
Flags: needinfo?(michael.l.comella)
Since bug 1236431 landed we do not use multiple colors in the url bar anymore. However we should verify that the normal text colors are the correct ones mentioned in comment 0.
Mentor: s.kaspari
And to make things a bit more confusing: The colors are back since bug 1250671 landed (on tablets only!)
(Assignee)

Comment 5

3 years ago
Hi, Can I investigate this bug and try to fix it.
Yes, it's all yours. Let me know if you need any help.
Assignee: nobody → k.krish
Status: NEW → ASSIGNED
(Assignee)

Comment 7

3 years ago
Posted patch Bug1242624.diff (obsolete) — Splinter Review
Changed the color code based on comment 0. Please review.
Attachment #8729863 - Flags: review?(s.kaspari)
(Assignee)

Updated

3 years ago
Attachment #8729863 - Attachment is obsolete: true
Attachment #8729863 - Flags: review?(s.kaspari)
(Assignee)

Comment 8

3 years ago
Posted patch Bug1242624.diff (obsolete) — Splinter Review
Please review this patch. The color codes are changed as mentioned in Comment 0.
Attachment #8729864 - Flags: review?(s.kaspari)
The patch looks good. Can you upload a screenshot and flag antlam for feedback?
Attachment #8729864 - Flags: review?(s.kaspari) → review+
(Assignee)

Updated

3 years ago
Attachment #8729864 - Attachment is obsolete: true
(Assignee)

Comment 10

3 years ago
Updated the fix to match ContextCompat instead of Colorutils.
Attachment #8731343 - Flags: review?(s.kaspari)
(Assignee)

Comment 11

3 years ago
Posted image urlbar.png
The screenshot of both phone and tablet. Please review
Attachment #8731344 - Flags: feedback?(alam)
(Reporter)

Comment 12

3 years ago
Comment on attachment 8731344 [details]
urlbar.png

Thanks! this looks good
Attachment #8731344 - Flags: feedback?(alam) → feedback+
Comment on attachment 8731343 [details] [diff] [review]
Bug1242624.diff

Review of attachment 8731343 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. Thank you!
Attachment #8731343 - Flags: review?(s.kaspari) → review+
Keywords: checkin-needed

Comment 15

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/730802d0014e
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
You need to log in before you can comment on or make changes to this bug.