Skip to content

Commit ab49f51

Browse files
committed
Fix up comments based on feedback
1 parent 61767da commit ab49f51

File tree

1 file changed

+21
-19
lines changed

1 file changed

+21
-19
lines changed

lib/mixins/template-stamp.js

+21-19
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ let placeholderBug = false;
2828

2929
function hasPlaceholderBug() {
3030
if (!placeholderBugDetect) {
31+
placeholderBugDetect = true;
3132
const t = document.createElement('textarea');
3233
t.placeholder = 'a';
3334
placeholderBug = t.placeholder === t.textContent;
@@ -42,27 +43,30 @@ function hasPlaceholderBug() {
4243
* If the placeholder is a binding, this can break template stamping in two
4344
* ways.
4445
*
45-
* One issue is that when the `placeholder` binding is removed, the textnode
46-
* child of the textarea is deleted, and the template info tries to bind into
47-
* that node.
46+
* One issue is that when the `placeholder` attribute is removed when the
47+
* binding is processed, the textnode child of the textarea is deleted, and the
48+
* template info tries to bind into that node.
4849
*
49-
* When `legacyOptimizations` is enabled, the node is removed from the textarea
50-
* when the `placeholder` binding is processed, leaving an "undefined" cell in
51-
* the binding metadata object.
50+
* With `legacyOptimizations` in use, when the template is stamped and the
51+
* `textarea.textContent` binding is processed, no corresponding node is found
52+
* because it was removed during parsing. An exception is generated when this
53+
* binding is updated.
5254
*
53-
* When `legacyOptimizations` is disabled, the template is cloned before
54-
* processing, and has an extra binding to the textContent of the text node
55-
* child of the textarea. This at best is an extra binding to process that has
56-
* no useful effect, and at worst throws exceptions trying to update the text
57-
* node.
55+
* With `legacyOptimizations` not in use, the template is cloned before
56+
* processing and this changes the above behavior. The cloned template also has
57+
* a value property set to the placeholder and textContent. This prevents the
58+
* removal of the textContent when the placeholder attribute is removed.
59+
* Therefore the exception does not occur. However, there is an extra
60+
* unnecessary binding.
5861
*
5962
* @param {!Node} node Check node for placeholder bug
60-
* @return {boolean} True if placeholder is bugged
63+
* @return {void}
6164
*/
62-
function shouldFixPlaceholder(node) {
63-
return hasPlaceholderBug()
64-
&& node.localName === 'textarea' && node.placeholder
65-
&& node.placeholder === node.textContent;
65+
function fixPlaceholder(node) {
66+
if (hasPlaceholderBug() && node.localName === 'textarea' && node.placeholder
67+
&& node.placeholder === node.textContent) {
68+
node.textContent = null;
69+
}
6670
}
6771

6872
function wrapTemplateExtension(node) {
@@ -294,9 +298,7 @@ export const TemplateStamp = dedupingMixin(
294298
// For ShadyDom optimization, indicating there is an insertion point
295299
templateInfo.hasInsertionPoint = true;
296300
}
297-
if (shouldFixPlaceholder(node)) {
298-
node.textContent = null;
299-
}
301+
fixPlaceholder(node);
300302
if (element.firstChild) {
301303
this._parseTemplateChildNodes(element, templateInfo, nodeInfo);
302304
}

0 commit comments

Comments
 (0)