Last Comment Bug 745995 - Natively-installed webapps should use the app's icon for their taskbar and system menu icons
: Natively-installed webapps should use the app's icon for their taskbar and sy...
Status: VERIFIED FIXED
:
Product: Firefox Graveyard
Classification: Graveyard
Component: Web Apps (show other bugs)
: Trunk
: x86 Windows 7
: -- normal
: Firefox 14
Assigned To: Tim Abraldes [:TimAbraldes] [:tabraldes]
: Jason Smith [:jsmith]
Mentors:
: 732301 (view as bug list)
Depends on: 725408
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-16 15:58 PDT by Tim Abraldes [:TimAbraldes] [:tabraldes]
Modified: 2016-02-04 15:00 PST (History)
5 users (show)
jsmith: in‑moztrap+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
WIP Patch 1 - Modify nsXREDirProvider to return profileDir when NS_APP_CHROME_DIR_LIST is requested (799 bytes, patch)
2012-04-16 16:23 PDT, Tim Abraldes [:TimAbraldes] [:tabraldes]
no flags Details | Diff | Review
Patch v1 - Modify WebappRT dir provider to provide NS_APP_CHROME_DIR_LIST (2.01 KB, patch)
2012-04-18 00:54 PDT, Tim Abraldes [:TimAbraldes] [:tabraldes]
no flags Details | Diff | Review
Patch v2 - Append chrome dir (2.01 KB, patch)
2012-04-18 01:24 PDT, Tim Abraldes [:TimAbraldes] [:tabraldes]
benjamin: review+
Details | Diff | Review
Patch v3 - Review comments addressed. Carrying forward r+ (2.52 KB, patch)
2012-04-18 12:28 PDT, Tim Abraldes [:TimAbraldes] [:tabraldes]
timabraldes: review+
gavin.sharp: approval‑mozilla‑central+
Details | Diff | Review

Description Tim Abraldes [:TimAbraldes] [:tabraldes] 2012-04-16 15:58:41 PDT
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] 2012-04-16 16:23:15 PDT
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.
Comment 2 Tim Abraldes [:TimAbraldes] [:tabraldes] 2012-04-18 00:54:15 PDT
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.
Comment 3 Tim Abraldes [:TimAbraldes] [:tabraldes] 2012-04-18 01:24:42 PDT
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?
Comment 4 Benjamin Smedberg [:bsmedberg] 2012-04-18 11:55:22 PDT
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] 2012-04-18 12:28:22 PDT
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
Comment 6 Tim Abraldes [:TimAbraldes] [:tabraldes] 2012-04-18 13:42:35 PDT
Patch v2 from this bug is included in this tryserver run:
  https://tbpl.mozilla.org/?tree=Try&rev=0d5309c488f4
Comment 7 :Felipe Gomes (needinfo me!) 2012-04-18 15:20:57 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/0c61704a02ea
Comment 8 :Ehsan Akhgari (out sick) 2012-04-19 07:24:55 PDT
https://hg.mozilla.org/mozilla-central/rev/0c61704a02ea
Comment 9 :Felipe Gomes (needinfo me!) 2012-04-19 22:45:46 PDT
*** Bug 732301 has been marked as a duplicate of this bug. ***
Comment 10 Jason Smith [:jsmith] 2012-05-04 16:24:10 PDT
Verified that this feature has landed. Followup bugs will be tracked in the webapps component separately.

Note You need to log in before you can comment on or make changes to this bug.