Merge branch 'master' of git://github.com/philc/vimium

4 files changed, 138 insertions, 49 deletions

diff --git a/background_page.html b/background_page.html
index bc908481..6b957864 100644
--- a/background_page.html
+++ b/background_page.html
@@ -204,6 +204,7 @@
   keyToCommandRegistry['zo'] = 'zoomOut';
 
   keyToCommandRegistry['f']  = 'activateLinkHintsMode';
+  keyToCommandRegistry['F']  = 'activateLinkHintsModeToOpenInNewTab';
 
   keyToCommandRegistry['/']  = 'enterFindMode';
   keyToCommandRegistry['n']  = 'performFind';
diff --git a/linkHints.js b/linkHints.js
index 3f261d36..92e4a7e4 100644
--- a/linkHints.js
+++ b/linkHints.js
@@ -11,6 +11,8 @@ var linkHintsCss =
     'font-size:12px;' +
     'padding:0 1px;' +
     'line-height:100%;' +
+    'width:auto;' +
+    'display:block;' +
     'border:1px solid #E3BE23;' +
     'z-index:99999999;' +
     'font-family:"Helvetica Neue", "Helvetica", "Arial", "Sans";' +
@@ -20,10 +22,11 @@ var linkHintsCss =
   '}';
 
 var hintMarkers = [];
-var hintCharacters = "asdfjkl";
+var hintCharacters = "sadfjkluewcm";
 // The characters that were typed in while in "link hints" mode.
 var hintKeystrokeQueue = [];
 var linkHintsModeActivated = false;
+var shouldOpenLinkHintInNewTab = false;
 // Whether we have added to the page the CSS needed to display link hints.
 var linkHintsCssAdded = false;
 
@@ -31,10 +34,14 @@ var linkHintsCssAdded = false;
 // attribute, but let's wait to see if that really is necessary.
 var clickableElementsXPath = "//a | //textarea | //button | //select | //input[not(@type='hidden')]";
 
