Bug 1511275 - Unify line numbers and source code listings in DOM. r=kats

.gitignore

@@ -11,8 +11,7 @@ clang-plugin/analysis
 /js
 tools/target
 paths
-config.json
 env
 repos
-index
+/index
 addons

static/css/mozsearch.css

@@ -119,37 +119,48 @@ td.code {
 td#line-numbers {
     padding: 0;
 }
+
+.source-line-with-number {
+  padding: 0rem 1rem;
+  line-height: 1.3;
+  display: flex;
+}
+
+/* In order to display the line numbers and have them visible to screen readers
+   but not appear in any copy-and-pasted selection, we create a before
+   pseudo-element with generated content. */
+.line-number::before {
+    content: attr(data-line-number);
+}
 .line-number {
-    display: block;
+    display: inline-block;
     cursor: pointer;
-    -moz-user-select: none;
-    -o-user-select: none;
-    -khtml-user-select: none;
-    -webkit-user-select: none;
-    -ms-user-select: none;
     user-select: none;
     text-align: right;
     padding: 0 0.5rem;
-    position: relative;
+    /* this was originally set on the ".file td:first-child" */
+    color: #aaa;
+    width: 8ex;
 }
 .highlighted,
 .multihighlight {
     background: none repeat scroll 0 0 rgb(255, 255, 204) !important;
 }
