Update UI and catch executeScript errors now shown in Canary

go-back-with-backspace/pages/options.js

 // Copyright 2016 Google Inc. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// Disable the whitelist field when it's not relevant.
+function updateEnabledInputs() {
+  var checkbox = document.getElementById('disableInApplets');
+  document.getElementById('whitelist').disabled = !checkbox.checked;

[Devlin, 2016/11/01 15:12:15]

Suggestion: at the top of this file (or in common.js), do
var $ = document.getElementById;

And then everywhere else, just
$('whitelist').disabled...

+}
+
 // Initialize the page.
 function init() {
-  LoadInternationalizedStrings();
+  loadInternationalizedStrings();
 
   var blacklist = document.getElementById('blacklist');
   var checkbox = document.getElementById('disableInApplets');
   var whitelist = document.getElementById('whitelist');
 
   // Configure the textboxes, allowing 200 characters for JSON serialization
   // and key length.
   blacklist.maxlength = chrome.storage.sync.QUOTA_BYTES_PER_ITEM - 200;
   whitelist.maxlength = chrome.storage.sync.QUOTA_BYTES_PER_ITEM - 200;
 
   // Set event handlers.
-  document.getElementById('done_button').onclick = function() {
+  document.getElementById('done-button').onclick = function() {
     chrome.storage.sync.set({
       // Split the lists into arrays at whitespace before saving.
       blacklist:
           document.getElementById('blacklist').value.split(/\s+/),

[Devlin, 2016/11/01 15:12:15]

Should we validate these are urls?

[Pam (message me for reviews), 2016/11/01 21:29:03]

Is there a handy, robust way to do that?

[Devlin, 2016/11/05 04:56:09]

One way would be to try to construct an URL object and catch errors, e.g.

try {
  var url = new URL(str);
} catch (e) {
  handleInvalidUrl(str);
}

It's a little inefficient, but if it's only on user action, that'd probably be
fine.

[Pam (message me for reviews), 2016/11/16 19:23:24]

Thinking about it, what I'd like to do is check the field as the user enters
items, and show non-URLs in another color. But that's a bigger change, so I'll
land and publish this first and follow up.

       disableInApplets: document.getElementById('disableInApplets').checked,
       whitelist:
           document.getElementById('whitelist').value.split(/\s+/)
     }, function() {
       // One easy way to force an error for testing is to change "sync" to
       // "managed" in the chrome.storage.sync.set() call above.
       if (chrome.runtime.lastError) {
         document.getElementById('error').textContent =
             chrome.i18n.getMessage('errorSaving',
                                    chrome.runtime.lastError.message);
       } else {
         window.close();
       }
     });
   };
 
-  document.getElementById('cancel_button').onclick = function() {
+  checkbox.onchange = updateEnabledInputs;
+
+  document.getElementById('cancel-button').onclick = function() {

[Devlin, 2016/11/01 15:12:15]

$('cancel-button').onclick = window.close;

     window.close();
   };
 
-  document.getElementById('report_page').onclick = function() {
-    reportPage();
+  document.getElementById('feedback-link').onclick = function() {

[Devlin, 2016/11/01 15:12:15]

$('feedback-link').onclick = sendFeedback;

[Pam (message me for reviews), 2016/11/01 21:29:03]

This one doesn't work, because sendFeedback takes an optional argument. If it's
not in an anonymous function, it's getting passed the click event. I could check
the arg type, but it seems cleaner to me to keep the call inside an anonymous
function.

+    sendFeedback();
   };
 
   // Load saved settings into the form fields.
   chrome.storage.sync.get({
     blacklist: [],
     disableInApplets: true,
     whitelist: []
   }, function(items) {
     blacklist.value = items.blacklist.join('\n');
     checkbox.checked = items.disableInApplets;
     whitelist.value = items.whitelist.join('\n');
+
+    updateEnabledInputs();
   });
 }
 
 window.addEventListener('load', init, false);

[Devlin, 2016/11/01 15:12:15]

useCapture defaults to false.