Safe Browsing malware interstitial redesign field trial.

chrome/browser/resources/safe_browsing/malware_block_v2.js

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+

[Dan Beam, 2012/08/22 02:34:03]

might want to add a doc comment (i.e. /** This is what I'm doing. */) to each of
these functions

[mattm, 2012/08/22 20:33:47]

Done.

+function sendCommand(cmd) {
+  window.domAutomationController.setAutomationId(1);
+  window.domAutomationController.send(cmd);
+}
+
+function savePreference() {
+  var checkBox = $('check-report');
+  if (checkBox.checked) {

[Dan Beam, 2012/08/22 02:34:03]

nit: no curlies for 1-liners at every level (conditional expressions, block
bodies, etc.)

[mattm, 2012/08/22 20:33:47]

Done.

+    sendCommand('doReport');
+  } else {
+    sendCommand('dontReport');
+  }
+}
+
+function seeMore() {
+  if ($('see-less-text').hidden) {
+    $('see-more-text').hidden = true;
+    $('see-less-text').hidden = false;
+    $('see-more-contents').hidden = false;
+    sendCommand('expandedSeeMore');
+  } else {
+    $('see-more-text').hidden = false;
+    $('see-less-text').hidden = true;
+    $('see-more-contents').hidden = true;
+  }
+}
+
+/**
+ * Context menu handler to disable context menu during interstitial.

[arv (Not doing code reviews), 2012/08/22 14:33:45]

Why?

[mattm, 2012/08/22 20:33:47]

I think it's because some browser functions don't work correctly when on an
interstitial page.  But on further testing I see the InterstitialPage
implementation itself already does this in c++, so I removed this handler.

+ * @param {Event} e The event which triggered the handler.
+ */
+document.oncontextmenu = function(e) {
+  e.preventDefault();
+};
+
+/**
+ * Onload listener to initialize javascript handlers.
+ */
+window.onload = function() {

[arv (Not doing code reviews), 2012/08/22 14:33:45]

DOMContentLoaded is faster

http://ie.microsoft.com/testdrive/HTML5/DOMContentLoaded/Default.html

[mattm, 2012/08/22 20:33:47]

Done.

+  $('back').onclick = function() {
+    sendCommand('takeMeBack');
+  }

[Dan Beam, 2012/08/22 02:34:03]

}; (after all these function expressions)

[mattm, 2012/08/22 20:33:47]

Done.

+  $('proceed').onclick = function(e) {
+    sendCommand('proceed');
+    e.preventDefault();

[Dan Beam, 2012/08/22 02:34:03]

none of these need prevenDefault() if they're simply clicks on <a href="#">,
that's what preventDefaultOnPoundLinkClicks() does.

[arv (Not doing code reviews), 2012/08/22 14:33:45]

Another rule is to never have a preventDefault or stopPropagation without a
comment explaining why. People abuse these two a lot.

[mattm, 2012/08/22 20:33:47]

Removed them all except for the 'see-more-link' one. 
preventDefaultOnPoundLinkClicks doesn't catch it since the click target is
actually the <span> inside the link.

[mattm, 2012/08/22 20:33:47]

Done.

+  }
+  $('learn-more-link').onclick = function(e) {
+    sendCommand('learnMore2');
+    e.preventDefault();
+  }
+  $('show-diagnostic-link').onclick = function(e) {
+    sendCommand('showDiagnostic');
+    e.preventDefault();
+  }
+  $('see-more-link').onclick = function(e) {
+    seeMore();
+    e.preventDefault();
+  }
+  $('check-report').onclick = savePreference;
+
+  preventDefaultOnPoundLinkClicks();  // From shared/js/util.js.
+};