Skip to content

Commit

Permalink
[BUGFIX] Fix link-element initialization race condition
Browse files Browse the repository at this point in the history
Children of custom elements are not guaranteed to be available by the
time the element is added to the DOM — i.e. when the connectedCallback()
is triggered by the browser. That means we can not assume that all
child elements will already be available. See #102550 for details.

To circumvent this race we make use of HTML event bubbling. That means
we register our event handlers on the wrapper element and will
receive events that are triggered on one of the children elements.
To circumvent the initialization race conditions in connectedCallback()
we shift the stateful initialization to the server-side rendering.

Note that we either need to render html components server or client-side
in order to avoid such race conditions or nasty workarounds (like adding
mutation observs and keeping track whether all elements have already
been added or not). Ideally the link-element component would be rendered
fully client-side at some point, but that's out of scope for now.

Resolves: #102603
Related: #102550
Releases: main, 12.4
Change-Id: I54715fd671f717e4253ee41e8ac33a1ca5960eae
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/82169
Reviewed-by: Benjamin Franzke <ben@bnf.dev>
Tested-by: core-ci <typo3@b13.com>
Tested-by: Benjamin Franzke <ben@bnf.dev>
  • Loading branch information
bnf committed Dec 11, 2023
1 parent 7546b03 commit 1f5e344
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 43 deletions.
3 changes: 2 additions & 1 deletion Build/Sources/Sass/typo3/_main_form.scss
Expand Up @@ -449,11 +449,12 @@ textarea {
min-width: 0;
}

