From ab49f51a669d173d9dcbdf38b6d57f5561033bb1 Mon Sep 17 00:00:00 2001
From: Daniel Freedman <dfreedm@google.com>
Date: Tue, 30 Jul 2019 17:23:27 -0700
Subject: [PATCH] Fix up comments based on feedback

---
 lib/mixins/template-stamp.js | 40 +++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/lib/mixins/template-stamp.js b/lib/mixins/template-stamp.js
index 736091416a..e9e4a16da7 100644
--- a/lib/mixins/template-stamp.js
+++ b/lib/mixins/template-stamp.js
@@ -28,6 +28,7 @@ let placeholderBug = false;
 
 function hasPlaceholderBug() {
   if (!placeholderBugDetect) {
+    placeholderBugDetect = true;
     const t = document.createElement('textarea');
     t.placeholder = 'a';
     placeholderBug = t.placeholder === t.textContent;
@@ -42,27 +43,30 @@ function hasPlaceholderBug() {
  * If the placeholder is a binding, this can break template stamping in two
  * ways.
  *
- * One issue is that when the `placeholder` binding is removed, the textnode
- * child of the textarea is deleted, and the template info tries to bind into
- * that node.
+ * One issue is that when the `placeholder` attribute is removed when the
+ * binding is processed, the textnode child of the textarea is deleted, and the
+ * template info tries to bind into that node.
  *
- * When `legacyOptimizations` is enabled, the node is removed from the textarea
- * when the `placeholder` binding is processed, leaving an "undefined" cell in
- * the binding metadata object.
+ * With `legacyOptimizations` in use, when the template is stamped and the
+ * `textarea.textContent` binding is processed, no corresponding node is found
+ * because it was removed during parsing. An exception is generated when this
+ * binding is updated.
  *
- * When `legacyOptimizations` is disabled, the template is cloned before
- * processing, and has an extra binding to the textContent of the text node
- * child of the textarea. This at best is an extra binding to process that has
- * no useful effect, and at worst throws exceptions trying to update the text
- * node.
+ * With `legacyOptimizations` not in use, the template is cloned before
+ * processing and this changes the above behavior. The cloned template also has
+ * a value property set to the placeholder and textContent. This prevents the
+ * removal of the textContent when the placeholder attribute is removed.
+ * Therefore the exception does not occur. However, there is an extra
+ * unnecessary binding.
  *
  * @param {!Node} node Check node for placeholder bug
- * @return {boolean} True if placeholder is bugged
+ * @return {void}
  */
-function shouldFixPlaceholder(node) {
-  return hasPlaceholderBug()
-    && node.localName === 'textarea' && node.placeholder
-    && node.placeholder === node.textContent;
+function fixPlaceholder(node) {
+  if (hasPlaceholderBug() && node.localName === 'textarea' && node.placeholder
+        && node.placeholder === node.textContent) {
+    node.textContent = null;
+  }
 }
 
 function wrapTemplateExtension(node) {
@@ -294,9 +298,7 @@ export const TemplateStamp = dedupingMixin(
         // For ShadyDom optimization, indicating there is an insertion point
         templateInfo.hasInsertionPoint = true;
       }
-      if (shouldFixPlaceholder(node)) {
-        node.textContent = null;
-      }
+      fixPlaceholder(node);
       if (element.firstChild) {
         this._parseTemplateChildNodes(element, templateInfo, nodeInfo);
       }