Stop rebuilding all elements in chrome://extensions to preserve focus on refresh

chrome/browser/resources/extensions/create_node.js

Index: chrome/browser/resources/extensions/create_node.js
diff --git a/chrome/browser/resources/extensions/create_node.js b/chrome/browser/resources/extensions/create_node.js
new file mode 100644
index 0000000000000000000000000000000000000000..c46adf114bd894ee94a47a33ffc405ae4dba8fb6
--- /dev/null
+++ b/chrome/browser/resources/extensions/create_node.js
@@ -0,0 +1,149 @@
+// TODO(hcarmona): Remove this temporary file.
+
+    /**
+     * Synthesizes and initializes an HTML element for the extension metadata
+     * given in |extension|.
+     * @param {ExtensionData} extension A dictionary of extension metadata.
+     * @private
+     */
+    createNode_: function(extension) {
+      var template = $('template-collection').querySelector(
+          '.extension-list-item-wrapper');
+      var node = template.cloneNode(true);
+      node.id = extension.id;
+
+      // The 'Show Browser Action' button.
+      var showButton = node.querySelector('.show-button');
+      showButton.addEventListener('click', function(e) {
+        chrome.send('extensionSettingsShowButton', [extension.id]);
+      });
+
+      // The 'allow in incognito' checkbox.
+      var incognito = node.querySelector('.incognito-control input');
+      var butterBar = node.querySelector('.butter-bar');
+      incognito.addEventListener('change', function(e) {
+        var checked = e.target.checked;
+        butterBarVisibility[extension.id] = checked;
+        butterBar.hidden = !checked || extension.is_hosted_app;

[not at google - send to devlin, 2015/02/05 22:25:03]

This butterbar visibility logic should be in update_node.js, because there's a
bug where:
1. Open up 2 chrome://extensions tabs
2. Click "allow in incognito" in one of them (butterbar shows)
3. Go to the other, deselect "allow in incognito"
4. Go back to the original; the butterbar is still there, even though it's not
enabled in incognito anymore.

The existing code has this bug too. And, fixing this by moving it into
update_node.js will introduce a different bug: apparently the desired behavior
is only to show the butterbar the first time you click it, and never after.

I'm dubious of whether this is actually a good feature, but let's say it is - a
good way to fix this would be to set some flag here like "didEnableIncognito_"
to true here, then check that in update_node.js and only show the butterbar if
that was true.

[hcarmona, 2015/02/05 23:27:14]

There's already code in update_node.js to update the butterbar visibility. We
can change it to also make sure that Allow in incognito is set for the extension
and use the flag to only change this value once. But this will still show the
butterbar if you open the page again and click the incognito checkbox. If it
needs to be more permanent, then that should be a different CL.

[not at google - send to devlin, 2015/02/05 23:34:06]

I tried this with your patch, and nothing seems to have changed. Another way to
phrase my comment is: this display logic belongs in update_node.js, not here,
however you'll need some special code to maintain the behavior where it only
shows once.

[hcarmona, 2015/02/06 02:25:10]

This new patch has this working so the butterbar shows once and goes away when
updated.

+        chrome.send('extensionSettingsEnableIncognito',
+                    [extension.id, String(checked)]);
+      });
+
+      // The 'collect errors' checkbox. This should only be visible if the
+      // error console is enabled - we can detect this by the existence of the
+      // |errorCollectionEnabled| property.
+      var errorCollection =
+          node.querySelector('.error-collection-control input');
+      errorCollection.addEventListener('change', function(e) {
+        chrome.send('extensionSettingsEnableErrorCollection',
+                    [extension.id, String(e.target.checked)]);
+      });
+
+      // The 'allow on all urls' checkbox. This should only be visible if
+      // active script restrictions are enabled. If they are not enabled, no
+      // extensions should want all urls.
+      var allUrls = node.querySelector('.all-urls-control');
+      allUrls.addEventListener('click', function(e) {
+        chrome.send('extensionSettingsAllowOnAllUrls',
+                    [extension.id, String(e.target.checked)]);
+      });
+
+      // The 'allow file:// access' checkbox.
+      var fileAccess = node.querySelector('.file-access-control');
+      fileAccess.addEventListener('click', function(e) {
+        chrome.send('extensionSettingsAllowFileAccess',
+                    [extension.id, String(e.target.checked)]);
+      });
+
+      // The 'Options' button or link, depending on its behaviour.
+      var optionsLink = node.querySelector('.options-link');
+      // Set an href to get the correct mouse-over appearance (link,
+      // footer) - but the actual link opening is done through chrome.send
+      // with a preventDefault().
+      optionsLink.setAttribute('href', extension.optionsPageHref);
+      optionsLink.addEventListener('click', function(e) {
+        chrome.send('extensionSettingsOptions', [extension.id]);
+        e.preventDefault();
+      });
+
+      var optionsButton = options = node.querySelector('.options-button');
+      optionsButton.addEventListener('click', function(e) {
+        this.showEmbeddedExtensionOptions_(extension.id, false);
+        e.preventDefault();
+      }.bind(this));
+
+      // The 'Permissions' link.
+      var permissions = node.querySelector('.permissions-link');
+      permissions.addEventListener('click', function(e) {
+        chrome.send('extensionSettingsPermissions', [extension.id]);
+        e.preventDefault();
+      });
+
+      // The 'Reload' link.
+      var reload = node.querySelector('.reload-link');
+      reload.addEventListener('click', function(e) {
+        chrome.send('extensionSettingsReload', [extension.id]);
+        extensionReloadedTimestamp[extension.id] = Date.now();
+      });
+
+      // The 'Launch' link.
+      var launch = node.querySelector('.launch-link');
+      launch.addEventListener('click', function(e) {
+        chrome.send('extensionSettingsLaunch', [extension.id]);
+      });
+
+      // The 'Reload' terminated link.
+      var terminatedReload = node.querySelector('.terminated-reload-link');
+      terminatedReload.addEventListener('click', function(e) {
+        chrome.send('extensionSettingsReload', [extension.id]);
+      });
+
+      // The 'Repair' corrupted link.
+      var repair = node.querySelector('.corrupted-repair-button');
+      repair.addEventListener('click', function(e) {
+        chrome.send('extensionSettingsRepair', [extension.id]);
+      });
+
+      // The 'Enabled' checkbox.
+      var enable = node.querySelector('.enable-checkbox');
+      enable.addEventListener('click', function(e) {
+        // When e.target is the label instead of the checkbox, it doesn't
+        // have the checked property and the state of the checkbox is
+        // left unchanged.

[not at google - send to devlin, 2015/02/05 22:25:03]

This (existing) code is weird, I wonder why it doesn't use the |enable| node to
determine check value, but instead tries to be smart and use e.target? I.e.
seems like you could remove lines 110-116 and change it to
chrome.send('extensionSettingsEnable', String(enable.checked));

[hcarmona, 2015/02/05 23:27:14]

enable is a div. if the click event happens on the label it sends
!checkbox.checked because the checkbox won't have toggled and if the target is
the checkbox then the value of checkbox.checked is used because it's already
been toggled.

[not at google - send to devlin, 2015/02/05 23:34:06]

You've lost me, but I think that if you change the selector to '.enable-checkbox
input' then what I said is correct.

[not at google - send to devlin, 2015/02/05 23:39:35]

What is said is wrong. You should be checking
enabled.querySelector('input').checked, as is what is basically already being
done, but it's too complicated bringing e.target into it.

(and yep - s/you/previous author/)

[hcarmona, 2015/02/06 02:25:10]

It's not possible to rely on the value of the checkbox because it should also be
possible to click on the label to make the checkbox toggle. Clicking on the
label doesn't toggle the value of the checkbox.

[not at google - send to devlin, 2015/02/06 15:48:00]

I just tried this and clicking on the label toggles the value of the checkbox.

[hcarmona, 2015/02/06 23:41:27]

I'm getting a bit confused now. Current functionality allows clicking either the
label or checkbox.

When I changed this locally, it doesn't toggle on the label.
This is my local change, that only works on the checkbox:

  var checked = node.querySelector('.enable-checkbox input').checked;
  chrome.send('extensionSettingsEnable', [extension.id, String(checked)]);

perhaps I'm not understanding the change you're suggesting

+        var checked = e.target.checked;
+        if (checked == undefined)
+          checked = !e.currentTarget.querySelector('input').checked;
+        chrome.send('extensionSettingsEnable',
+                    [extension.id, checked ? 'true' : 'false']);
+
+        // This may seem counter-intuitive (to not set/clear the checkmark)
+        // but this page will be updated asynchronously if the extension
+        // becomes enabled/disabled. It also might not become enabled or
+        // disabled, because the user might e.g. get prompted when enabling
+        // and choose not to.
+        e.preventDefault();
+      });
+
+      // 'Remove' button.
+      var trashTemplate = $('template-collection').querySelector('.trash');
+      var trash = trashTemplate.cloneNode(true);
+      trash.title = loadTimeData.getString('extensionUninstall');
+      trash.addEventListener('click', function(e) {
+        butterBarVisibility[extension.id] = false;
+        chrome.send('extensionSettingsUninstall', [extension.id]);
+      });
+      node.querySelector('.enable-controls').appendChild(trash);
+
+      // Developer mode ////////////////////////////////////////////////////////
+
+      // The path, if provided by unpacked extension.
+      var loadPath = node.querySelector('.load-path');
+      var pathLink = loadPath.querySelector('a:nth-of-type(1)');
+      pathLink.addEventListener('click', function(e) {
+        chrome.send('extensionSettingsShowPath', [String(extension.id)]);
+        e.preventDefault();
+      });
+
+      this.appendChild(node);
+      this.updateNode_(extension, node);
+    },