Closed
Bug 1275880
Opened 8 years ago
Closed 8 years ago
"Settings" header is incorrectly colored
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox47 unaffected, firefox48 verified, firefox49 verified, fennec48+)
VERIFIED
FIXED
Firefox 49
Tracking | Status | |
---|---|---|
firefox47 | --- | unaffected |
firefox48 | --- | verified |
firefox49 | --- | verified |
fennec | 48+ | --- |
People
(Reporter: TeoVermesan, Assigned: mcomella)
References
Details
(Keywords: regression)
Attachments
(6 files)
159.29 KB,
image/png
|
Details | |
85.93 KB,
image/png
|
Details | |
121.06 KB,
image/png
|
Details | |
58 bytes,
text/x-review-board-request
|
liuche
:
review+
gchang
:
approval-mozilla-aurora+
|
Details |
58 bytes,
text/x-review-board-request
|
liuche
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
liuche
:
review+
|
Details |
Steps to reproduce: 1. Go to Menu -> Settings Expected results: - Grey background and white text Actual results: - Grey background and black text Note: - on Moto X (Android 4.4) the text header is not visible: http://i.imgur.com/ShorO4K.png -regression: 25-05 unaffected 26-05 affected pushlog:http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5511d54a3f172c1d68f98cc55dce4de1d0ba1b51&tochange=8d0aadfe7da782d415363880008b4ca027686137 Bug 1268606 - URL bar text selection action bar is incorrectly colored
Reporter | ||
Comment 1•8 years ago
|
||
Also action bar is incorrectly colored.
Hi Margaret, this seems to be a concerning issue on Nightly. It was mentioned at the channel meeting this morning. Could you please help find an owner who can investigate? Thanks!
Flags: needinfo?(margaret.leibovic)
Comment 3•8 years ago
|
||
mcomella, can you look into this? Or maybe ahunt or liuche can help.
Flags: needinfo?(margaret.leibovic) → needinfo?(michael.l.comella)
Updated•8 years ago
|
tracking-fennec: --- → 49+
Comment 4•8 years ago
|
||
Is this actually a regression from bug 1268606 if it doesn't affect 48? Or is that flag incorrect?
Flags: needinfo?(teodora.vermesan)
Assignee | ||
Comment 5•8 years ago
|
||
I'd guess it's a regression from bug 1268606.
Assignee: nobody → michael.l.comella
Flags: needinfo?(michael.l.comella)
Assignee | ||
Comment 6•8 years ago
|
||
Anthony, to keep things simple, after my patch, the settings screen on all devices will look like this (whereas it looked different on pre-Lollipop devices before). fwiw, it already looks like this on my GS5. Does that sound okay?
Flags: needinfo?(alam)
Comment 7•8 years ago
|
||
(In reply to Michael Comella (:mcomella) from comment #6) > Created attachment 8756929 [details] > Prefs: screenshot after patch > > Anthony, to keep things simple, after my patch, the settings screen on all > devices will look like this (whereas it looked different on pre-Lollipop > devices before). fwiw, it already looks like this on my GS5. Does that sound > okay? Talked on IRC too, I'm ok with this. Just to double check here. Did the Appbar color change?
Flags: needinfo?(alam) → needinfo?(michael.l.comella)
Assignee | ||
Comment 8•8 years ago
|
||
(In reply to Anthony Lam (:antlam) from comment #7) > Just to double check here. Did the Appbar color change? What's the app bar? That being said, my intent is to keep everything the same as it is on the latest Android devices. This includes the things broken in this bug (preferences action bar & text-selection action bar in web content body) as well as the bug I was originally solving (text-selection action bar in url bar).
Flags: needinfo?(michael.l.comella) → needinfo?(alam)
Comment 9•8 years ago
|
||
The app bar is just the dark grey bar up top (https://www.google.com/design/spec/layout/structure.html#structure-app-bar) As long as that hasn't changed color from before, then its all good. I just wanted to double check.
Flags: needinfo?(alam)
Reporter | ||
Comment 10•8 years ago
|
||
(In reply to :Margaret Leibovic from comment #4) > Is this actually a regression from bug 1268606 if it doesn't affect 48? Or > is that flag incorrect? status-firefox48: unaffected is correct. I cannot reproduce the issue on Firefox for Android 48.0a2 (2016-05-26).
Flags: needinfo?(teodora.vermesan)
Assignee | ||
Comment 11•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/55902/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/55902/
Attachment #8757465 -
Flags: review?(liuche)
Attachment #8757466 -
Flags: review?(liuche)
Attachment #8757467 -
Flags: review?(liuche)
Assignee | ||
Comment 12•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/55904/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/55904/
Assignee | ||
Comment 13•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/55906/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/55906/
Comment 14•8 years ago
|
||
Comment on attachment 8757465 [details] MozReview Request: Bug 1275880 - Consolidate gecko preferences theme to one api level. r=liuche https://reviewboard.mozilla.org/r/55902/#review52636 Asked about android: prefix IRL, summary is, AppCompat doesn't use some attributes with the android: prefix, but the one in this commit is actually removed in a later commit.
Attachment #8757465 -
Flags: review?(liuche) → review+
Comment 15•8 years ago
|
||
Comment on attachment 8757466 [details] MozReview Request: Bug 1275880 - Give preferences action bar the correct config & delete unused. r=liuche https://reviewboard.mozilla.org/r/55904/#review52638 ::: mobile/android/base/java/org/mozilla/gecko/preferences/GeckoPreferences.java (Diff revision 1) > - * > - * Declaring these attributes in XML does not work on some devices for an unknown reason > - * (e.g. the back button stops working or the logo disappears; see bug 1152314) so we > - * duplicate those attributes in code here. Note: the order of these calls matters. > - * > - * We keep the XML attributes because not all of these methods are available on pre-v14. Great! Remove special-case code :) ::: mobile/android/base/resources/values-v21/styles.xml (Diff revision 1) > <item name="android:textColor">#fff</item> > </style> > > - <style name="ActionBarThemeGeckoPreferences"> > - <!-- Icon color --> > - <item name="colorControlNormal">#fff</item> It's confusing that we used to have these around but apparently they're unused (since we don't have to add them back in).
Attachment #8757466 -
Flags: review?(liuche) → review+
Comment 16•8 years ago
|
||
Comment on attachment 8757467 [details] MozReview Request: Bug 1275880 - Correct action mode styling. r=liuche https://reviewboard.mozilla.org/r/55906/#review52652
Attachment #8757467 -
Flags: review?(liuche) → review+
Assignee | ||
Comment 17•8 years ago
|
||
https://reviewboard.mozilla.org/r/55904/#review52638 > It's confusing that we used to have these around but apparently they're unused (since we don't have to add them back in). Actually, they're defined by the parent style: ThemeOverlay.AppCompat.Dark.ActionBar inherits from Base.ThemeOverlay.AppCompat.Dark.ActionBar which [sets colorControlNormal to the primary text color](http://androidxref.com/6.0.1_r10/xref/frameworks/support/v7/appcompat/res/values/themes_base.xml#612), which we [set to @color/primary_text](https://dxr.mozilla.org/mozilla-central/rev/4d63dde701b47b8661ab7990f197b6b60e543839/mobile/android/base/resources/values/themes.xml#35), which eventually is found to be #999. I assume the title text follows the same style.
Assignee | ||
Comment 18•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/13b13d02ddad18766fa565196c11d5fe6ff3f7b3 Bug 1275880 - Consolidate gecko preferences theme to one api level. r=liuche https://hg.mozilla.org/integration/fx-team/rev/cb494ac32123f3498ef62b5e312dc4970f15cbc1 Bug 1275880 - Give preferences action bar the correct config & delete unused. r=liuche https://hg.mozilla.org/integration/fx-team/rev/7f6135916882e9dff30c5549e278f49f2fca2c17 Bug 1275880 - Correct action mode styling. r=liuche
Assignee | ||
Comment 19•8 years ago
|
||
Comment on attachment 8757465 [details] MozReview Request: Bug 1275880 - Consolidate gecko preferences theme to one api level. r=liuche Approval Request Comment [Feature/regressing bug #]: bug 1268606 (which was getting uplifted to fix older issues) [User impact if declined]: Users will have inconsistent action bar coloring [Describe test coverage new/current, TreeHerder]: Tested locally on my 5+ N9 & 4.4 GS4. [Risks and why]: Low. For the most part we're just changing style code so in the worst case, we get the style wrong again (but it's unlikely to be as wrong as it is now). [String/UUID change made/needed]: None
Attachment #8757465 -
Flags: approval-mozilla-aurora?
Comment 20•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/13b13d02ddad https://hg.mozilla.org/mozilla-central/rev/cb494ac32123 https://hg.mozilla.org/mozilla-central/rev/7f6135916882
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Reporter | ||
Comment 22•8 years ago
|
||
"Settings" header is correctly colored, so: Verified as fixed using: Devices: Moto X (Android 4.4), Nexus 7 (Android 5.1.1), One A2001 (Android 5.1.1), Nexus 5X (Android 6.0) Build: Firefox for Android 49.0a1 (2016-06-01)
Status: RESOLVED → VERIFIED
Comment 24•8 years ago
|
||
48 is now affected too.
Updated•8 years ago
|
tracking-fennec: 49+ → 48+
Comment 25•8 years ago
|
||
Comment on attachment 8757465 [details] MozReview Request: Bug 1275880 - Consolidate gecko preferences theme to one api level. r=liuche This patch fixes an inconsistent action bar coloring regression. Take it in aurora.
Attachment #8757465 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 26•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/ebc5b4222c2b https://hg.mozilla.org/releases/mozilla-aurora/rev/1f4a8e832810 https://hg.mozilla.org/releases/mozilla-aurora/rev/1d0f99529b31
Reporter | ||
Comment 27•8 years ago
|
||
"Settings" header is correctly colored, so: Verified as fixed using: Devices: Moto X (Android 4.4), One A2001 (Android 5.1.1) Build: Firefox for Android 48.0a2 (2016-06-06)
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•