Skip to content

Commit 0a9d33c

Browse files
Fix an issue with PE nav updating form fields
1 parent 208aeda commit 0a9d33c

File tree

2 files changed

+60
-1
lines changed

2 files changed

+60
-1
lines changed

src/Components/Web.JS/src/Rendering/DomSpecialPropertyUtil.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,15 @@ export function applyAnyDeferredValue(element: Element) {
6969
trySetSelectValueFromOptionElement(element);
7070
} else if (deferredValuePropname in element) {
7171
// Situation 2
72-
setDeferredElementValue(element, element[deferredValuePropname]);
72+
const deferredValue = element[deferredValuePropname];
73+
setDeferredElementValue(element, deferredValue);
74+
75+
// Once the deferred value is accepted, stop tracking it to ensure it doesn't reappear later
76+
// If the value isn't yet accepted, keep tracking it. This is relevant, e.g., for <select> in the
77+
// case where we haven't yet got the corresponding <option>
78+
if ((element as any).value === deferredValue) {
79+
delete element[deferredValuePropname];
80+
}
7381
}
7482
}
7583

src/Components/Web.JS/test/DomSync.test.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,57 @@ describe('DomSync', () => {
408408
expect(inputRange.max).toBe('1050');
409409
});
410410

411+
test('should not replay old deferred value on subsequent update (input)', () => {
412+
// This case may seem obscure but represents a bug that existed at one point.
413+
// The 'deferred' values tracked for some element types need to be cleared
414+
// after usage otherwise older values can overwrite newer ones.
415+
416+
const destination = makeExistingContent(`<input value='First'>`);
417+
const newContent1 = makeNewContent(`<input value='Second'>`);
418+
const newContent2 = makeNewContent(`<input value='Third'>`);
419+
420+
const elem = destination.startExclusive.nextSibling as HTMLInputElement;
421+
expect(elem.value).toBe('First');
422+
423+
// Act/Assert 1: Initial update
424+
synchronizeDomContent(destination, newContent1);
425+
expect(elem.value).toBe('Second');
426+
427+
// Act/Assert 2: The user performs an edit, then we try to synchronize the DOM
428+
// with some content that matches the edit exactly. The diff algorithm will see
429+
// that the DOM already matches the desired output, so it won't track any new
430+
// deferred value. We need to check the old deferred value doesn't reappear.
431+
elem.value = 'Third';
432+
synchronizeDomContent(destination, newContent2);
433+
expect(elem.value).toBe('Third');
434+
});
435+
436+
test('should not replay old deferred value on subsequent update (select)', () => {
437+
// This case may seem obscure but represents a bug that existed at one point.
438+
// The 'deferred' values tracked for some element types need to be cleared
439+
// after usage otherwise older values can overwrite newer ones.
440+
441+
const destination = makeExistingContent(`<select><option value='v1' selected /><option value='v2' /><option value='v3' /></select>`);
442+
const newContent1 = makeNewContent(`<select><option value='v1' /><option value='v2' selected /><option value='v3' /></select>`);
443+
const newContent2 = makeNewContent(`<select><option value='v1' /><option value='v2' /><option value='v3' selected /></select>`);
444+
445+
const elem = destination.startExclusive.nextSibling as HTMLSelectElement;
446+
expect(elem.selectedIndex).toBe(0);
447+
448+
// Act/Assert 1: Initial update
449+
synchronizeDomContent(destination, newContent1);
450+
expect(elem.selectedIndex).toBe(1);
451+
452+
// Act/Assert 2: The user performs an edit, then we try to synchronize the DOM
453+
// with some content that matches the edit exactly. The diff algorithm will see
454+
// that the DOM already matches the desired output, so it won't track any new
455+
// deferred value. We need to check the old deferred value doesn't reappear.
456+
elem.selectedIndex = 2;
457+
expect(elem.selectedIndex).toBe(2);
458+
synchronizeDomContent(destination, newContent2);
459+
expect(elem.selectedIndex).toBe(2);
460+
});
461+
411462
test('should treat doctype nodes as unchanged', () => {
412463
// Can't update a doctype after the document is created, nor is there a use case for doing so
413464
// We just have to skip them, as it would be an error to try removing or inserting them

0 commit comments

Comments
 (0)