Fix some of the most egregious errors in Harbormaster log paging

Summary:
Depends on D19141. Ref T13088. Some of the fundamental log behaviors like "loading the correct rows" are now a bit better behaved.

The UI is a little less garbage, too.

Test Plan: Viewed some logs and loaded more context by clicking the buttons.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19142

resources/celerity/map.php

@@ -78,7 +78,7 @@ return array(
     'rsrc/css/application/feed/feed.css' => 'ecd4ec57',
     'rsrc/css/application/files/global-drag-and-drop.css' => 'b556a948',
     'rsrc/css/application/flag/flag.css' => 'bba8f811',
-    'rsrc/css/application/harbormaster/harbormaster.css' => 'fecac64f',
+    'rsrc/css/application/harbormaster/harbormaster.css' => 'c7e29d9e',
     'rsrc/css/application/herald/herald-test.css' => 'a52e323e',
     'rsrc/css/application/herald/herald.css' => 'cd8d0134',
     'rsrc/css/application/maniphest/report.css' => '9b9580b7',
@@ -416,7 +416,7 @@ return array(
     'rsrc/js/application/drydock/drydock-live-operation-status.js' => '901935ef',
     'rsrc/js/application/files/behavior-icon-composer.js' => '8499b6ab',
     'rsrc/js/application/files/behavior-launch-icon-composer.js' => '48086888',
-    'rsrc/js/application/harbormaster/behavior-harbormaster-log.js' => '0844f3c1',
+    'rsrc/js/application/harbormaster/behavior-harbormaster-log.js' => 'be6974cc',
     'rsrc/js/application/herald/HeraldRuleEditor.js' => 'dca75c0e',
     'rsrc/js/application/herald/PathTypeahead.js' => 'f7fc67ec',
     'rsrc/js/application/herald/herald-rule-editor.js' => '7ebaeed3',
@@ -579,7 +579,7 @@ return array(
     'font-fontawesome' => 'e838e088',
     'font-lato' => 'c7ccd872',
     'global-drag-and-drop-css' => 'b556a948',
-    'harbormaster-css' => 'fecac64f',
+    'harbormaster-css' => 'c7e29d9e',
     'herald-css' => 'cd8d0134',
     'herald-rule-editor' => 'dca75c0e',
     'herald-test-css' => 'a52e323e',
@@ -636,7 +636,7 @@ return array(
     'javelin-behavior-event-all-day' => 'b41537c9',
     'javelin-behavior-fancy-datepicker' => 'ecf4e799',
     'javelin-behavior-global-drag-and-drop' => '960f6a39',
-    'javelin-behavior-harbormaster-log' => '0844f3c1',
+    'javelin-behavior-harbormaster-log' => 'be6974cc',
     'javelin-behavior-herald-rule-editor' => '7ebaeed3',
     'javelin-behavior-high-security-warning' => 'a464fe03',
     'javelin-behavior-history-install' => '7ee2b591',
@@ -962,9 +962,6 @@ return array(
       'javelin-stratcom',
       'javelin-workflow',
     ),
-    '0844f3c1' => array(
-      'javelin-behavior',
-    ),
     '08f4ccc3' => array(
       'phui-oi-list-view-css',
     ),
@@ -1892,6 +1889,9 @@ return array(
       'javelin-util',
       'javelin-request',
     ),
+    'be6974cc' => array(
+      'javelin-behavior',
+    ),
     'bea6e7f4' => array(
       'javelin-install',
       'javelin-dom',

src/applications/harbormaster/controller/HarbormasterBuildLogRenderController.php

@@ -61,6 +61,7 @@ final class HarbormasterBuildLogRenderController
         'offset' => $head_offset,
         'lines' => $head_lines,
         'direction' => 1,
+        'limit' => $tail_offset,
       );
     }
 
@@ -69,6 +70,7 @@ final class HarbormasterBuildLogRenderController
         'offset' => $tail_offset,
         'lines' => $tail_lines,
         'direction' => -1,
+        'limit' => $head_offset,
       );
     }
 
@@ -128,6 +130,10 @@ final class HarbormasterBuildLogRenderController
     foreach ($views as $view_key => $view) {
       $anchor_byte = $view['offset'];
 
+      if ($view['direction'] < 0) {
+        $anchor_byte = $anchor_byte - 1;
+      }
+
       $data_key = null;
       foreach ($reads as $read_key => $read) {
         $s = $read['fetchOffset'];
@@ -148,7 +154,8 @@ final class HarbormasterBuildLogRenderController
       foreach ($reads[$data_key]['lines'] as $line_key => $line) {
         $s = $line['offset'];
         $e = $s + $line['length'];
-        if (($s <= $anchor_byte) && ($e >= $anchor_byte)) {
+
+        if (($s <= $anchor_byte) && ($e > $anchor_byte)) {
           $anchor_key = $line_key;
           break;
         }
@@ -161,9 +168,9 @@ final class HarbormasterBuildLogRenderController
       }
 
       if ($direction > 0) {
-        $slice_offset = $line_key;
+        $slice_offset = $anchor_key;
       } else {
-        $slice_offset = max(0, $line_key - ($view['lines'] - 1));
+        $slice_offset = max(0, $anchor_key - ($view['lines'] - 1));
       }
       $slice_length = $view['lines'];
 
@@ -200,12 +207,23 @@ final class HarbormasterBuildLogRenderController
           $trim = ($view_offset - $data_offset);
           $view_length -= $trim;
         }
+
+        $limit = $view['limit'];
+        if ($limit < ($view_offset + $view_length)) {
+          $view_length = ($limit - $view_offset);
+        }
       } else {
         $view_offset = $data_offset;
         $view_length = $data_length;
         if ($data_offset + $data_length > $view['offset']) {
           $view_length -= (($data_offset + $data_length) - $view['offset']);
         }
+
+        $limit = $view['limit'];
+        if ($limit > $view_offset) {
+          $view_length -= ($limit - $view_offset);
+          $view_offset = $limit;
+        }
       }
 
       $views[$view_key] += array(
@@ -232,12 +250,12 @@ final class HarbormasterBuildLogRenderController
         }
 
         $trim = ($view_offset - $line_offset);
-        $line_data = substr($line['data'], $trim);
+        if ($trim && ($trim >= strlen($line['data']))) {
-        if (!strlen($line_data)) {
           unset($lines[$line_key]);
           continue;
         }
 
+        $line_data = substr($line['data'], $trim);
         $lines[$line_key]['data'] = $line_data;
         $lines[$line_key]['length'] = strlen($line_data);
         $lines[$line_key]['offset'] += $trim;
@@ -248,16 +266,16 @@ final class HarbormasterBuildLogRenderController
       foreach ($lines as $line_key => $line) {
         $line_end = $line['offset'] + $line['length'];
         if ($line_end <= $view_end) {
-          break;
+          continue;
         }
 
         $trim = ($line_end - $view_end);
-        $line_data = substr($line['data'], -$trim);
+        if ($trim && ($trim >= strlen($line['data']))) {
-        if (!strlen($line_data)) {
           unset($lines[$line_key]);
           continue;
         }
 
+        $line_data = substr($line['data'], -$trim);
         $lines[$line_key]['data'] = $line_data;
         $lines[$line_key]['length'] = strlen($line_data);
       }
@@ -267,6 +285,17 @@ final class HarbormasterBuildLogRenderController
 
     $spacer = null;
     $render = array();
+
+    $head_view = head($views);
+    if ($head_view['viewOffset'] > $head_offset) {
+      $render[] = array(
+        'spacer' => true,
+        'head' => $head_offset,
+        'tail' => $head_view['viewOffset'],
+      );
+    }
+
+
     foreach ($views as $view) {
       if ($spacer) {
         $spacer['tail'] = $view['viewOffset'];
@@ -281,49 +310,22 @@ final class HarbormasterBuildLogRenderController
       );
     }
 
+    $tail_view = last($views);
+    if ($tail_view['viewOffset'] + $tail_view['viewLength'] < $tail_offset) {
+      $render[] = array(
+        'spacer' => true,
+        'head' => $tail_view['viewOffset'] + $tail_view['viewLength'],
+        'tail' => $tail_offset,
+      );
+    }
+
     $uri = $log->getURI();
     $highlight_range = $request->getURIData('lines');
 
     $rows = array();
     foreach ($render as $range) {
       if (isset($range['spacer'])) {
-        $rows[] = phutil_tag(
+        $rows[] = $this->renderExpandRow($range);
-          'tr',
-          array(),
-          array(
-            phutil_tag(
-              'th',
-              array(),
-              null),
-            phutil_tag(
-              'td',
-              array(),
-              array(
-                javelin_tag(
-                  'a',
-                  array(
-                    'sigil' => 'harbormaster-log-expand',
-                    'meta' => array(
-                      'headOffset' => $range['head'],
-                      'tailOffset' => $range['tail'],
-                      'head' => 4,
-                    ),
-                  ),
-                  'Show Up ^^^^'),
-                '... '.($range['tail'] - $range['head']).' bytes ...',
-                javelin_tag(
-                  'a',
-                  array(
-                    'sigil' => 'harbormaster-log-expand',
-                    'meta' => array(
-                      'headOffset' => $range['head'],
-                      'tailOffset' => $range['tail'],
-                      'tail' => 4,
-                    ),
-                  ),
-                  'Show Down VVVV'),
-              )),
-          ));
         continue;
       }
 
@@ -559,4 +561,108 @@ final class HarbormasterBuildLogRenderController
     return $views;
   }
 
+  private function renderExpandRow($range) {
+
+    $icon_up = id(new PHUIIconView())
+      ->setIcon('fa-chevron-up');
+
+    $icon_down = id(new PHUIIconView())
+      ->setIcon('fa-chevron-down');
+
+    $up_text = array(
+      pht('Show More Above'),
+      ' ',
+      $icon_up,
+    );
+
+    $expand_up = javelin_tag(
+      'a',
+      array(
+        'sigil' => 'harbormaster-log-expand',
+        'meta' => array(
+          'headOffset' => $range['head'],
+          'tailOffset' => $range['tail'],
+          'head' => 4,
+          'tail' => 0,
+        ),
+      ),
+      $up_text);
+
+    $mid_text = pht(
+      'Show More (%s bytes Hidden)',
+      new PhutilNumber($range['tail'] - $range['head']));
+
+    $expand_mid = javelin_tag(
+      'a',
+      array(
+        'sigil' => 'harbormaster-log-expand',
+        'meta' => array(
+          'headOffset' => $range['head'],
+          'tailOffset' => $range['tail'],
+          'head' => 2,
+          'tail' => 2,
+        ),
+      ),
+      $mid_text);
+
+    $down_text = array(
+      $icon_down,
+      ' ',
+      pht('Show More Below'),
+    );
+
+    $expand_down = javelin_tag(
+      'a',
+      array(
+        'sigil' => 'harbormaster-log-expand',
+        'meta' => array(
+          'headOffset' => $range['head'],
+          'tailOffset' => $range['tail'],
+          'head' => 0,
+          'tail' => 4,
+        ),
+      ),
+      $down_text);
+
+    $expand_cells = array(
+      phutil_tag(
+        'td',
+        array(
+          'class' => 'harbormaster-log-expand-up',
+        ),
+        $expand_up),
+      phutil_tag(
+        'td',
+        array(
+          'class' => 'harbormaster-log-expand-mid',
+        ),
+        $expand_mid),
+      phutil_tag(
+        'td',
+        array(
+          'class' => 'harbormaster-log-expand-down',
+        ),
+        $expand_down),
+    );
+    $expand_row = phutil_tag('tr', array(), $expand_cells);
+    $expand_table = phutil_tag(
+      'table',
+      array(
+        'class' => 'harbormaster-log-expand-table',
+      ),
+      $expand_row);
+
+    $cells = array(
+      phutil_tag('th', array()),
+      phutil_tag(
+        'td',
+        array(
+          'class' => 'harbormaster-log-expand-cell',
+        ),
+        $expand_table),
+    );
+
+    return phutil_tag('tr', array(), $cells);
+  }
+
 }

webroot/rsrc/css/application/harbormaster/harbormaster.css

@@ -37,7 +37,7 @@
   color: {$lightgreytext};
 }
 
-.harbormaster-log-table th {
+.harbormaster-log-table > tbody > tr > th {
   background-color: {$paste.highlight};
   border-right: 1px solid {$paste.border};
 
@@ -48,23 +48,77 @@
   user-select: none;
 }
 
-.harbormaster-log-table th a {
+.harbormaster-log-table > tbody > tr > th a {
   display: block;
   color: {$darkbluetext};
   text-align: right;
   padding: 2px 6px 1px 12px;
 }
 
-.harbormaster-log-table th a:hover {
+.harbormaster-log-table > tbody > tr > th a:hover {
   background: {$paste.border};
 }
 
-.harbormaster-log-table td {
+.harbormaster-log-table > tbody > tr > td {
   white-space: pre-wrap;
   padding: 2px 8px 1px;
   width: 100%;
 }
 
-.harbormaster-log-table tr.harbormaster-log-highlighted td {
+.harbormaster-log-table > tbody > tr > td.harbormaster-log-expand-cell {
+  padding: 4px 0;
+}
+
+.harbormaster-log-table tr.harbormaster-log-highlighted > td {
   background: {$paste.highlight};
 }
+
+.harbormaster-log-expand-table {
+  width: 100%;
+  background: {$paste.highlight};
+  border-top: 1px solid {$paste.border};
+  border-bottom: 1px solid {$paste.border};
+}
+
+.harbormaster-log-expand-table a {
+  display: block;
+  padding: 6px 16px;
+  color: {$darkbluetext};
+}
+
+.device-desktop .harbormaster-log-expand-table a:hover {
+  background: {$paste.border};
+  text-decoration: none;
+}
+
+.harbormaster-log-expand-table td {
+  vertical-align: middle;
+  font: 13px 'Segoe UI', 'Segoe UI Emoji', 'Segoe UI Symbol', 'Lato',
+    'Helvetica Neue', Helvetica, Arial, sans-serif;
+}
+
+
+.harbormaster-log-expand-up {
+  text-align: right;
+  width: 50%;
+}
+
+.harbormaster-log-expand-up .phui-icon-view {
+  margin: 0 0 0px 4px;
+}
+
+.harbormaster-log-expand-mid {
+  text-align: center;
+  white-space: nowrap;
+  border-left: 1px solid {$paste.border};
+  border-right: 1px solid {$paste.border};
+}
+
+.harbormaster-log-expand-down {
+  text-align: left;
+  width: 50%;
+}
+
+.harbormaster-log-expand-down .phui-icon-view {
+  margin: 0 4px 0 0;
+}

webroot/rsrc/js/application/harbormaster/behavior-harbormaster-log.js

@@ -14,6 +14,8 @@ JX.behavior('harbormaster-log', function(config) {
     e.kill();
 
     var row = e.getNode('tag:tr');
+    row = JX.DOM.findAbove(row, 'tr');
+
     var data = e.getNodeData('harbormaster-log-expand');
 
     var uri = new JX.URI(config.renderURI)
@@ -21,7 +23,7 @@ JX.behavior('harbormaster-log', function(config) {
 
     var request = new JX.Request(uri, function(r) {
       var result = JX.$H(r.markup).getNode();
-      var rows = JX.DOM.scry(result, 'tr');
+      var rows = [].slice.apply(result.firstChild.childNodes);
 
       JX.DOM.replace(row, rows);
     });