-td.code {
-    display: block;
+.source-line {
     margin: 0;
     padding: 0;
-    position: relative;
-    z-index: 1;
+    display: inline-block;
+    white-space: pre;
+    flex-grow: 1;
 }
-td pre code {
-    padding: 0 0 0 1em;
-    margin: 0;
-    padding: 0;
-    display: block;
+.source-line-with-number.stuck {
+  background: #ddf;
+}
+.source-line-with-number.last-stuck {
+  background: linear-gradient(#ddf, #ddf 90%, blue);
 }
-code a {
+
+.source-line a {
     cursor: pointer;
 }
 .code code {
@@ -205,18 +216,10 @@ body {
 
 /**
  * Nesting!
- *
- * Because the search bar thing is position: fixed we need to add an offset to
- * all of our position:sticky "top" directives.  This needs to be the height of
- * the search box.  CSS variables provide us a way to adjust this based on other
- * factors.  Currently the main influencing factor is whether there is a
- * "Showing REVHASH: Commit message" line or if it's empty.  The presence of the
- * line matches up with the "old-rev" class being set on the body, so we use
- * this to decide which hard-coded value to use below.  JS could also update
- * the variable, but we currently don't need to do that.
  */
 
 .nesting-container {
+  display: block;
   position: relative;
 }
 .nesting-depth-1,
@@ -230,47 +233,50 @@ body {
 .nesting-depth-9,
 .nesting-depth-10 {
   position: sticky;
-  background-color: #f8f8f8;
+  /* even with our "stuck" and "last-stuck" classes, it's important to have an
+     opaque background color for these because those classes don't take effect
+     immediately. */
+  background-color: white;
 }
 .nesting-depth-1 {
   top: calc(0 * 12px * 1.3);
-  z-index: 900;
+  z-index: 100;
 }
 .nesting-depth-2 {
   top: calc(1 * 12px * 1.3);
-  z-index: 899;
+  z-index: 99;
 }
 .nesting-depth-3 {
   top: calc(2 * 12px * 1.3);
-  z-index: 898;
+  z-index: 98;
 }
 .nesting-depth-4 {
   top: calc(3 * 12px * 1.3);
-  z-index: 897;
+  z-index: 97;
 }
 .nesting-depth-5 {
   top: calc(4 * 12px * 1.3);
-  z-index: 896;
+  z-index: 96;
 }
 .nesting-depth-6 {
   top: calc(5 * 12px * 1.3);
-  z-index: 895;
+  z-index: 95;
 }
 .nesting-depth-7 {
   top: calc(6 * 12px * 1.3);
-  z-index: 894;
+  z-index: 94;
 }
 .nesting-depth-8 {
   top: calc(7 * 12px * 1.3);
-  z-index: 893;
+  z-index: 93;
 }
 .nesting-depth-9 {
   top: calc(8 * 12px * 1.3);
-  z-index: 892;
+  z-index: 92;
 }
 .nesting-depth-10 {
   top: calc(9 * 12px * 1.3);
-  z-index: 891;
+  z-index: 91;
 }
 
 /* Search results */
@@ -373,7 +379,7 @@ body {
     border-top-left-radius: 0;
     width: auto;
     list-style: none;
-    z-index: 2;
+    z-index: 102;
 }
 .context-menu a {
     display: block;
@@ -388,7 +394,7 @@ body {
 }
 
 .content {
-    padding: 1rem 2rem;
+    padding: 0.5rem 0rem;
 }
 
 /* Breadcrumbs */
@@ -398,7 +404,7 @@ body {
 .breadcrumbs {
     display: inline-block;
     margin: 0;
-    padding: .5rem 0 1rem 0;
+    padding: 1rem;
     text-align: left;
 }
 
@@ -440,7 +446,7 @@ body {
     overflow-y: auto;
     overflow-x: hidden;
     max-height: calc(100% - 150px);
-    z-index: 2;
+    z-index: 102;
 }
 .panel button{
     display: inline-block;
@@ -535,11 +541,12 @@ span[data-symbols].hovered {
 
 /* Blame */
 .blame-strip {
-    top: 0px;
+    display: inline-block;
+    position: relative;
     width: 20px;
-    left: -20px;
     height: 16px;
-    position: absolute;
+    padding: 0;
+    margin: 0;
 }
 .c1 {
     background: lightgray;
@@ -551,15 +558,15 @@ span[data-symbols].hovered {
     display: inline !important;
     position: absolute;
     top: 0;
-    left: 0;
+    left: 20px;
     width: 600px;
     padding: 10px;
     background: #404040;
     box-shadow: 3px 3px 3px grey;
     border-radius: 3px;
     color: white;
     text-align: left;
-    z-index: 100;
+    z-index: 200;
     cursor: auto;
     -moz-user-select: text;
     -o-user-select: text;

static/js/blame.js

@@ -1,12 +1,25 @@
 var popupBox = null;
 
+// The .blame-strip element for which blame is currently being displayed.
 var blameElt;
+// The .blame-strip element the mouse is hovering over.  Clicking on the element
+// will null this out as a means of letting the user get rid of the popup if
+// they didn't actually want to see the pop-up.
 var mouseElt;
 
+// A very simply MRU cache of size 1.  We don't issue an XHR if we already have
+// the data.  This is important for the case where the user is moving their
+// mouse along the same contiguous run of blame data.  In that case, the
+// blameElt changes, but the `revs` stays the same.
 var prevRevs;
 var prevJson;
 
+/**
+ * Asynchronously initiates lookup and display of the blame data for the current
+ * blameElt.  The popup is added as a child of the blameElt in the DOM.
+ */
 function updateBlamePopup() {
+  // If there's no blameElt, just remove the popup if it exists and bail.
   if (!blameElt) {
     if (popupBox) {
       popupBox.remove();
@@ -15,6 +28,8 @@ function updateBlamePopup() {
     return;
   }
 
+  // Latch the current blameElt in case by the time our XHR comes back it's no
+  // longer the current blameElt.
   var elt = blameElt;
   var blame = elt.dataset.blame;
 
@@ -23,6 +38,8 @@ function updateBlamePopup() {
   var tree = $("#data").data("tree");
 
   function showPopup(json) {
+    // If the XHR was too slow, we may no longer want to display blame for this
+    // element, bail.
     if (blameElt != elt) {
       return;
     }
@@ -80,14 +97,13 @@ function updateBlamePopup() {
       content += `<br><details><summary>${ignored.length} ignored changesets</summary>${ignored.join("")}</details>`;
     }
 
-    var parent = blameElt.parentNode;
     var height = blameElt.getBoundingClientRect().height;
 
     popupBox = document.createElement("div");
     popupBox.id = "blame-popup";
     popupBox.innerHTML = content;
     popupBox.className = "blame-popup";
-    parent.appendChild(popupBox);
+    blameElt.appendChild(popupBox);
 
     $(popupBox).on("mouseenter", blameHoverHandler);
     $(popupBox).on("mouseleave", blameHoverHandler);
@@ -111,6 +127,7 @@ function updateBlamePopup() {
   }
 }
 
+
 function setBlameElt(elt) {
   if (blameElt == elt) {
     return;
@@ -125,10 +142,15 @@ function setBlameElt(elt) {
 }
 
 function blameHoverHandler(event) {
+  // Suppress the blame hover popup if the context menu is visible.
   if ($('#context-menu').length) {
     return;
   }
 
+  // Debounced pop-up closer.  If the mouse leaves a blame-strip element and
+  // doesn't move onto another one within 100ms, close the popup.  Also, if the
+  // user clicks on the blame-strip element and doesn't move onto a new element,
+  // close the pop-up.
   if (event.type == "mouseleave" ||
       (event.type == "click" && blameElt != null)) {
     mouseElt = null;
@@ -140,6 +162,11 @@ function blameHoverHandler(event) {
       }
     }, 100);
   } else {
+    // This is a mouseenter event.  Because the popup is a child of the
+    // .blame-strip element, we may be deep inside popup content if the popup is
+    // visible.  So we walk upwards until we find either a .blame-strip element
+    // with data-blame set (which may be a new blameElt) or the existing popup,
+    // in which case we can reuse the blameElt.
     var elt = event.target;
     while (elt && elt instanceof Element) {
       if (elt.hasAttribute("data-blame")) {
@@ -164,4 +191,3 @@ function blameHoverHandler(event) {
 $(".blame-strip").on("mouseenter", blameHoverHandler);
 $(".blame-strip").on("mouseleave", blameHoverHandler);
 $(".blame-strip").on("click", blameHoverHandler);
-