From e76c79bc055aa61d2fceb128b0750885ed466faa Mon Sep 17 00:00:00 2001
From: Becky Siegel <beckysiegel@google.com>
Date: Mon, 6 Mar 2017 09:38:05 -0800
Subject: [PATCH] Fix problem where gr-formatted-text would sometimes not
 display

There was an issue where gr-formatted-text would not display when the
config was not loaded yet.  This change adds a function to display text
as is, in the event that the config is not yet loaded. It also refactors
the existing functions to make it more clear where config is needed.

Bug: Issue 5690
Change-Id: I74896bd59793b26b2b8fe289c13e5762c60fe8df
---
 .../diff/gr-diff-comment/gr-diff-comment.html |  2 +-
 .../gr-formatted-text/gr-formatted-text.js    | 16 +++++++--
 .../gr-formatted-text_test.html               | 23 ++++++++++++
 .../gr-linked-text/gr-linked-text_test.html   | 36 +++++++++++++++++++
 4 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
index 8ef9ad60e4..e3a33275ce 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
@@ -201,7 +201,7 @@ limitations under the License.
         </div>
       </div>
       <template is="dom-if" if="[[comment.robot_id]]">
-        <div class="robotId" hidden$="[[collapsed]]"">
+        <div class="robotId" hidden$="[[collapsed]]">
           [[comment.robot_id]]
         </div>
       </template>
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.js b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.js
index c17e9e4e14..a2130b2849 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.js
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.js
@@ -20,7 +20,10 @@
     is: 'gr-formatted-text',
 
     properties: {
-      content: String,
+      content: {
+        type: String,
+        observer: '_contentChanged',
+      },
       config: Object,
       noTrailingMargin: {
         type: Boolean,
@@ -51,6 +54,14 @@
       return this._blocksToText(this._computeBlocks(this.content));
     },
 
+    _contentChanged: function(content) {
+      // In the case where the config may not be set (perhaps due to the
+      // request for it still being in flight), set the content anyway to
+      // prevent waiting on the config to display the text.
+      if (this.config) { return; }
+      this.$.container.textContent = content;
+    },
+
     /**
      * Given a source string, update the DOM inside #container.
      */
@@ -63,7 +74,8 @@
       }
 
       // Add new content.
-      this._computeNodes(this._computeBlocks(content)).forEach(function(node) {
+      this._computeNodes(this._computeBlocks(content))
+          .forEach(function(node) {
         container.appendChild(node);
       });
     },
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.html b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.html
index 1477d43726..b0054fcf64 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.html
@@ -32,6 +32,7 @@ limitations under the License.
 <script>
   suite('gr-formatted-text tests', function() {
     var element;
+    var sandbox;
 
     function assertBlock(result, index, type, text) {
       assert.equal(result[index].type, type);
@@ -45,6 +46,11 @@ limitations under the License.
 
     setup(function() {
       element = fixture('basic');
+      sandbox = sinon.sandbox.create();
+    });
+
+    teardown(function() {
+      sandbox.restore();
     });
 
     test('parse null undefined and empty', function() {
@@ -354,5 +360,22 @@ limitations under the License.
       var expected = 'Paragraph\n\n  pre\n\nList\nOf\nItems\n\nQuote';
       assert.equal(result, expected);
     });
+
+    test('_contentOrConfigChanged not called without config', function() {
+      var contentStub = sandbox.stub(element, '_contentChanged');
+      var contentConfigStub = sandbox.stub(element, '_contentOrConfigChanged');
+      element.content = 'some text';
+      assert.isTrue(contentStub.called);
+      assert.isFalse(contentConfigStub.called);
+    });
+
+    test('_contentOrConfigChanged called with config', function() {
+      var contentStub = sandbox.stub(element, '_contentChanged');
+      var contentConfigStub = sandbox.stub(element, '_contentOrConfigChanged');
+      element.content = 'some text';
+      element.config = {};
+      assert.isTrue(contentStub.called);
+      assert.isTrue(contentConfigStub.called);
+    });
   });
 </script>
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
index 311275dd32..afb4a65456 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
@@ -35,9 +35,11 @@ limitations under the License.
 <script>
   suite('gr-linked-text tests', function() {
     var element;
+    var sandbox;
 
     setup(function() {
       element = fixture('basic');
+      sandbox = sinon.sandbox.create();
       element.config = {
         ph: {
           match: '([Bb]ug|[Ii]ssue)\\s*#?(\\d+)',
@@ -68,6 +70,10 @@ limitations under the License.
       };
     });
 
+    teardown(function() {
+      sandbox.restore();
+    });
+
     test('URL pattern was parsed and linked.', function() {
       // Reguar inline link.
       var url = 'https://code.google.com/p/gerrit/issues/detail?id=3650';
@@ -207,5 +213,35 @@ limitations under the License.
       assert.equal(links[1].href, 'ftp://foo/45');
       assert.equal(links[1].textContent, '45');
     });
+
+    test('_contentOrConfigChanged called with config', function() {
+      var contentStub = sandbox.stub(element, '_contentChanged');
+      var contentConfigStub = sandbox.stub(element, '_contentOrConfigChanged');
+      element.content = 'some text';
+      assert.isTrue(contentStub.called);
+      assert.isTrue(contentConfigStub.called);
+    });
+  });
+
+  suite('gr-linked-text with null config', function() {
+    var element;
+    var sandbox;
+
+    setup(function() {
+      element = fixture('basic');
+      sandbox = sinon.sandbox.create();
+    });
+
+    teardown(function() {
+      sandbox.restore();
+    });
+
+    test('_contentOrConfigChanged not called without config', function() {
+      var contentStub = sandbox.stub(element, '_contentChanged');
+      var contentConfigStub = sandbox.stub(element, '_contentOrConfigChanged');
+      element.content = 'some text';
+      assert.isTrue(contentStub.called);
+      assert.isFalse(contentConfigStub.called);
+    });
   });
 </script>