Closed Bug 1200299 Opened 4 years ago Closed 4 years ago

Change background color of "header" block on edit logins page

Categories

(Firefox for Android :: Logins, Passwords and Form Fill, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 43
Tracking Status
firefox42 --- fixed
firefox43 --- fixed

People

(Reporter: ally, Assigned: ally)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

https://bugzilla.mozilla.org/show_bug.cgi?id=1185777#c13

Make header background color always #F5F5F5
Assignee: nobody → ally
side note: there's also bug 1198935 that just contains the spec for this part of the UI if you want to roll it in together with those.
So the underlying cause of this is that .header class does not specify a background color. So you get whatever the background color of the layout is. The list view is likewise the color of the background.

1 bug, 1 problem antlam. :)
Flags: needinfo?(alam)
Attached patch giveHeadersBackgroundColor (obsolete) — Splinter Review
:antlam would like to see this uplifted. Since the patch contains nothing risky, I am open to the idea.
Attachment #8655116 - Flags: review?(mark.finkle)
Thanks for this Ally! This would be great to nail down as it's the only part of our UI that has a white header background currently :)
Flags: needinfo?(alam)
Comment on attachment 8655116 [details] [diff] [review]
giveHeadersBackgroundColor

I wondered why the #header class in aboutBase.css didn't already handle this. Turns out we handle the F5F5F5 background color in the | html | class in aboutBase.css via a filter substitution. aboutLogins.css has it's own | html | class definition. Does this override aboutBase.css?

Can you take a longer look into why the aboutBase.css code is not working?
Attachment #8655116 - Flags: review?(mark.finkle) → review-
header in aboutBase.css does not actually explicitly set a background color.  The edit-logins-page has its own background color set, which overrides the default one as set in the html rule in aboutBase.css. edit-logins-page whose background is set to white, is higher priority than the html background color rule. The header is a child about edit-logins-page, so its background color is that of its parent unless set explicitly by a rule (like the .about-header one in the above patch).

The list view pge of aboutLogins is correct by accident. It has no explicit background color, so its header inherits the html{}'s background color rule. I had explicitly set it in my previous patch to ensure no surprises in the future, though its not actually needed for correctness. 

So, since it is clear that we expect that headers have a specific background color, we(okay, probably I) will be modifying aboutBase.css to state that clearly. Bug 1201340

For _this_ bug, we'll use a smidge of css to override the white background on the edit-logins-page header.
Attachment #8655116 - Attachment is obsolete: true
Attachment #8656327 - Flags: review?(mark.finkle)
Attachment #8656327 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/integration/fx-team/rev/2b373308181710e3b8667a0215a612335b73cc92
Bug 1200299 - Change background color of header block on edit logins page.r=mfinkle
Comment on attachment 8656327 [details] [diff] [review]
changeHeaderColor

Approval Request Comment
[Feature/regressing bug #]: about:logins
[User impact if declined]: a header is wrong color, inconsistent ui
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: none
[String/UUID change made/needed]: none
Attachment #8656327 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/2b3733081817
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
Attachment #8656327 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.