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

Status: VERIFIED FIXED

(Firefox Graveyard :: Web Apps, defect)

Description

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

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

Comment 1

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: 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.

Comment 2

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: 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.

Comment 3

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: 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?

Comment 4

[Benjamin Smedberg]

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.

Comment 5

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: 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

Comment 6

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Patch v2 from this bug is included in this tryserver run:
  https://tbpl.mozilla.org/?tree=Try&rev=0d5309c488f4

Comment 7

[:Felipe Gomes (needinfo for replies!)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0c61704a02ea

Comment 8

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/0c61704a02ea

Comment 10

[Jason Smith [:jsmith]]

Verified that this feature has landed. Followup bugs will be tracked in the webapps component separately.