From 61767da2c0f21ac40123900c5a52cf645b21d117 Mon Sep 17 00:00:00 2001
From: Daniel Freedman <dfreedm@google.com>
Date: Tue, 30 Jul 2019 14:03:45 -0700
Subject: [PATCH 1/2] Workaround bindings to textarea.placeholder in IE

Textareas have a very weird bug in IE, where the text of the placeholder
is copied to the content of the textarea when set.

This a creates two bugs:
1. An unintended binding is made to the textContent of the textarea's
   text child node, meaning updates to the `placeholder` will be an
   unnecessary binding process in the best case, or an exception thrown
   when updating the text child node in the worst case.
2. When `legacyOptimizations` is enabled, the child node of the text
   area is removed when the binding for `placeholder` is processed and
   removed, leaving a binding to a `null` node, and throwing exceptions.

Therefore, when we detect this placeholder behavior, we will remove the
textnode before template processing, preventing both bugs.
---
 lib/mixins/template-stamp.js             | 46 ++++++++++++++++++++++++
 test/unit/property-effects-template.html | 40 +++++++++++++++++++--
 2 files changed, 83 insertions(+), 3 deletions(-)

diff --git a/lib/mixins/template-stamp.js b/lib/mixins/template-stamp.js
index e7a576ec2a..736091416a 100644
--- a/lib/mixins/template-stamp.js
+++ b/lib/mixins/template-stamp.js
@@ -22,6 +22,49 @@ const templateExtensions = {
   'dom-if': true,
   'dom-repeat': true
 };
+
+let placeholderBugDetect = false;
+let placeholderBug = false;
+
+function hasPlaceholderBug() {
+  if (!placeholderBugDetect) {
+    const t = document.createElement('textarea');
+    t.placeholder = 'a';
+    placeholderBug = t.placeholder === t.textContent;
+  }
+  return placeholderBug;
+}
+
+/**
+ * Some browsers have a bug with textarea, where placeholder text is copied as
+ * a textnode child of the textarea.
+ *
+ * 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.
+ *
+ * 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.
+ *
+ * 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.
+ *
+ * @param {!Node} node Check node for placeholder bug
+ * @return {boolean} True if placeholder is bugged
+ */
+function shouldFixPlaceholder(node) {
+  return hasPlaceholderBug()
+    && node.localName === 'textarea' && node.placeholder
+    && node.placeholder === node.textContent;
+}
+
 function wrapTemplateExtension(node) {
   let is = node.getAttribute('is');
   if (is && templateExtensions[is]) {
@@ -251,6 +294,9 @@ export const TemplateStamp = dedupingMixin(
         // For ShadyDom optimization, indicating there is an insertion point
         templateInfo.hasInsertionPoint = true;
       }
+      if (shouldFixPlaceholder(node)) {
+        node.textContent = null;
+      }
       if (element.firstChild) {
         this._parseTemplateChildNodes(element, templateInfo, nodeInfo);
       }
diff --git a/test/unit/property-effects-template.html b/test/unit/property-effects-template.html
index 2f761f672a..56327de127 100644
--- a/test/unit/property-effects-template.html
+++ b/test/unit/property-effects-template.html
@@ -13,7 +13,6 @@
   <meta charset="utf-8">
   <script src="../../node_modules/@webcomponents/webcomponentsjs/webcomponents-bundle.js"></script>
   <script src="wct-browser-config.js"></script>
-  <script src="wct-browser-config.js"></script>
   <script src="../../node_modules/wct-browser-legacy/browser.js"></script>
   <script type="module" src="../../polymer-element.js"></script>
   <script type="module" src="../../lib/mixins/gesture-event-listeners.js"></script>
@@ -282,9 +281,10 @@
 </dom-module>
 
 <script type="module">
-import '../../polymer-element.js';
+import {PolymerElement, html} from '../../polymer-element.js';
 import '../../lib/mixins/gesture-event-listeners.js';
 import '../../lib/elements/dom-if.js';
+import {setLegacyOptimizations} from '../../lib/utils/settings.js';
 
 suite('runtime template stamping', function() {
 
@@ -694,7 +694,6 @@
       'x-runtime'
     ]);
   });
-
 });
 
 suite('template parsing hooks', () => {
@@ -818,6 +817,41 @@
     document.body.removeChild(el);
   });
 });
+
+suite('textarea placeholder bug', function() {
+  class PlaceholderBase extends PolymerElement {
+    static get template() {
+      return html`<textarea id="textarea" placeholder="[[value]]"></textarea>`;
+    }
+    static get properties() {
+      return {value: {type: String}};
+    }
+  }
+  test('placeholder binding does not leak to textContent', function() {
+    customElements.define('placeholder-duplicate', class extends PlaceholderBase {});
+    const el = document.createElement('placeholder-duplicate');
+    document.body.appendChild(el);
+    const textarea = el.$.textarea;
+    el.value = 'before';
+    textarea.value = 'Hello!';
+    el.value = 'after';
+    assert.equal(textarea.value, 'Hello!');
+  });
+  suite('legacyOptimizations', function() {
+    suiteSetup(function() {
+      setLegacyOptimizations(true);
+    });
+    suiteTeardown(function() {
+      setLegacyOptimizations(false);
+    });
+    test('textarea placeholder binding works with legacyOptimizations', function() {
+      customElements.define('placeholder-bug', class extends PlaceholderBase {});
+      const el = document.createElement('placeholder-bug');
+      document.body.appendChild(el);
+      assert.doesNotThrow(() => {el.value = 'bar';});
+    });
+  });
+});
 </script>
 
 </body>

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 2/2] 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);
       }