Replace Webstore link with app info dialog link in chrome://apps

chrome/browser/resources/ntp4/apps_page.js

Index: chrome/browser/resources/ntp4/apps_page.js
diff --git a/chrome/browser/resources/ntp4/apps_page.js b/chrome/browser/resources/ntp4/apps_page.js
index 5420f1944fa3bb96c3fc9889f3b2b0de52a7b518..f25332a9f8624f7a7f391af4f1816a293e62ac28 100644
--- a/chrome/browser/resources/ntp4/apps_page.js
+++ b/chrome/browser/resources/ntp4/apps_page.js
@@ -67,12 +67,21 @@ cr.define('ntp', function() {
       this.launchTypeMenuSeparator_ = cr.ui.MenuItem.createSeparator();
       menu.appendChild(this.launchTypeMenuSeparator_);
       this.options_ = this.appendMenuItem_('appoptions');
-      this.details_ = this.appendMenuItem_('appdetails');
       this.uninstall_ = this.appendMenuItem_('appuninstall');
+
+      if (loadTimeData.getBoolean('enableAppInfoDialog')) {

[sashab, 2015/02/06 16:23:00]

I'd rename this to 'canShowAppInfoDialog' ('enable' suggests its a flag).

[tsergeant, 2015/02/09 03:10:49]

Done.

+        this.appinfo_ = this.appendMenuItem_('appinfodialog');
+        this.appinfo_.addEventListener('activate',
+                                     this.onShowAppInfo_.bind(this));
+      } else {
+        this.details_ = this.appendMenuItem_('appdetails');

[sashab, 2015/02/06 16:23:00]

If they're actually replacing each other, why not just re-use the details_
field? You could then avoid bugs caused from things like 'member variables in
javascript are not null by default' (although I think they are, so you're OK
there).

+        this.details_.addEventListener('activate',
+                                     this.onShowDetails_.bind(this));
+      }
+
       this.options_.addEventListener('activate',
                                      this.onShowOptions_.bind(this));
-      this.details_.addEventListener('activate',
-                                     this.onShowDetails_.bind(this));
+
       this.uninstall_.addEventListener('activate',
                                        this.onUninstall_.bind(this));
 
@@ -144,7 +153,8 @@ cr.define('ntp', function() {
       this.launchTypeMenuSeparator_.hidden = app.appData.packagedApp;
 
       this.options_.disabled = !app.appData.optionsUrl || !app.appData.enabled;
-      this.details_.disabled = !app.appData.detailsUrl;
+      if (this.details_)
+        this.details_.disabled = !app.appData.detailsUrl;

[sashab, 2015/02/06 16:23:00]

Yeah... I don't like this. I think maybe create the link anyway but just disable
it.

So the change above would be like:
this.details = ...
if (enable dialog) {
  this.dialog = ...
  this.details.disabled = true
}

Then here you could say:
this.details.disabled = this.details.disabled || !app has a url;

[tsergeant, 2015/02/09 03:10:49]

Creating the link and then disabling it means that non-Mac platforms will
*always* have a disabled link, which seems like bad UX to me. I'm not sure that
overlapping the two in the same variable will make things any cleaner either.

[sashab, 2015/02/09 11:47:07]

Ohh I didn't realise - yes having it disabled on mac is awful. Ok, sure, I'm
happy with this.

       this.uninstall_.disabled = !app.appData.mayDisable;
 
       if (cr.isMac) {
@@ -197,6 +207,9 @@ cr.define('ntp', function() {
     onCreateShortcut_: function(e) {
       chrome.send('createAppShortcut', [this.app_.appData.id]);
     },
+    onShowAppInfo_: function(e) {
+      chrome.send('showAppInfo', [this.app_.appData.id]);
+    }
   };
 
   /**