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

fix: avoid reading element-specific node properties of non-element node types #4317

Merged
merged 2 commits into from Feb 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 12 additions & 17 deletions lib/core/base/virtual-node/virtual-node.js
Expand Up @@ -51,30 +51,25 @@ class VirtualNode extends AbstractVirtualNode {
// add to the prototype so memory is shared across all virtual nodes
get props() {
if (!this._cache.hasOwnProperty('props')) {
const {
nodeType,
nodeName,
id,
multiple,
nodeValue,
value,
selected,
checked,
indeterminate
} = this.actualNode;
const { nodeType, nodeName, id, nodeValue } = this.actualNode;

this._cache.props = {
nodeType,
nodeName: this._isXHTML ? nodeName : nodeName.toLowerCase(),
id,
type: this._type,
multiple,
nodeValue,
value,
selected,
checked,
indeterminate
nodeValue
};

// We avoid reading these on node types where they won't be relevant
// to work around issues like #4316.
if (nodeType === 1) {
this._cache.props.multiple = this.actualNode.multiple;
this._cache.props.value = this.actualNode.value;
this._cache.props.selected = this.actualNode.selected;
this._cache.props.checked = this.actualNode.checked;
this._cache.props.indeterminate = this.actualNode.indeterminate;
}
}

return this._cache.props;
Expand Down
48 changes: 39 additions & 9 deletions test/core/base/virtual-node/virtual-node.js
Expand Up @@ -37,15 +37,45 @@ describe('VirtualNode', () => {
assert.equal(vNode.props.type, 'text');
});

it('should reflect selected property', () => {
node = document.createElement('option');
let vNode = new VirtualNode(node);
assert.equal(vNode.props.selected, false);

node.selected = true;
vNode = new VirtualNode(node);
assert.equal(vNode.props.selected, true);
});
for (const [prop, tagName, examplePropValue] of [
['value', 'input', 'test value'],
['selected', 'option', true],
['checked', 'input', true],
['indeterminate', 'input', true],
['multiple', 'select', true]
]) {
describe(`props.${prop}`, () => {
it(`should reflect a ${tagName} element's ${prop} property`, () => {
node = document.createElement(tagName);
let vNode = new VirtualNode(node);
assert.equal(vNode.props[prop], '');

node[prop] = examplePropValue;
vNode = new VirtualNode(node);
assert.equal(vNode.props[prop], examplePropValue);
});

it('should be undefined for a text node', () => {
node = document.createTextNode('text content');
let vNode = new VirtualNode(node);
assert.equal(vNode.props[prop], undefined);
});

// Regression test for #4316
it(`should be resilient to text node with un-gettable ${prop} property`, () => {
node = document.createTextNode('text content');
Object.defineProperty(node, prop, {
get() {
throw new Error('Unqueryable value');
}
});
let vNode = new VirtualNode(node);
assert.throws(() => node[prop]);
assert.doesNotThrow(() => vNode.props[prop]);
assert.equal(vNode.props[prop], undefined);
});
});
}

it('should lowercase type', () => {
node = document.createElement('input');
Expand Down