-function activateLinkHintsMode() {
+// We need this as a top-level function because our command system doesn't yet support arguments.
+function activateLinkHintsModeToOpenInNewTab() { activateLinkHintsMode(true); }
+
+function activateLinkHintsMode(openInNewTab) {
   if (!linkHintsCssAdded)
     addCssToPage(linkHintsCss);
   linkHintsModeActivated = true;
+  shouldOpenLinkHintInNewTab = openInNewTab
   buildLinkHints();
   document.addEventListener("keydown", onKeyDownInLinkHintsMode, true);
 }
@@ -47,14 +54,16 @@ function buildLinkHints() {
 
   // Initialize the number used to generate the character hints to be as many digits as we need to
   // highlight all the links on the page; we don't want some link hints to have more chars than others.
-  var digitsNeeded = digitsNeededToRepresentLinks(visibleElements.length);
-  var linkHintNumber = Math.pow(hintCharacters.length, digitsNeeded - 1);
+  var digitsNeeded = Math.ceil(logXOfBase(visibleElements.length, hintCharacters.length));
+  var linkHintNumber = 0;
   for (var i = 0; i < visibleElements.length; i++) {
-    hintMarkers.push(addMarkerFor(visibleElements[i], linkHintNumber));
+    hintMarkers.push(addMarkerFor(visibleElements[i], linkHintNumber, digitsNeeded));
     linkHintNumber++;
   }
 }
 
+function logXOfBase(x, base) { return Math.log(x) / Math.log(base); }
+
 /*
  * Returns all clickable elements that are not hidden and are in the current viewport.
  * We prune invisible elements partly for performance reasons, but moreso it's to decrease the number
@@ -75,8 +84,12 @@ function getVisibleClickableElements() {
       continue;
 
     // Using getElementFromPoint will omit elements which have visibility=hidden or display=none, and
-    // elements inside of containers that are also hidden.
-    if (!elementOccupiesPoint(element, boundingRect.left, boundingRect.top))
+    // elements inside of containers that are also hidden. Check for whether the element occupies the center
+    // of its bounding box instead of simply the upper-left corner of that box because this is more accurate
+    // when inline links have vertical padding, like in the links ("Source", "Commits") at the top of github.com.
+    // This will not exclude links with "opacity=0", like the links on Google's homepage (see bug #16).
+    if (!elementOccupiesPoint(element, boundingRect.left + boundingRect.width / 2,
+          boundingRect.top + boundingRect.height / 2))
       continue;
 
     visibleElements.push(element);
@@ -107,22 +120,6 @@ function getElementFromPoint(x, y) {
   return document.elementFromPoint(Math.ceil(x * zoomFactor), Math.ceil(y * zoomFactor));
 }
 
-/*
- * Returns the number of digits that will be needed by the link hints to represent all of the elements
- * on screen. This assumes that we want all of the elements to have the same number of characters in
- * their link hints.
- */
-function digitsNeededToRepresentLinks(numElements) {
-  for (var i = 1; i < 5; i++) {
-    var maxCharactersRepresented = Math.pow(hintCharacters.length, i);
-    for (var j = 1; j < i; j++)
-      maxCharactersRepresented -= Math.pow(hintCharacters.length, j);
-    if (maxCharactersRepresented >= numElements)
-      return i;
-  }
-  return 6;
-}
-
 function onKeyDownInLinkHintsMode(event) {
   var keyChar = String.fromCharCode(event.keyCode).toLowerCase();
   if (!keyChar)
@@ -160,11 +157,18 @@ function updateLinkHints() {
     deactivateLinkHintsMode();
   else if (linksMatched.length == 1) {
     var matchedLink = linksMatched[0];
-    // Don't navigate to the selected link immediately; we want to give the user some feedback depicting
-    // which link they've selected by focusing it. Note that for textareas and inputs, the click
-    // event is ignored, but focus causes the desired behavior.
-    setTimeout(function() { simulateClick(matchedLink); }, 600);
-    matchedLink.focus();
+    if (isInputOrText(matchedLink)) {
+      matchedLink.focus();
+      matchedLink.setSelectionRange(matchedLink.value.length, matchedLink.value.length);
+    } else {
+      // When we're opening the link in the current tab, don't navigate to the selected link immediately;
+      // we want to give the user some feedback depicting which link they've selected by focusing it.
+      if (!shouldOpenLinkHintInNewTab)
+        setTimeout(function() { simulateClick(matchedLink); }, 400);
+      else
+        simulateClick(matchedLink);
+      matchedLink.focus();
+    }
     deactivateLinkHintsMode();
   }
 }
@@ -192,24 +196,32 @@ function highlightLinkMatches(searchString) {
 
 /*
  * Converts a number like "8" into a hint string like "JK". This is used to sequentially generate all of
- * the hint text.
+ * the hint text. The hint string will be "padded with zeroes" to ensure its length is equal to numHintDigits.
  */
-function numberToHintString(number) {
+function numberToHintString(number, numHintDigits) {
   var base = hintCharacters.length;
   var hintString = [];
   var remainder = 0;
-  while (number > 0) {
+  do {
     remainder = number % base;
     hintString.unshift(hintCharacters[remainder]);
     number -= remainder;
     number /= Math.floor(base);
-  }
+  } while (number > 0);
+
+  // Pad the hint string we're returning so that it matches numHintDigits.
+  var hintStringLength = hintString.length;
+  for (var i = 0; i < numHintDigits - hintStringLength; i++)
+    hintString.unshift(hintCharacters[0]);
   return hintString.join("");
 }
 
 function simulateClick(link) {
   var event = document.createEvent("MouseEvents");
-  event.initMouseEvent("click", true, true, window, 1, 0, 0, 0, 0, false, false, false, false, 0, null);
+  // When "clicking" on a link, dispatch the event with the meta key on Mac to open it in a new tab.
+  // TODO(philc): We should dispatch this event with CTRL down on Windows and Linux.
+  event.initMouseEvent("click", true, true, window, 1, 0, 0, 0, 0, false, false, false,
+      shouldOpenLinkHintInNewTab, 0, null);
   // Debugging note: Firefox will not execute the link's default action if we dispatch this click event,
   // but Webkit will. Dispatching a click on an input box does not seem to focus it; we do that separately
   link.dispatchEvent(event);
@@ -228,8 +240,8 @@ function deactivateLinkHintsMode() {
  * Adds a link marker for the given link by adding a new element to <body> and positioning it on top of
  * the link.
  */
-function addMarkerFor(link, linkHintNumber) {
-  var hintString = numberToHintString(linkHintNumber);
+function addMarkerFor(link, linkHintNumber, linkHintDigits) {
+  var hintString = numberToHintString(linkHintNumber, linkHintDigits);
   var marker = document.createElement("div");
   marker.className = "vimiumHintMarker";
   var innerHTML = [];
diff --git a/test_harnesses/has_popup_and_link_hud.html b/test_harnesses/has_popup_and_link_hud.html
new file mode 100644
index 00000000..0f7b40d0
--- /dev/null
+++ b/test_harnesses/has_popup_and_link_hud.html
@@ -0,0 +1,51 @@
+<!--
+  This page when loaded causes Chrome to show both its "link" and "popup blocked" HUDs.
+  (mouse over the giant link to see the link HUD)
+  -->
+<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN"
+  "http://www.w3.org/TR/html4/strict.dtd">
+<html>
+<head>
+  <title>Link and popup HUD</title>
+  <style type="text/css" media="screen">
+    body {
+      height:900px;
+      width:800px;
+    }
+    
+    #biglink {
+      display:block;
+      width:400px;
+      height:400px;
+      background-color:#123456;
+    }
+
+    #hud {
+      position:absolute;
+      height:13px;
+      right:150px;
+      bottom:0;
+      background-color:#ebebeb;
+      font-family: "Lucida Grande";
+      font-size:11px;
+      padding:3px 2px 0 2px;
+      padding:3px 3px 2px 3px;
+      text-shadow: 0px 1px 2px #FFF;
+      border:1px solid #b3b3b3;
+      border-radius:4px 4px 0 0;
+    }
+      
+  </style>
+  <script type="text/javascript" charset="utf-8">
+    // Trigger the popup warning.
+    window.open("http://www.google.com");
+  </script>
+
+</head>
+<body>
+  <h2>Loading and popup HUD</h2>
+  <a id="biglink" href="http://ninjawords.com/one,two,three,four,five,six,seven,eight,nine">Big link</a>
+
+  <!-- <div id="hud">Pop-ups Blocked</div> -->
+</body>
+</html>
\ No newline at end of file
diff --git a/vimiumFrontend.js b/vimiumFrontend.js
index 1c9a4b50..ce37db3f 100644
--- a/vimiumFrontend.js
+++ b/vimiumFrontend.js
@@ -106,7 +106,10 @@ function saveZoomLevel(domain, zoomLevel) {
  * Zoom in increments of 20%; this matches chrome's CMD+ and CMD- keystrokes.
  * Set the zoom style on documentElement because document.body does not exist pre-page load.
  */
-function setPageZoomLevel(zoomLevel) { document.documentElement.style.zoom = zoomLevel + "%"; }
+function setPageZoomLevel(zoomLevel) {
+  document.documentElement.style.zoom = zoomLevel + "%";
+  HUD.updatePageZoomLevel(zoomLevel);
+}
 
 function zoomIn() {
   setPageZoomLevel(currentZoomLevel += 20);
@@ -177,9 +180,12 @@ function onKeydown(event) {
 
   if (insertMode && event.keyCode == keyCodes.ESC)
   {
-    // Remove focus so the user can't just get himself back into insert mode by typing in the same input box.
-    if (isInputOrText(event.srcElement)) { event.srcElement.blur(); }
-    exitInsertMode();
+    // Note that we can't programmatically blur out of Flash embeds from Javascript.
+    if (event.srcElement.tagName != "EMBED") {
+      // Remove focus so the user can't just get himself back into insert mode by typing in the same input box.
+      if (isInputOrText(event.srcElement)) { event.srcElement.blur(); }
+      exitInsertMode();
+    }
   }
   else if (findMode)
   {
@@ -201,15 +207,20 @@ function onKeydown(event) {
 }
 
 function onFocusCapturePhase(event) {
-  if (isInputOrText(event.target))
+  if (isFocusable(event.target))
     enterInsertMode();
 }
 
 function onBlurCapturePhase(event) {
-  if (isInputOrText(event.target))
+  if (isFocusable(event.target))
     exitInsertMode();
 }
 
+/*
+ * Returns true if the element is focusable. This includes embeds like Flash, which steal the keybaord focus.
+ */
+function isFocusable(element) { return isInputOrText(element) || element.tagName == "EMBED"; }
+
 function isInputOrText(target) {
   return ((target.tagName == "INPUT" && (target.type == "text" || target.type == "password")) ||
           target.tagName == "TEXTAREA");
@@ -294,27 +305,41 @@ HUD = {
     HUD.displayElement().style.display = "";
   },
 
+  updatePageZoomLevel: function(pageZoomLevel) {
+    // Since the chrome HUD does not scale with the page's zoom level, neither will this HUD.
+    HUD.displayElement().style.zoom = (100.0 / pageZoomLevel) * 100 + "%";
+  },
+
   /*
    * Retrieves the HUD HTML element, creating it if necessary.
    */
   displayElement: function() {
     if (!HUD._displayElement) {
+      // This is styled to precisely mimick the chrome HUD. Use the "has_popup_and_link_hud.html" test harness
+      // to tweak these styles to match Chrome's. One limitation of our HUD display is that it doesn't sit
+      // on top of horizontal scrollbars like Chrome's HUD does.
       var element = document.createElement("div");
-      element.innerHTML = "howdy";
       element.style.position = "fixed";
       element.style.bottom = "0px";
-      element.style.right = "20px";
-      element.style.backgroundColor = " #e5e5e5";
+      // Keep this far enough to the right so that it doesn't collide with the "popups blocked" chrome HUD.
+      element.style.right = "150px";
+      element.style.height = "13px";
       element.style.maxWidth = "400px";
+      element.style.minWidth = "150px";
+      element.style.backgroundColor = "#ebebeb";
       element.style.fontSize = "11px";
-      element.style.padding = "3px";
-      element.style.border = "1px solid #cccccc";
-      element.style.borderBottomWidth = "0px";
-      // element.style.fontFamily = "monospace";
+      element.style.padding = "3px 3px 2px 3px";
+      element.style.border = "1px solid #b3b3b3";
+      element.style.borderRadius = "4px 4px 0 0";
+      element.style.fontFamily = "Lucida Grande";
+      element.style.textShadow = "0px 1px 2px #FFF";
+      element.style.display = "none";
+
       document.body.appendChild(element);
       HUD._displayElement = element
+      HUD.updatePageZoomLevel(currentZoomLevel);
     }
-    return HUD._displayElement
+    return HUD._displayElement;
   },
 
   hide: function() {