Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workaround bindings to textarea.placeholder in IE #5577

Merged
merged 2 commits into from
Jul 31, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions lib/mixins/template-stamp.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

placeholderBugDetect = true

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

placeholder attribute is removed when the binding is processed

* 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 the `placeholder` binding is processed, leaving an "undefined" cell in
* the binding metadata object.
*
* When `legacyOptimizations` is disabled, the template is cloned before
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When legacyOptimizations is not used, 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. There's an extra unnecessary binding.

* 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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest refactoring to fixPlaceholder and apply the fix.

return hasPlaceholderBug()
&& node.localName === 'textarea' && node.placeholder
&& node.placeholder === node.textContent;
}

function wrapTemplateExtension(node) {
let is = node.getAttribute('is');
if (is && templateExtensions[is]) {
Expand Down Expand Up @@ -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);
}
Expand Down
40 changes: 37 additions & 3 deletions test/unit/property-effects-template.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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() {

Expand Down Expand Up @@ -694,7 +694,6 @@
'x-runtime'
]);
});

});

suite('template parsing hooks', () => {
Expand Down Expand Up @@ -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>
Expand Down