Skip to content

Commit

Permalink
fix(attributes): Boolean attributes should not be special in xmlMode (#…
Browse files Browse the repository at this point in the history
…1903)

Fixes #1805
  • Loading branch information
fb55 authored Jun 2, 2021
1 parent 41f68b0 commit b393e4a
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 18 deletions.
18 changes: 18 additions & 0 deletions src/api/attributes.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,15 @@ describe('$(...)', () => {
expect($('.pear').attr('foo')).toBeUndefined();
expect($pear).toBeInstanceOf($);
});

it("(bool) shouldn't treat boolean attributes differently in XML mode", () => {
const $xml = $.load(`<input checked=checked disabled=yes />`, {
xml: true,
})('input');

expect($xml.attr('checked')).toBe('checked');
expect($xml.attr('disabled')).toBe('yes');
});
});

describe('.prop', () => {
Expand Down Expand Up @@ -302,6 +311,15 @@ describe('$(...)', () => {
'<a test-name="tester">1</a>TEXT<b test-name="tester">2</b>'
);
});

it("(bool) shouldn't treat boolean attributes differently in XML mode", () => {
const $xml = $.load(`<input checked=checked disabled=yes />`, {
xml: true,
})('input');

expect($xml.prop('checked')).toBe('checked');
expect($xml.prop('disabled')).toBe('yes');
});
});

describe('.data', () => {
Expand Down
58 changes: 40 additions & 18 deletions src/api/attributes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const primitives: Record<string, unknown> = {
const rboolean =
/^(?:autofocus|autoplay|async|checked|controls|defer|disabled|hidden|loop|multiple|open|readonly|required|scoped|selected)$/i;
// Matches strings that look like JSON objects or arrays
const rbrace = /^(?:{[\w\W]*}|\[[\w\W]*])$/;
const rbrace = /^{[^]*}$|^\[[^]*]$/;

/**
* Gets a node's attribute. For boolean attributes, it will return the value's
Expand All @@ -39,11 +39,20 @@ const rbrace = /^(?:{[\w\W]*}|\[[\w\W]*])$/;
* @param name - Name of the attribute.
* @returns The attribute's value.
*/
function getAttr(elem: Node, name: undefined): Record<string, string>;
function getAttr(elem: Node, name: string): string | undefined;
function getAttr(
elem: Node,
name: string | undefined
name: undefined,
xmlMode?: boolean
): Record<string, string>;
function getAttr(
elem: Node,
name: string,
xmlMode?: boolean
): string | undefined;
function getAttr(
elem: Node,
name: string | undefined,
xmlMode?: boolean
): Record<string, string> | string | undefined {
if (!elem || !isTag(elem)) return undefined;

Expand All @@ -56,7 +65,7 @@ function getAttr(

if (hasOwn.call(elem.attribs, name)) {
// Get the (decoded) attribute
return rboolean.test(name) ? name : elem.attribs[name];
return !xmlMode && rboolean.test(name) ? name : elem.attribs[name];
}

// Mimic the DOM and return text content as value for `option's`
Expand Down Expand Up @@ -210,7 +219,9 @@ export function attr<T extends Node>(
});
}

return arguments.length > 1 ? this : getAttr(this[0], name as string);
return arguments.length > 1
? this
: getAttr(this[0], name as string, this.options.xmlMode);
}

/**
Expand All @@ -224,16 +235,17 @@ export function attr<T extends Node>(
*/
function getProp(
el: Node | undefined,
name: string
name: string,
xmlMode?: boolean
): string | undefined | Element[keyof Element] {
if (!el || !isTag(el)) return;

return name in el
? // @ts-expect-error TS doesn't like us accessing the value directly here.
el[name]
: rboolean.test(name)
? getAttr(el, name) !== undefined
: getAttr(el, name);
: !xmlMode && rboolean.test(name)
? getAttr(el, name, false) !== undefined
: getAttr(el, name, xmlMode);
}

/**
Expand All @@ -244,12 +256,16 @@ function getProp(
* @param name - The prop's name.
* @param value - The prop's value.
*/
function setProp(el: Element, name: string, value: unknown) {
function setProp(el: Element, name: string, value: unknown, xmlMode?: boolean) {
if (name in el) {
// @ts-expect-error Overriding value
el[name] = value;
} else {
setAttr(el, name, rboolean.test(name) ? (value ? '' : null) : `${value}`);
setAttr(
el,
name,
!xmlMode && rboolean.test(name) ? (value ? '' : null) : `${value}`
);
}
}

Expand Down Expand Up @@ -353,7 +369,7 @@ export function prop<T extends Node>(
return this.html();

default:
return getProp(this[0], name);
return getProp(this[0], name, this.options.xmlMode);
}
}

Expand All @@ -363,7 +379,13 @@ export function prop<T extends Node>(
throw new Error('Bad combination of arguments.');
}
return domEach(this, (el, i) => {
if (isTag(el)) setProp(el, name, value.call(el, i, getProp(el, name)));
if (isTag(el))
setProp(
el,
name,
value.call(el, i, getProp(el, name, this.options.xmlMode)),
this.options.xmlMode
);
});
}

Expand All @@ -373,10 +395,10 @@ export function prop<T extends Node>(
if (typeof name === 'object') {
Object.keys(name).forEach((key) => {
const val = name[key];
setProp(el, key, val);
setProp(el, key, val, this.options.xmlMode);
});
} else {
setProp(el, name, value);
setProp(el, name, value, this.options.xmlMode);
}
});
}
Expand Down Expand Up @@ -810,8 +832,8 @@ export function addClass<T extends Node, R extends ArrayLike<T>>(
// If selected element isn't a tag, move on
if (!isTag(el)) continue;

// If we don't already have classes
const className = getAttr(el, 'class');
// If we don't already have classes — always set xmlMode to false here, as it doesn't matter for classes
const className = getAttr(el, 'class', false);

if (!className) {
setAttr(el, 'class', classNames.join(' ').trim());
Expand Down

0 comments on commit b393e4a

Please sign in to comment.