Add icons to Windows jumplist items

RESOLVED FIXED in Thunderbird 8.0

Status

defect
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: rain1, Assigned: rain1)

Tracking

unspecified
Thunderbird 8.0
x86
Windows 7

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Assignee

Description

8 years ago
Posted patch patch v1 (obsolete) — Splinter Review
Currently our Windows jumplist items use the default Thunderbird icon. We can do better. My proposal is to extract the icon from the toolbar sprite sheet and embed it into the EXE at build time.

The attached patch does precisely that. It does depend on pypng [1] though, and we're going to need to get it on at least the Windows builders to get the patch to work. We seem to have at least three options to do so:
1. Get Mozilla-Build to include it. It's pure python, so not a hassle at all to include.
2. Add it to the comm-central repo. It's MIT-licensed and build-only, so I don't really see an issue there.
3. Get people on Windows to easy_install it. easy_install pypng.

Mark, Gozer, opinions?

We could also use a more heavyweight library like the Python Imaging Library [2], but I don't really see the point in doing so.

[1] https://code.google.com/p/pypng/
[2] http://www.pythonware.com/products/pil/
Assignee

Updated

8 years ago
Attachment #535214 - Attachment is patch: true
Attachment #535214 - Flags: ui-review?(bwinton)
Attachment #535214 - Flags: review?(mbanner)
Assignee

Comment 1

8 years ago
Posted image Screenshot
Assignee: nobody → sid.bugzilla
Status: NEW → ASSIGNED
Assignee

Updated

8 years ago
Attachment #535228 - Attachment is patch: false
Attachment #535228 - Attachment mime type: text/plain → image/png
Comment on attachment 535214 [details] [diff] [review]
patch v1

Slick looking!  I haven't tested the UI, but assuming it works the way it looks like it works, ui-r=me!

Thanks,
Blake.
Attachment #535214 - Flags: ui-review?(bwinton) → ui-review+

Comment 4

8 years ago
I filed a similar bug 661502 for Firefox. I think they might be interested to have this functionality too.
Ted, see comment 0, would lib pypng be something you would be willing to include in mozilla-build?
Sure, but we'd have to get out a new MozillaBuild release, which I haven't had time to do.
Assignee

Comment 7

8 years ago
Now that we're about to include MozMill in the tree, I think it'd make sense to include this in the tree too, and maybe even combine the virtualenvs for the two.
(In reply to comment #7)
> Now that we're about to include MozMill in the tree, I think it'd make sense
> to include this in the tree too, and maybe even combine the virtualenvs for
> the two.

Assuming the code isn't massive, I think that could be reasonable to do.
Comment on attachment 535214 [details] [diff] [review]
patch v1

Just on file placement, I'd like to see the python script in c-c/build/ instead, so that SeaMonkey could in theory use this same approach.
Attachment #535214 - Flags: feedback-
Assignee

Comment 10

8 years ago
Posted patch patch v2Splinter Review
All right, this moves pypng into the tree. It doesn't use a virtualenv though, just straightforward pythonpath.py.

The library's MIT-licensed and used only at build time, so adding it shouldn't be a problem I think. Do we need to update about:license for this?
Attachment #535214 - Attachment is obsolete: true
Attachment #535214 - Flags: review?(mbanner)
Attachment #550226 - Flags: review?(mbanner)
Comment on attachment 550226 [details] [diff] [review]
patch v2

>diff --git a/mailnews/base/src/nsMessengerWinIntegration.cpp b/mailnews/base/src/nsMessengerWinIntegration.cpp

>-#define IDI_MAILBIFF 32767
>+#define IDI_MAILBIFF 32576

If you are changing this here, you have to change it in the suite manifest as well.
Attachment #550226 - Flags: feedback-
Assignee

Comment 12

8 years ago
Good point, thanks.
Comment on attachment 550226 [details] [diff] [review]
patch v2

This seems fine to me.

I think bringing the code in should be fine, but please check with licensing about the incorporation - more details here: http://www.mozilla.org/MPL/license-policy.html
Comment on attachment 550226 [details] [diff] [review]
patch v2

(r+ assuming licensing is happy).
Attachment #550226 - Flags: review?(mbanner) → review+
Yes, this is fine.

Gerv
Assignee

Comment 16

8 years ago
Thanks Gerv!

https://hg.mozilla.org/comm-central/rev/923b924c9422
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 8.0
You need to log in before you can comment on or make changes to this bug.