Port xpinstallItemGeneric.png file removal from bug 572043 to Modern

RESOLVED FIXED in seamonkey2.7


7 years ago
7 years ago


(Reporter: InvisibleSmiley, Assigned: InvisibleSmiley)


Dependency tree / graph

Firefox Tracking Flags

(Not tracked)



(1 attachment)



7 years ago
Created attachment 566378 [details] [diff] [review]
patch [Checkin: comment 4]

Found while working on bug 692765: Bug 572043 removed the xpinstallItemGeneric.png file (cf. bug 572043 comment 18) and let the corresponding chrome path point to the (then) new extensionGeneric.png. Modern missed this change, but we already have the new file so all we have to do is adapt the pointer and remove the old file.

Original changesets (winstripe):

Current use:
Attachment #566378 - Flags: review?(philip.chee)

Comment 1

7 years ago
Comment on attachment 566378 [details] [diff] [review]
patch [Checkin: comment 4]

There is one reference to chrome://mozapps/skin/xpinstall/xpinstallItemGeneric.png in suite/common/bindings/notification.xml

KaiRo already noticed this in https://bugzilla.mozilla.org/show_bug.cgi?id=572043#c22

> Don't care about modern, and I actually hate how notification.xml directly
> points to an URL without giving themes to point to a possibly
> differently-named file.

I think we should avoid hard coding image paths in themes (or anywhere else).

Looking at the toolkit PopupNotifications.jsm for inspiration:

In our notification.xml::addonInstallBlocked(), we can add at the end:

var notification = this.appendNotification(messageString, notificationName,
                                           iconURL, priority, buttons);
notification.setAttribute("class", notificationName);

Then in navigator.css near /* ::::: notification popups ::::: */ we can add:

.addon-install-blocked {
  list-style-image: url("chrome://mozapps/skin/extensions/extensionGeneric.png");

or perhaps:
.addon-install-blocked > .notification-inner > hbox > .messageImage {
  list-style-image: url("chrome://mozapps/skin/extensions/extensionGeneric.png");

May need !important; to override the default icons in global/notifications.css.
Attachment #566378 - Flags: review?(philip.chee) → review-

Comment 2

7 years ago
With my patch, notification.xml would be the only consumer left in SM code and since this is a chrome URL, it can easily be translated to some other ultimate file name using jar.mn. So if this needs proper fixing another way I'll leave that to you guys. I'd have to go through several iterations to get it right, which doesn't seem too enjoyable to me.
Assignee: jh → nobody

Comment 3

7 years ago
Comment on attachment 566378 [details] [diff] [review]
patch [Checkin: comment 4]

Actually the notification.xml stuff should go in a followup bug.
Attachment #566378 - Flags: review- → review+

Comment 4

7 years ago
Comment on attachment 566378 [details] [diff] [review]
patch [Checkin: comment 4]

Attachment #566378 - Attachment description: patch → patch [Checkin: comment 4]

Comment 5

7 years ago
Phil, I think you already have a rough understanding of what that f'up should be about. Could you be so kind and create that bug? Thanks.
Assignee: nobody → jh
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.7

Comment 6

7 years ago
How about "Remove hard coded dependency on xpinstallItemGeneric.png from notification.xml"? And I don't know what's so f**ked up about this??

Comment 7

7 years ago
He, sorry, I meant f'up = follow-up. I'll try to avoid abbreviations next time. ;-)


7 years ago
Blocks: 694786
You need to log in before you can comment on or make changes to this bug.