From bcd6dba57d2cef55afd49fda71367bd12c849a61 Mon Sep 17 00:00:00 2001
From: Tao Zhou <taoalpha@google.com>
Date: Wed, 16 Oct 2019 10:22:48 +0200
Subject: [PATCH] Fix for disabled syntax_highlighting

Change-Id: I8ee96702d99af9c7503832d9478a577bc0d14e52
---
 .../diff/gr-diff-host/gr-diff-host.js         | 26 +++++++++--
 .../diff/gr-diff-host/gr-diff-host_test.html  | 46 +++++++++++++++++--
 2 files changed, 64 insertions(+), 8 deletions(-)

diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
index 7382af6a8f..d6aa8ee710 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
@@ -223,7 +223,7 @@
       _syntaxHighlightingEnabled: {
         type: Boolean,
         computed:
-          '_isSyntaxHighlightingEnabled(prefs.syntax_highlighting, _diff)',
+          '_isSyntaxHighlightingEnabled(prefs.*, diff)',
       },
 
       _layers: {
@@ -258,6 +258,7 @@
     observers: [
       '_whitespaceChanged(prefs.ignore_whitespace, _loadedWhitespaceLevel,' +
           ' noRenderOnPrefsChange)',
+      '_syntaxHighlightingChanged(noRenderOnPrefsChange, prefs.*)',
     ],
 
     ready() {
@@ -811,6 +812,24 @@
       }
     },
 
+    _syntaxHighlightingChanged(noRenderOnPrefsChange, prefsChangeRecord) {
+      // Polymer 2: check for undefined
+      if ([
+        noRenderOnPrefsChange,
+        prefsChangeRecord,
+      ].some(arg => arg === undefined)) {
+        return;
+      }
+
+      if (prefsChangeRecord.path !== 'prefs.syntax_highlighting') {
+        return;
+      }
+
+      if (!noRenderOnPrefsChange) {
+        this.reload();
+      }
+    },
+
     /**
      * @param {Object} patchRangeRecord
      * @return {number|null}
@@ -892,8 +911,9 @@
           item => item.__draftID === comment.__draftID);
     },
 
-    _isSyntaxHighlightingEnabled(preference, diff) {
-      if (!preference) return false;
+    _isSyntaxHighlightingEnabled(preferenceChangeRecord, diff) {
+      if (!preferenceChangeRecord || !diff) return false;
+      if (!preferenceChangeRecord.base.syntax_highlighting) return false;
       return !this._anyLineTooLong(diff) &&
           this.$.diff.getDiffLength(diff) <= SYNTAX_MAX_DIFF_LENGTH;
     },
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
index cdbdeb5bab..16b7728d25 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
@@ -1344,7 +1344,7 @@ limitations under the License.
           Gerrit.DiffSide.RIGHT), [r]);
     });
 
-    suite('syntax layer', () => {
+    suite('syntax layer with syntax_highlighting on', () => {
       setup(() => {
         const prefs = {
           line_length: 10,
@@ -1353,17 +1353,17 @@ limitations under the License.
           context: -1,
           syntax_highlighting: true,
         };
+        element.patchRange = {};
         element.prefs = prefs;
       });
 
       test('gr-diff-host provides syntax highlighting layer to gr-diff', () => {
-        element.patchRange = {};
         element.reload();
         assert.equal(element.$.diff.layers[0], element.$.syntaxLayer);
       });
 
       test('rendering normal-sized diff does not disable syntax', () => {
-        element._diff = {
+        element.diff = {
           content: [{
             a: ['foo'],
           }],
@@ -1373,7 +1373,7 @@ limitations under the License.
 
       test('rendering large diff disables syntax', () => {
         // Before it renders, set the first diff line to 500 '*' characters.
-        element._diff = {
+        element.diff = {
           content: [{
             a: [new Array(501).join('*')],
           }],
@@ -1385,7 +1385,6 @@ limitations under the License.
         sandbox.stub(element.$.syntaxLayer, 'process').returns(Promise.resolve());
         sandbox.stub(element.$.restAPI, 'getDiff').returns(
             Promise.resolve({content: []}));
-        element.patchRange = {};
         element.reload();
         setTimeout(() => {
           element.dispatchEvent(
@@ -1395,5 +1394,42 @@ limitations under the License.
         });
       });
     });
+
+    suite('syntax layer with syntax_highlgihting off', () => {
+      setup(() => {
+        const prefs = {
+          line_length: 10,
+          show_tabs: true,
+          tab_size: 4,
+          context: -1,
+        };
+        element.diff = {
+          content: [{
+            a: ['foo'],
+          }],
+        };
+        element.patchRange = {};
+        element.prefs = prefs;
+      });
+
+      test('gr-diff-host provides syntax highlighting layer', () => {
+        element.reload();
+        assert.equal(element.$.diff.layers[0], element.$.syntaxLayer);
+      });
+
+      test('syntax layer should be disabled', () => {
+        assert.isFalse(element.$.syntaxLayer.enabled);
+      });
+
+      test('still disabled for large diff', () => {
+        // Before it renders, set the first diff line to 500 '*' characters.
+        element.diff = {
+          content: [{
+            a: [new Array(501).join('*')],
+          }],
+        };
+        assert.isFalse(element.$.syntaxLayer.enabled);
+      });
+    });
   });
 </script>