418550 - nothing happens when clicking OK in Install Shortcut dialog on Linux

Status: VERIFIED FIXED

Description

[Myk Melez [:myk] [@mykmelez]]

Created attachment 304386 [details] [diff] [review]
patch v1: switches from XPM to PNG for Linux

When I click OK in the Install Shortcut dialog on Linux using the latest trunk version of Prism, nothing happens.  Some debugging traces the problem down to the ImageUtils module, which specifies that the native icon format is XPM.

But Mozilla has no XPM encoder, so imgITools::encodeScaledImage throws an exception when called by ImageUtils::createNativeIcon, which is what stops the accept handler from completing.

"locate /usr/share/icons" returns about ~15k lines on my Linux VM.  Of those, ~12k contain .png, while only 9 contain .xpm:

myk@myk:~$ locate /usr/share/icons | wc
  15507   15507  904367
myk@myk:~$ locate /usr/share/icons | grep .png | wc
  11866   11866  702568
myk@myk:~$ locate /usr/share/icons | grep .xpm | wc
      9       9     521

So PNG seems the dominant icon format, and I think the solution here is simply to encode the icons in that format.

Here's a patch that does that.  With this patch, OK closes the dialog, and an icon appears on my desktop.  Double-clicking the icon opens Prism to the web site for which I was installing the shortcut, so all works well.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 304386 [details] [diff] [review]
patch v1: switches from XPM to PNG for Linux

Great fix and it fits in with our plan of removing XPM altogether.

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Note: I don't have write access to the Prism repository, so I'll need this checked in for me.

Comment 3

[Fredrik Larsson (:nossralf)]

Yay!

Should probably mark bug 414594 a duplicate of this one. (I don't have the Bugzilla permissions to do so.)

Comment 4

[Matthew Gertner]

Fredrik,

I asked the admins to give you CANCONFIRM and EDITBUGS permissions. You should have received a confirmation mail.

Comment 5

[Matthew Gertner]

Oh, and thanks Myk! The Linux version has been sorely neglected. It's great to hear that it's up and running now.

Comment 6

[Fredrik Larsson (:nossralf)]

Matt, thanks! (I actually mailed Gerv myself after writing comment 3 here, heh.)

Marking this as VERIFIED.