Skip to content

Commit

Permalink
Fix up comments based on feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
dfreedm committed Jul 31, 2019
1 parent 61767da commit ab49f51
Showing 1 changed file with 21 additions and 19 deletions.
40 changes: 21 additions & 19 deletions lib/mixins/template-stamp.js
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
}
Expand Down

0 comments on commit ab49f51

Please sign in to comment.