738241 - Filelink account setup providers menulist should contain their respective icons

Status: RESOLVED FIXED

(Thunderbird :: Preferences, defect)

Description

[Jb Piacentino]

Currently, the list only makes reference to the providers name. 
However, once a provider is selected, its icon is displayed.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Attachment: WIP Patch 1

Got it working for Windows, stashing my work here to make sure it works on OSX and Ubuntu...

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v1

Ok, I've copied the CSS for both pinstripe and gnomestripe.

I still don't see the icons on Ubuntu - but I think this is a toolkit limitation.  Firefox does the same thing in the Feed Subscribe page.  Go to http://www.mikeconley.ca/blog/feed for example, and click on the "Subscribe to this feed using"...no icons there either on Ubuntu, but they exist on OSX and Windows.  So I think we might have to punt there.

I'm just going to test this patch on OSX (which originally worked, but I'm copying the CSS over for consistency).

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 608767 [details] [diff] [review]
Patch v1

Andreas:

Ok, here's my solution.  Does it make sense?  Also, I noticed we've got some styles specific to the addAccountDialog stuffed into preferences.css that we should probably move into addAccountDialog.css.  I've filed bug 738699 about that.

-Mike

Comment 4

[Chris Coulson]

Sounds like you probably need the menuitem-with-favicon class if you want to see the icons on Ubuntu

see http://hg.mozilla.org/mozilla-central/file/d41503780635/toolkit/content/xul.css#l367

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 608767 [details] [diff] [review]
Patch v1

Chris Coulson just told me how I might get icons to display in Ubuntu - going to try that, and will update patch if it works.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v2

This patch works across all three platforms.  Woo!

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 608810 [details] [diff] [review]
Patch v2

Screenshots en-route

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Attachment: Provider icons in Windows 7

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Attachment: Provider icons in Ubuntu

Comment 10

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 608810 [details] [diff] [review]
Patch v2

>+++ b/mail/components/cloudfile/content/addAccountDialog.js
>@@ -95,17 +95,17 @@ let addAccountDialog = {
>       if (provider.iconClass) {
>-        menuitem.setAttribute("class", "menuitem-iconic");
>+        menuitem.setAttribute("class", "menuitem-iconic menuitem-with-favicon");
>         menuitem.setAttribute("image", provider.iconClass);
>       }

Huh, so we were always setting the image, but it wasn't showing up?  Cool.

In either case, it looks good, so r=me, and ui-r=me.

Thanks,
Blake.

Comment 11

[Mike Conley (:mconley) (:⚙️)]

Attachment: Provider icons in OSX

Here's the last one.

Comment 12

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 608810 [details] [diff] [review]
Patch v2

We'll want this for 13.

Comment 13

[Mike Conley (:mconley) (:⚙️)]

Committed to comm-central as http://hg.mozilla.org/comm-central/rev/c2e492105d6d

Comment 14

[Mike Conley (:mconley) (:⚙️)]

Committed to comm-aurora as http://hg.mozilla.org/releases/comm-aurora/rev/b93d8d423cc0