Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1438,7 +1438,7 @@ export const AST_ALLOW = {
type: 2,
tagName: 'option',
attributes: {
selected: true,
selected: '',
'aria-label': 'A',
value: 'private option A',
},
Expand Down Expand Up @@ -1526,7 +1526,7 @@ export const AST_ALLOW = {
type: 'checkbox',
name: 'inputFoo',
value: 'on',
checked: true,
checked: '',
},
childNodes: [],
},
Expand Down Expand Up @@ -1558,7 +1558,6 @@ export const AST_ALLOW = {
type: 'radio',
name: 'radioGroup',
value: 'bar-private',
checked: false,
},
childNodes: [],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,19 +61,9 @@ describe('serializeDOMAttributes', () => {
// serialized at all. Used for boolean form element attributes like `<option selected>`.
// - 'maskable-image': Like 'maskable', except that we expect the masked
// representation to be a data URL image.
// - 'maskable-option-selected': Like 'maskable-boolean', in that when unmasked the
// representation is a boolean value. However, when the value is masked, instead of
// not being serialized, its underlying string value is used. The effect is a weird
// hybrid of 'always-unmasked' and 'maskable-boolean'.
// TODO: Reduce the complexity here by aligning 'maskable-form',
// 'maskable-form-boolean', and 'maskable-option-selected'.
expectedBehavior:
| 'always-unmasked'
| 'maskable'
| 'maskable-form'
| 'maskable-form-boolean'
| 'maskable-image'
| 'maskable-option-selected'
// TODO: Reduce the complexity here by aligning 'maskable-form' and
// 'maskable-form-boolean'.
expectedBehavior: 'always-unmasked' | 'maskable' | 'maskable-form' | 'maskable-form-boolean' | 'maskable-image'

// How to treat the IGNORE privacy level. The default is 'unmasked'.
// TODO: Eliminate this inconsistency by always masking for IGNORE.
Expand Down Expand Up @@ -225,10 +215,7 @@ describe('serializeDOMAttributes', () => {
// when masked, the entire attribute should not be serialized.
{
html: '<option selected>',
// TODO: This is a bug! If the <option> is masked, we don't set the value of
// 'selected' based on the property, but we still allow the DOM attribute to be
// recorded. We should fix this; this should be 'maskable-boolean'.
expectedBehavior: 'maskable-option-selected',
expectedBehavior: 'maskable-form-boolean',
ignoreBehavior: 'masked',
},
{
Expand Down Expand Up @@ -285,13 +272,7 @@ describe('serializeDOMAttributes', () => {
attribute: { name: string; value: string | undefined },
privacyLevel: NodePrivacyLevel
): boolean | string | undefined {
let unmaskedValue: boolean | string | undefined = testCase.attrValue ?? attribute.value
if (
testCase.expectedBehavior === 'maskable-form-boolean' ||
testCase.expectedBehavior === 'maskable-option-selected'
) {
unmaskedValue = attribute.value === ''
}
const unmaskedValue: boolean | string | undefined = testCase.attrValue ?? attribute.value

let maskedValue: boolean | string | undefined
if (testCase.expectedBehavior === 'maskable-form') {
Expand All @@ -300,8 +281,6 @@ describe('serializeDOMAttributes', () => {
maskedValue = undefined
} else if (testCase.expectedBehavior === 'maskable-image') {
maskedValue = CENSORED_IMG_MARK
} else if (testCase.expectedBehavior === 'maskable-option-selected') {
maskedValue = attribute.value
} else {
maskedValue = CENSORED_STRING_MARK
}
Expand Down Expand Up @@ -346,7 +325,7 @@ describe('serializeDOMAttributes', () => {
// behavior; it shouldn't behave differently because Element#tagName is lowercase.
;(element as any).selected = true
const attributes = serializeDOMAttributes(element, NodePrivacyLevel.ALLOW, transaction)
expect(attributes['selected']).toBe(true)
expect(attributes['selected']).toBe('')
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export function serializeAttributes(
element: Element,
nodePrivacyLevel: NodePrivacyLevel,
transaction: SerializationTransaction
): Record<string, boolean | number | string> {
): Record<string, number | string> {
return {
...serializeDOMAttributes(element, nodePrivacyLevel, transaction),
...serializeVirtualAttributes(element, nodePrivacyLevel, transaction),
Expand All @@ -21,12 +21,12 @@ export function serializeDOMAttributes(
element: Element,
nodePrivacyLevel: NodePrivacyLevel,
transaction: SerializationTransaction
): Record<string, boolean | string> {
): Record<string, string> {
if (nodePrivacyLevel === NodePrivacyLevel.HIDDEN) {
return {}
}

const attrs: Record<string, string | boolean> = {}
const attrs: Record<string, string> = {}
const tagName = normalizedTagName(element)

for (let i = 0; i < element.attributes.length; i += 1) {
Expand All @@ -51,11 +51,12 @@ export function serializeDOMAttributes(
/**
* <Option> can be selected, which occurs if its `value` matches ancestor `<Select>.value`
*/
if (tagName === 'option' && nodePrivacyLevel === NodePrivacyLevel.ALLOW) {
// For privacy=`MASK`, all the values would be the same, so skip.
if (tagName === 'option') {
const optionElement = element as HTMLOptionElement
if (optionElement.selected) {
attrs.selected = optionElement.selected
if (optionElement.selected && !shouldMaskNode(optionElement, nodePrivacyLevel)) {
attrs.selected = ''
} else {
delete attrs.selected
}
}

Expand All @@ -69,9 +70,9 @@ export function serializeDOMAttributes(
*/
const inputElement = element as HTMLInputElement
if (tagName === 'input' && (inputElement.type === 'radio' || inputElement.type === 'checkbox')) {
if (nodePrivacyLevel === NodePrivacyLevel.ALLOW) {
attrs.checked = !!inputElement.checked
} else if (shouldMaskNode(inputElement, nodePrivacyLevel)) {
if (inputElement.checked && !shouldMaskNode(inputElement, nodePrivacyLevel)) {
attrs.checked = ''
} else {
delete attrs.checked
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ describe('serializeNodeWithId', () => {
jasmine.objectContaining({
attributes: {
value: 'bar',
selected: true,
selected: '',
},
}),
],
Expand Down Expand Up @@ -271,7 +271,6 @@ describe('serializeNodeWithId', () => {
attributes: {
type: 'checkbox',
value: 'on',
checked: false,
},
})
)
Expand All @@ -283,7 +282,7 @@ describe('serializeNodeWithId', () => {
attributes: {
type: 'checkbox',
value: 'on',
checked: true,
checked: '',
},
})
)
Expand All @@ -295,15 +294,14 @@ describe('serializeNodeWithId', () => {
expect(serializeNodeWithId(radio, NodePrivacyLevel.ALLOW, transaction)?.attributes).toEqual({
type: 'radio',
value: 'on',
checked: false,
})

radio.checked = true

expect(serializeNodeWithId(radio, NodePrivacyLevel.ALLOW, transaction)?.attributes).toEqual({
type: 'radio',
value: 'on',
checked: true,
checked: '',
})
})

Expand Down