817497 - Generic extension icons do not display anymore in Add-ons Manager

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Alice0775 White]

Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/0352a32fde64
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20121202 Firefox/20.0 ID:20121202030723

Steps to reproduce:
1. Install a Add-on which do not have own icon.
   Such as https://addons.mozilla.org/en-US/firefox/addon/dom-inspector-6622/
2. Restart browser if necessary
3. Open Add-ons Manager (Ctrl+Shift+A)
4. Switch Extensions pane

Actual results:
  Generic extension icons(a piece of green) do not display

Expected results:
  Generic extension iconsshould display

Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/5c8ee6600533
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20121130 Firefox/20.0 ID:20121130122242
Bad:
http://hg.mozilla.org/mozilla-central/rev/ecdf0e332f17
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20121201 Firefox/20.0 ID:20121201072744
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5c8ee6600533&tochange=ecdf0e332f17


Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/87c471673853
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20121130 Firefox/20.0 ID:20121130201946
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/c1425a30de0e
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20121130 Firefox/20.0 ID:20121130214842
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=87c471673853&tochange=c1425a30de0e

Suspected: bug 814195

Comment 1

[Alice0775 White]

s/(a piece of green)/(a piece of blue)/

Comment 2

[Alice0775 White]

Attachment: screenshot

Comment 3

[Lukas Blakk [:lsblakk] use ?needinfo]

Tracking and assigning to Peter to look into fixing this regression, if possible before merge to Aurora in 2.5 more weeks.

Comment 4

[Peter Van der Beken [:peterv]]

Attachment: v1

Null is being stringified, so we try to load an icon with url "null".

Comment 6

[Justin Dolske [:Dolske]]

Comment on attachment 689358 [details] [diff] [review]
v1

I'm just going to r+ this, looks simple enough.

Comment 7

[Justin Dolske [:Dolske]]

...although I'd worry a bit if the patch that broke this is web-compatible? I've always used |someimage.src = ""|, but if using null happened to work I'd wonder how many pages are doing that.

In any case, feels like the right style to use in our code.

Comment 8

[Masatoshi Kimura [:emk]]

IE and Opera stringify null. WebKit seems to treat the src attribute as nullable.

Comment 9

[Peter Van der Beken [:peterv]]

Note that this is using a XUL image element, but I'd rather make it mirror HTMLImageElement which is spec'ed to stringify null.

https://hg.mozilla.org/integration/mozilla-inbound/rev/5b01e63c0b59

Comment 10

[[e-mail]]

http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/content/extensions.xml#1830
I can't tell whether that assignment is "safe".

(In reply to Peter Van der Beken from comment #9)
> Note that this is using a XUL image element
This particular property is actually handled by an XBL binding that (in this case) simply forwards to setAttribute. Note that the C++ XUL element properties themselves don't (currently) stringify null. See also bug 821530.

> but I'd rather make it mirror HTMLImageElement which is spec'ed to stringify null.
Presumably this will take effect once its Paris bindings have been implemented?

Comment 11

[Peter Van der Beken [:peterv]]

(In reply to [e-mail] from comment #10)
> http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/
> content/extensions.xml#1830
> I can't tell whether that assignment is "safe".

Me neither, file a bug?

> This particular property is actually handled by an XBL binding that (in this
> case) simply forwards to setAttribute.

I'm aware of how XUL is implemented.

> Presumably this will take effect once its Paris bindings have been
> implemented?

Yes, see bug 825527.

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/5b01e63c0b59

Comment 13

[Virgil Dicu [:virgil] [QA]]

Mozilla/5.0 (Windows NT 6.1; rv:20.0) Gecko/20100101 Firefox/20.0

Verified in Windows 7 with Dom Inspector. Icon displayed correctly for both default and custom icons.