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

VERIFIED FIXED in Firefox 14

Status

Firefox Graveyard
Web Apps
VERIFIED FIXED
5 years ago
a year ago

People

(Reporter: TimAbraldes, Assigned: TimAbraldes)

Tracking

Trunk
Firefox 14
x86
Windows 7
Bug Flags:
in-moztrap +

Details

Attachments

(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 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,
  QueryInterface:...,
  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:
  https://tbpl.mozilla.org/?tree=Try&rev=0d5309c488f4
Attachment #616244 - Flags: approval-mozilla-central? → approval-mozilla-central+
https://hg.mozilla.org/integration/mozilla-inbound/rev/0c61704a02ea
Target Milestone: --- → Firefox 14
https://hg.mozilla.org/mozilla-central/rev/0c61704a02ea
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Duplicate of this bug: 732301

Updated

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

Updated

5 years ago
No longer blocks: 731054

Updated

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

Updated

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

Updated

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.