.form-control:not(.hidden) + .form-control-clearable-wrapper {
.form-control:not(.hidden):not([hidden]) + .form-control-clearable-wrapper {
flex-grow: 0;
width: auto;
}

.form-control[hidden] + .close,
.form-control.hidden + .close {
display: none;
}
Expand Down
Expand Up @@ -18,6 +18,7 @@ enum Selectors {
inputFieldSelector = '.t3js-form-field-link-input',
explanationSelector = '.t3js-form-field-link-explanation',
iconSelector = '.t3js-form-field-link-icon',
containerSelector = '.t3js-form-field-link',
}

/**
Expand All @@ -34,64 +35,86 @@ enum Selectors {
* https://developer.mozilla.org/en-US/docs/Web/Web_Components/Using_custom_elements
*/
class LinkElement extends HTMLElement {
private element: HTMLSelectElement = null;
private container: HTMLElement = null;
private toggleSelector: HTMLButtonElement = null;
private explanationField: HTMLInputElement = null;
private icon: HTMLSpanElement = null;
constructor() {
super();

public connectedCallback(): void {
this.addEventListener('click', (e: Event) => this.handleClick(e));
this.addEventListener('change', (e: Event) => this.handleChange(e));
}

private get element(): HTMLSelectElement {
const recordFieldId = this.getAttribute('recordFieldId');
if (recordFieldId === null) {
return;
throw new Error('Missing recordFieldId attribute on <typo3-formengine-element-link>');
}

this.element = this.querySelector<HTMLSelectElement>(selector`#${recordFieldId}`);
if (!this.element) {
return;
const element = this.querySelector<HTMLSelectElement>(selector`#${recordFieldId}`);
if (element === null) {
throw new Error(`recordFieldId #${recordFieldId} not found in <typo3-formengine-element-link>`);
}

this.container = <HTMLElement>this.element.closest('.t3js-form-field-link');
this.toggleSelector = <HTMLButtonElement>this.container.querySelector(Selectors.toggleSelector);
this.explanationField = <HTMLInputElement>this.container.querySelector(Selectors.explanationSelector);
this.icon = <HTMLSpanElement>this.container.querySelector(Selectors.iconSelector);
this.toggleVisibility(this.explanationField.value === '');
this.registerEventHandler();
return element;
}

/**
* @param {boolean} explanationShown
*/
private toggleVisibility(explanationShown: boolean): void {
this.explanationField.classList.toggle('hidden', explanationShown);
this.element.classList.toggle('hidden', !explanationShown);
private get container(): HTMLElement {
return this.element.closest<HTMLElement>(Selectors.containerSelector);
}

private registerEventHandler(): void {
this.toggleSelector.addEventListener('click', (e: Event): void => {
e.preventDefault();
private get toggleSelector(): HTMLButtonElement {
return this.container.querySelector<HTMLButtonElement>(Selectors.toggleSelector);
}

const explanationShown = !this.explanationField.classList.contains('hidden');
this.toggleVisibility(explanationShown);
});
private get explanationField(): HTMLInputElement {
return this.container.querySelector<HTMLInputElement>(Selectors.explanationSelector);
}

this.container.querySelector(Selectors.inputFieldSelector).addEventListener('change', (): void => {
const explanationShown = !this.explanationField.classList.contains('hidden');
if (explanationShown) {
this.toggleVisibility(explanationShown);
private get icon(): HTMLSpanElement {
return this.container.querySelector<HTMLSpanElement>(Selectors.iconSelector);
}

private handleClick(e: Event): void {
const initiator = e.target as Element;
const isToggleButton = initiator.closest(Selectors.toggleSelector) !== null;
if (isToggleButton) {
e.preventDefault();
const explanationHidden = this.explanationField.hasAttribute('hidden');
if (explanationHidden) {
this.showExplanation();
} else {
this.hideExplanation();
}
}
}

private handleChange(e: Event): void {
const initiator = e.target as Element;
const isInputField = initiator.closest(Selectors.inputFieldSelector) !== null;
if (isInputField) {
const explanationVisible = !this.explanationField.hasAttribute('hidden');
if (explanationVisible) {
this.hideExplanation();
}
this.disableToggle();
this.clearIcon();
});
}
}

private showExplanation(): void {
this.explanationField.removeAttribute('hidden');
this.element.setAttribute('hidden', '');
}

private hideExplanation(): void {
this.explanationField.setAttribute('hidden', '');
this.element.removeAttribute('hidden');
}

private disableToggle(): void {
this.toggleSelector.classList.add('disabled');
this.toggleSelector.setAttribute('disabled', 'disabled');
this.toggleSelector.setAttribute('disabled', '');
}

private clearIcon(): void {
this.icon.innerHTML = '';
this.icon.replaceChildren();
}
}

Expand Down
9 changes: 6 additions & 3 deletions typo3/sysext/backend/Classes/Form/Element/LinkElement.php
Expand Up @@ -143,7 +143,6 @@ public function render(): array
'form-control-clearable',
't3js-clearable',
't3js-form-field-link-input',
'hidden',
'hasDefaultValue',
]),
'data-formengine-validation-rules' => $this->getValidationDataAsJsonString($config),
Expand Down Expand Up @@ -198,6 +197,10 @@ public function render(): array
$resultArray = $this->mergeChildReturnIntoExistingResult($resultArray, $fieldControlResult, false);

$linkExplanation = $this->getLinkExplanation((string)$itemValue);
$hasExplanation = ($linkExplanation['text'] ?? '') !== '';
if ($hasExplanation) {
$attributes['hidden'] = 'hidden';
}
$explanation = htmlspecialchars($linkExplanation['text'] ?? '');
$toggleButtonTitle = $languageService->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:buttons.toggleLinkExplanation');

Expand All @@ -207,9 +210,9 @@ public function render(): array
$expansionHtml[] = '<div class="form-wizards-element">';
$expansionHtml[] = '<div class="input-group t3js-form-field-link">';
$expansionHtml[] = '<span class="t3js-form-field-link-icon input-group-addon">' . ($linkExplanation['icon'] ?? '') . '</span>';
$expansionHtml[] = '<input class="form-control t3js-form-field-link-explanation" title="' . $explanation . '" value="' . $explanation . '" readonly>';
$expansionHtml[] = '<input class="form-control t3js-form-field-link-explanation" title="' . $explanation . '" value="' . $explanation . '"' . ' readonly' . ($hasExplanation ? '' : ' hidden') . '>';
$expansionHtml[] = '<input type="text" ' . GeneralUtility::implodeAttributes($attributes, true) . ' />';
$expansionHtml[] = '<button class="btn btn-default t3js-form-field-link-explanation-toggle" type="button" title="' . htmlspecialchars($toggleButtonTitle) . '">';
$expansionHtml[] = '<button class="btn btn-default t3js-form-field-link-explanation-toggle" type="button" title="' . htmlspecialchars($toggleButtonTitle) . '"' . ($hasExplanation ? '' : ' disabled') . '>';
$expansionHtml[] = $this->iconFactory->getIcon('actions-version-workspaces-preview-link', Icon::SIZE_SMALL)->render();
$expansionHtml[] = '</button>';
$expansionHtml[] = '<input type="hidden" name="' . $itemName . '" value="' . htmlspecialchars((string)$itemValue) . '" />';
Expand Down
4 changes: 2 additions & 2 deletions typo3/sysext/backend/Resources/Public/Css/backend.css
Expand Up @@ -4296,8 +4296,8 @@ select.icon-select option{padding-inline-start:22px}
.form-multigroup-wrap .form-wizards-wrap{width:100%}
textarea.formengine-textarea{resize:none}
.input-group>.form-control-clearable-wrapper{flex:1 1 auto;width:1%;min-width:0}
.form-control:not(.hidden)+.form-control-clearable-wrapper{flex-grow:0;width:auto}
.form-control.hidden+.close{display:none}
.form-control:not(.hidden):not([hidden])+.form-control-clearable-wrapper{flex-grow:0;width:auto}
.form-control.hidden+.close,.form-control[hidden]+.close{display:none}
.sticky-form-actions{position:sticky;top:0;z-index:2;padding:calc(1rem / 2) 1rem;background:#fff}
.modal-body .sticky-form-actions{margin-inline:calc(var(--bs-modal-padding) * -1);margin-bottom:var(--bs-modal-padding);padding:calc(var(--bs-modal-padding)/ 2) var(--bs-modal-padding);border-bottom:var(--bs-modal-border-width) solid var(--bs-modal-border-color)}
.modal-body .sticky-form-actions:first-child{transform:translateY(calc(var(--bs-modal-padding) * -1));margin-bottom:0}
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 1f5e344

Please sign in to comment.