Take tab size into account for line-wrapping Previously, the logic was only checking exact line wrap boundaries and missed tabs advancing over the boundary. Also, line breaks were added inside span elements wrapping tabs, which prevented correct rendering. Bug: Issue 5597 Change-Id: I750547cb574c02965d5a30ba57f791841779297a

diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
index d62dbf8..54a72f6 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js

@@ -480,20 +480,38 @@
     return index + 1;
   };
 
+  GrDiffBuilder.prototype._advancePastTagClose = function(html, index) {
+    while (index < html.length &&
+           html.charCodeAt(index) !== GrDiffBuilder.GREATER_THAN_CODE) {
+      index++;
+    }
+    return index + 1;
+  };
+
   GrDiffBuilder.prototype._addNewlines = function(text, html) {
     var htmlIndex = 0;
     var indices = [];
     var numChars = 0;
+    var prevHtmlIndex = 0;
     for (var i = 0; i < text.length; i++) {
       if (numChars > 0 && numChars % this._prefs.line_length === 0) {
         indices.push(htmlIndex);
       }
       htmlIndex = this._advanceChar(html, htmlIndex);
       if (text[i] === '\t') {
+        // Advance past tab closing tag.
+        htmlIndex = this._advancePastTagClose(html, htmlIndex);
+        // ~~ is a faster Math.floor
+        if (~~(numChars / this._prefs.line_length) !==
+            ~~((numChars + this._prefs.tab_size) / this._prefs.line_length)) {
+          // Tab crosses line limit - push it to the next line.
+          indices.push(prevHtmlIndex);
+        }
         numChars += this._prefs.tab_size;
       } else {
         numChars++;
       }
+      prevHtmlIndex = htmlIndex;
     }
     var result = html;
     // Since the result string is being altered in place, start from the end

diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
index e5fe1d7..88ce477 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html

@@ -97,7 +97,7 @@
       assert.equal(buttons[2].textContent, '+10↓');
     });
 
-    test('newlines', function() {
+    test('newlines 1', function() {
       var text = 'abcdef';
       assert.equal(builder._addNewlines(text, text), text);
       text = 'a'.repeat(20);
@@ -105,8 +105,10 @@
           'a'.repeat(10) +
           GrDiffBuilder.LINE_FEED_HTML +
           'a'.repeat(10));
+    });
 
-      text = '<span class="thumbsup">👍</span>';
+    test('newlines 2', function() {
+      var text = '<span class="thumbsup">👍</span>';
       var html = '&lt;span class=&quot;thumbsup&quot;&gt;👍&lt;&#x2F;span&gt;';
       assert.equal(builder._addNewlines(text, html),
           '&lt;span clas' +
@@ -116,10 +118,13 @@
           'p&quot;&gt;👍&lt;&#x2F;spa' +
           GrDiffBuilder.LINE_FEED_HTML +
           'n&gt;');
+    });
 
-      text = '01234\t56789';
-      assert.equal(builder._addNewlines(text, text),
-          '01234\t5' +
+    test('newlines 3', function() {
+      var text = '01234\t56789';
+      var html = '01234<span>\t</span>56789';
+      assert.equal(builder._addNewlines(text, html),
+          '01234<span>\t</span>5' +
           GrDiffBuilder.LINE_FEED_HTML +
           '6789');
     });
@@ -154,6 +159,17 @@
       });
     });
 
+    test('_createTextEl linewrap with tabs', function() {
+      var text = _.times(7, _.constant('\t')).join('') + '!';
+      var line = {text: text, highlights: []};
+      var el = builder._createTextEl(line);
+      var tabEl = el.querySelector('.contentText > .br');
+      assert.isOk(tabEl);
+      assert.equal(
+          el.querySelector('.contentText .tab:nth-child(2)').nextSibling,
+          tabEl);
+    });
+
     test('text length with tabs and unicode', function() {
       assert.equal(builder._textLength('12345', 4), 5);
       assert.equal(builder._textLength('\t\t12', 4), 10);