Natively-installed webapps should use the app's icon for their taskbar and system menu icons

VERIFIED FIXED in Firefox 14


Firefox Graveyard
Web Apps
5 years ago
2 years ago


(Reporter: TimAbraldes, Assigned: TimAbraldes)


Firefox 14
Windows 7
Bug Flags:
in-moztrap +



(1 attachment, 3 obsolete attachments)

XULRunner apps have the icons for their windows in "${AppDir}\chrome\icons\default".  We will want to use a similar approach for natively-installed webapps, placing the icon for the app in a subdirectory of the app's install directory and loading it at runtime.  As mentioned in bug 725408, two possible approaches are:
  1) Modify nsXREDirProvider to return the natively-installed app's application directory when NS_APP_CHROME_DIR_LIST is requested
  2) Modify nsWindow::SetIcon to use the embedded icon of the running EXE if it does not find a suitable .ico file
Created attachment 615529 [details] [diff] [review]
WIP Patch 1 - Modify nsXREDirProvider to return profileDir when NS_APP_CHROME_DIR_LIST is requested

This patch modifies nsXREDirProvider to return the User Data directory when NS_APP_CHROME_DIR_LIST is requested (and only when this code is being run in the webapprt stub).  This works because the User Data directory and the natively-installed app directory happen to be the same directory.  However, it would be nice to explicitly specify the app's directory and not use the User Data directory here.
Created attachment 616041 [details] [diff] [review]
Patch v1 - Modify WebappRT dir provider to provide NS_APP_CHROME_DIR_LIST

This patch modifies the WebappRT directory service provider to return the natively-installed-webapp's application directory when asked for NS_APP_CHROME_DIR_LIST.  As long as we place our icons at "${AppDir}/icons/default/${windowNameOrDefault}.ico" this patch will make icons work correctly in natively-installed webapps.
Attachment #615529 - Attachment is obsolete: true
Created attachment 616048 [details] [diff] [review]
Patch v2 - Append chrome dir

Slight change; "${AppDir}\chrome\icons\default" instead of "${AppDir}\icons\default"

This makes natively-installed webapps more like XUL apps.

Benjamin: I hate to pile another review on your plate, but since you reviewed bug 725408 I think you're the right person for this review?
Attachment #616041 - Attachment is obsolete: true
Attachment #616048 - Flags: review?(benjamin)

Comment 4

5 years ago
Comment on attachment 616048 [details] [diff] [review]
Patch v2 - Append chrome dir

Nit, unsnuggle "if("

Your enumerator is rather complicated for a single element, it could be:

return {
  _done: false,
  hasMoreElements: function() {
    return !this._done;
  getNext: function () {
    return FileUtils.getDir("AppRegD", ["chrome"], false);
    this._done = true;

Up to you if you want to make that change.
Attachment #616048 - Flags: review?(benjamin) → review+
Created attachment 616244 [details] [diff] [review]
Patch v3 - Review comments addressed. Carrying forward r+

[Approval Request Comment]

This is part of the webapps work which is targeted for Firefox 14.  Without this patch, the Windows taskbar will show the Windows default icon for any running webapps.  With this patch, the Windows taskbar will show the correct icon for each running webapp.

Risk for Fennec: none, the code here affects only webapprt/WebappRTDirectoryProvider.js
Attachment #616048 - Attachment is obsolete: true
Attachment #616244 - Flags: review+
Attachment #616244 - Flags: approval-mozilla-central?
Patch v2 from this bug is included in this tryserver run:
Attachment #616244 - Flags: approval-mozilla-central? → approval-mozilla-central+
Target Milestone: --- → Firefox 14
Last Resolved: 5 years ago
Resolution: --- → FIXED
Duplicate of this bug: 732301


5 years ago
Blocks: 731054
Verified that this feature has landed. Followup bugs will be tracked in the webapps component separately.


5 years ago
No longer blocks: 731054


5 years ago
Flags: in-moztrap?(jsmith)


5 years ago
Flags: in-moztrap?(jsmith) → in-moztrap+


5 years ago
QA Contact: jsmith
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.