543490 - Icons specified by an extension don't replace icons specified by an application

Status: RESOLVED WONTFIX

(Core :: Widget, defect)

Description

[Mike Kaply [:mkaply]]

An extension can replace icons by putting an icon in chrome/icons/default where the icon name is the same as the window ID.

For instance, to replace the main window in Firefox, you put a file called main-window.ico

However this doesn't work if the application has an icon in the chrome/icons/default directory in its own program directory.

Extensions should be able to specify icons that override the applications icons.

This is most evident in seamonkey since it ships an icon for main-window.

If an extension specifies a main-window.ico, it doesn't override the seamonkey icon.

Comment 2

[Mike Kaply [:mkaply]]

This appears to be another case where things are not read from the packed XPI.

If you unpack your XPI, it works.

Comment 3

[Jiri TRAVNICEK [:JITR]]

Hmm, according to my experiments last Friday, this has nothing to do with the XPI being unpacked or not. The XPI being unpacked still seems to be a necessity, yes, but it does not seem sufficient.

My experiment confirmed exactly what has been said in the description -- if the application already provides an icon, an extension cannot override it. During my experiment, an extension-provided icon did not take effect until I disabled (renamed) a corresponding icon file within the application's directory. The XPI definitely had to be extracted for this to work at all. I have been testing this with the latest stable SeaMonkey (ver. 2.26.1) as well as Firefox (ver. 30) on Windows 7.

However, from your last comment it seems you have acquired a different results later. Could I be doing something wrong?

Anyway, the document at https://developer.mozilla.org/en-US/docs/Window_icons might need an update or even a fix. At least the unpacking requirement should be mentioned. (This requirement is also noted within the DOM Inspector's sources and can even be easily confirmed as the DOM Inspector is being pre-installed without being unpacked in SeaMonkey. ;-) ) Do you want me to open a separate bug for this?

Comment 4

[Jiri TRAVNICEK [:JITR]]

After a cursory MXR search, short sources study and futher experiment with Process Monitor I think I have determined the cause. This seems to be implemented in `nsBaseWidget::ResolveIconName()':

    http://mxr.mozilla.org/mozilla-central/source/widget/xpwidgets/nsBaseWidget.cpp#1298

I believe the problems is caused by the application's chrome directory being returned among the other auxiliary chrome directories (possibly as the first entry) and thus being searched *before* the other auxiliary directories instead of *after* them, which prevents application-provided icons override. From the Process Monitor output it is obvious the `browser/chrome/icons/default' in the application directory is being checked first as soon as the path prefix `browser/chrome/' exists.

Tested in the same environment as in the previous comment. Both Firefox and SeaMonkey exhibit the same behavior (except that the `browser/chrome/' path prefix shortens to `chrome/' in SeaMonkey).

Comment 5

[Mike Kaply [:mkaply]]

Looking at that function, though, it checks other directories before checking the main directory...

Comment 6

[Jiri TRAVNICEK [:JITR]]

You mean looking at the overall `nsBaseWidget::ResolveIconName()' method or you've been analyzing the first part including the list returned by `dirSvc->Get(NS_APP_CHROME_DIR_LIST, ...)'?

Unfortunately I am not able to analyze the code deep enough to be sure, but I believe the call to `dirSvc->Get(NS_APP_CHROME_DIR_LIST, ...)' is being handled by `nsXREDirProvider::GetFilesInternal()' here:

    http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsXREDirProvider.cpp#704

If that is correct, than the call

    LoadDirIntoArray(mXULAppDir, kAppendChromeDir, directories);

within `GetFilesInternal()' could be what makes the application's chrome directory the first entry in the returned (auxiliary chrome directories) list.

Mind you, I am having problems properly verifying that the `nsXREDirProvider' is really registered as a directory provider with the `nsIDirectoryService' instance in `dirSvc' at this point. This is based solely on the result of identifier search for `NS_APP_CHROME_DIR_LIST', which only returns 2 results and the other one (in webapprt) seems to be rather unlikely.

Comment 7

[Jiri TRAVNICEK [:JITR]]

(In reply to Jiri TRAVNICEK [:JITR] from comment #3)

> {...}
> 
> 
> Anyway, the document at
> https://developer.mozilla.org/en-US/docs/Window_icons might need an update
> or even a fix. {...}

Done, added a reference to this bug:

    https://developer.mozilla.org/en-US/docs/Window_icons$compare?to=630391&from=619409

I also think this bug could already be marked as `CONFIRMED', but don't have the privileges to change this myself.

Comment 8

[Jiri TRAVNICEK [:JITR]]

Reproduced with the official Linux build of SeaMonkey ver. 2.26.1 as well.

There were a minor differences, though:

    (1) Changes in icon files only took effect after restarting SeaMonkey (in Windows it was enough to just (re)open a window).

    (2) Only PNG icons seemed to be used, XPM files seemed to be completely ignored.

None of these should IMHO invalidate the fact that the problem is in the cross-platform code, thus changing component.

- -

(In reply to myself from comment #7)

> I also think this bug could already be marked as `CONFIRMED',

Already done, it seems, sorry for this nonsense. :-( (I probably got confused by the `UNCONFIRMED' state to which the "opposite" is `NEW' and not `CONFIRMED'.)

> but don't have the privileges to change this myself.

Got that fixed, too. :-)

Comment 9

[Philippe VIGNEAU]

affected too by this problem...

Comment 10

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.

Comment 11

[Mike Kaply [:mkaply]]

Legacy Extension related.