Closed Bug 1092526 Opened 5 years ago Closed 5 years ago

about:downloads needs CSS styles on each platforms

Categories

(Thunderbird :: General, defect)

defect
Not set

Tracking

(thunderbird38+ fixed)

RESOLVED FIXED
Thunderbird 39.0
Tracking Status
thunderbird38 + fixed

People

(Reporter: hiro, Assigned: Paenglab)

References

Details

Attachments

(2 files, 5 obsolete files)

No description provided.
Depends on: 1087233
Assignee: nobody → richard.marti
Attached patch aboutDownloadsStyles.patch (obsolete) β€” β€” Splinter Review
All styles are taken from Firefox.

I've also added a icon for the tab.
Attachment #8515488 - Flags: review?(mconley)
Status: NEW → ASSIGNED
Comment on attachment 8515488 [details] [diff] [review]
aboutDownloadsStyles.patch

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

This looks fine to me - though I think that a good chunk of the .css can probably be de-duped and put into mail/themes/shared instead.

::: mail/themes/linux/mail/downloads/aboutDownloads.css
@@ +3,5 @@
> + * You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +/*** Panel and outer controls ***/
> +
> +#aboutDownloads {

Seems to me that a lot of this css could probably be put under mail/themes/shared, no?
Attachment #8515488 - Flags: review?(mconley) → review+
Attached patch aboutDownloadsStyles.patch (obsolete) β€” β€” Splinter Review
Yes, you are right. Now with shared aboutDownloads.css.
Attachment #8515488 - Attachment is obsolete: true
Attachment #8542678 - Flags: review?(mconley)
Attachment #8542678 - Flags: review?(mconley) → review?(josiah)
Comment on attachment 8542678 [details] [diff] [review]
aboutDownloadsStyles.patch

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

Seems fine to me, but the OS X icon seems wrong to me. I'm guessing it looks a lot nicer on other platforms. It's slightly blurry and perhaps a little large. Could we use the newer download icon Fx is using on OS X? Eventually we want to switch all of our icons to something similar anyway.

Other than that, LGTM.
Attachment #8542678 - Flags: review?(josiah) → review+
Attached image Comparison of Icons β€”
Notice how the downloads icon seems to stick out more than the others.
Attached patch aboutDownloadsStyles.patch (obsolete) β€” β€” Splinter Review
I've directly extracted the icon from FX's toolbar.png. Is this better?

We could also use the colored icon from http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/themes/osx/mozapps/downloads/downloadIcon.png. What do you think?
Attachment #8542678 - Attachment is obsolete: true
Attachment #8575136 - Flags: review?(josiah)
It still looks wrong to me. Could we use the download icon from the yosemite icon set?
Attached patch aboutDownloadsStyles.patch (obsolete) β€” β€” Splinter Review
Okay, now with the Yosemite icon.
Attachment #8575136 - Attachment is obsolete: true
Attachment #8575136 - Flags: review?(josiah)
Attachment #8577775 - Flags: review?(josiah)
Comment on attachment 8577775 [details] [diff] [review]
aboutDownloadsStyles.patch

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

There we go. Just please include the HiDPI version of the icon as well and we'll be good to go.
Attachment #8577775 - Flags: review?(josiah)
Hmm, I don't see how this can be done as the tab is of type="chromeTab" and I can't use a special style for this tab. And so the icon is defined through link rel="shortcut icon"... in aboutDownloads.xul.
Attached patch aboutDownloadsStyles.patch (obsolete) β€” β€” Splinter Review
Approach using svg images to be resolution independent.
Attachment #8577775 - Attachment is obsolete: true
Attachment #8577791 - Flags: review?(josiah)
Optimized the Windows svg a bit.
Attachment #8577791 - Attachment is obsolete: true
Attachment #8577791 - Flags: review?(josiah)
Attachment #8577971 - Flags: review?(josiah)
Comment on attachment 8577971 [details] [diff] [review]
aboutDownloadsStyles.patch

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

Excellent, thanks.
Attachment #8577971 - Flags: review?(josiah) → review+
Keywords: checkin-needed
Comment on attachment 8577971 [details] [diff] [review]
aboutDownloadsStyles.patch

Approval Request Comment
Consider after nightly

http://hg.mozilla.org/comm-central/rev/66ff5297b8cf
Attachment #8577971 - Flags: approval-mozilla-aurora?
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 39.0
James, you requested the uplift to Firefox aurora itself?
Are you sure about that?
Flags: needinfo?(rkent)
Comment on attachment 8577971 [details] [diff] [review]
aboutDownloadsStyles.patch

I'm sorry, I keep hitting the m-c flags instead of the c-c flags.
Flags: needinfo?(rkent)
Attachment #8577971 - Flags: approval-mozilla-aurora? → approval-comm-aurora?
Comment on attachment 8577971 [details] [diff] [review]
aboutDownloadsStyles.patch

https://hg.mozilla.org/releases/comm-aurora/rev/0d49514e9ba2
Attachment #8577971 - Flags: approval-comm-aurora? → approval-comm-aurora+
You need to log in before you can comment on or make changes to this bug.