Closed Bug 678761 Opened 8 years ago Closed 8 years ago

Add-on selection UI footer won't look good on non-aero themes

Categories

(Toolkit :: Add-ons Manager, defect)

All
Windows XP
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla9
Tracking Status
firefox8 --- fixed

People

(Reporter: mossop, Assigned: Unfocused)

References

Details

(Whiteboard: [qa-])

Attachments

(4 files, 1 obsolete file)

From bug 596343 comment 173:

> >+++ b/toolkit/themes/winstripe/mozapps/extensions/selectAddons.css
> >+#footer {
> >+  padding: 15px 12px;
> >+  background-color: #f1f5fb;
> >+  box-shadow: 0px 1px 2px rgb(204,214,234) inset;
> >+}
> >+
> >+.progress-label,
> >+#footer-label {
> >+  font-style: italic;
> >+  color: GrayText;
> >+}
> 
> This is entirely broken. The footer styling won't fit non-aero themes such
> as any theme on Windows XP and the label is guaranteed to be invisible with 
> dark (e.g. high-contrast) themes, where GrayText isn't gray. Please file a
> bug on fixing this?
Assignee: nobody → bmcbride
Attached patch Patch v1 (obsolete) — Splinter Review
Turned out to be not a huge issue. Footer looked a little out of place, but not terribly so. And things were still actually readable on high-contrast themes, although I bet some dark themes made it impossible to read. Still, some tweaks seem to help make it look better.
Attachment #553349 - Flags: review?(dtownsend)
Attachment #553349 - Flags: review?(dtownsend) → review?(dao)
Status: NEW → ASSIGNED
(In reply to Blair McBride (:Unfocused) from comment #1)
> And things were still actually readable on high-contrast themes,

For you or for those people who would select a high-contrast theme? ;-)
Comment on attachment 553349 [details] [diff] [review]
Patch v1

> #footer {
>   padding: 15px 12px;
>-  background-color: #f1f5fb;
>-  box-shadow: 0px 1px 2px rgb(204,214,234) inset;
> }

What background color will this fall back to? White (e.g. window) or gray (e.g. -moz-dialog)?

> .progress-label,
> #footer-label {
>   font-style: italic;
>-  color: GrayText;
> }

Why are you removing this? GrayText is okay, as long as you're not mixing it with a hardcoded background.

>+@media all and (-moz-windows-compositor) {
>+  #footer {
>+    background-color: #f1f5fb;
>+    box-shadow: 0px 1px 2px rgb(204,214,234) inset;
>+  }
>+
>+  .progress-label,
>+  #footer-label {
>+    color: #6D6D6D;
>+  }
>+}

Are you sure you want -moz-windows-compositor? What's the expected look for aero basic?
Also, could you add selectAddons-aero.css?
Attachment #553349 - Flags: review?(dao)
(In reply to Dão Gottwald [:dao] from comment #3)
> What background color will this fall back to? White (e.g. window) or gray
> (e.g. -moz-dialog)?

-moz-dialog (ie, gray), so it still looks like a footer. The background grey and GrayText are different enough for the text to be readable, although I'm on the fence over whether the text should just be black to help with legibility (which was why I originally removed GrayText in the first patch).

> Are you sure you want -moz-windows-compositor? What's the expected look for
> aero basic?

I had a closer look at some of the windows/dialogs in Window 7 under Aero Basic, and they use the blue footer style. So made Aero Basic here get the styled footer too.
Attachment #553349 - Attachment is obsolete: true
Attachment #555017 - Flags: review?(dao)
Attached image Windows XP
Comment on attachment 555017 [details] [diff] [review]
[checked-in] Patch v2

> .progress-label,
> #footer-label {
>   font-style: italic;
>   color: GrayText;
> }

I think you should add %ifdef WINSTRIPE_AERO around font-style:italic, as this seems non-native on XP.
Attachment #555017 - Flags: review?(dao) → review+
Done.

http://hg.mozilla.org/integration/fx-team/rev/cd8f12dc6dce
Flags: in-testsuite-
Flags: in-litmus-
Whiteboard: [fixed-in-fx-team]
Comment on attachment 555017 [details] [diff] [review]
[checked-in] Patch v2

http://hg.mozilla.org/mozilla-central/rev/cd8f12dc6dce
Attachment #555017 - Attachment description: Patch v2 → [checked-in] Patch v2
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
Whiteboard: [fixed-in-fx-team]
Attachment #555017 - Flags: approval-mozilla-aurora?
Attachment #555017 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [qa-]
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.