fix(picker): restore overlay declarative rendering to avoid VO crash on Safari (#5983)

Author: rubencarvalho

.changeset/fix-safari-voiceover-crash.md

@@ -0,0 +1,6 @@
+---
+'@spectrum-web-components/picker': patch
+'@spectrum-web-components/action-menu': patch
+---
+
+**Fixed**: Safari + VoiceOver crash when opening Picker and ActionMenu. The issue was caused by an imperative `render()` call that mutated the DOM during the render cycle, causing Safari to crash while VoiceOver scanned the accessibility tree.

1st-gen/packages/picker/src/InteractionController.ts

@@ -50,24 +50,11 @@ export class InteractionController implements ReactiveController {
         if (this._open === open) return;
         this._open = open;
 
-        if (this.overlay) {
-            this.host.open = open;
-            return;
-        }
+        this.host.open = open;
 
-        // When there is no Overlay and `open` is moving to `true`, lazily import/create
-        // an Overlay and apply that state to it.
-        customElements
-            .whenDefined('sp-overlay')
-            .then(async (): Promise<void> => {
-                const { Overlay } = await import(
-                    '@spectrum-web-components/overlay/src/Overlay.js'
-                );
-                this.overlay = new Overlay();
-                this.host.open = true;
-                this.host.requestUpdate();
-            });
-        import('@spectrum-web-components/overlay/sp-overlay.js');
+        if (!this.overlay && this.host.overlayElement) {
+            this.overlay = this.host.overlayElement;
+        }
     }
 
     private _overlay!: AbstractOverlay;
@@ -110,8 +97,6 @@ export class InteractionController implements ReactiveController {
             if (this.preventNextToggle === 'no') {
                 this.open = false;
             } else if (!this.pointerdownState) {
-                // Prevent browser driven closure while opening the Picker
-                // and the expected event series has not completed.
                 this.overlay?.manuallyKeepOpen();
             }
         }
@@ -195,13 +180,12 @@ export class InteractionController implements ReactiveController {
     }
 
     public hostUpdated(): void {
-        if (
-            this.overlay &&
-            this.host.dependencyManager.loaded &&
-            this.host.open !== this.overlay.open
-        ) {
+        if (!this.overlay && this.host.overlayElement) {
+            this.overlay = this.host.overlayElement;
+        }
+
+        if (this.overlay && this.host.dependencyManager.loaded) {
             this.overlay.willPreventClose = this.preventNextToggle !== 'no';
-            this.overlay.open = this.host.open;
         }
     }
 }

1st-gen/packages/picker/src/Picker.ts

@@ -16,7 +16,6 @@ import {
     html,
     nothing,
     PropertyValues,
-    render,
     SizedMixin,
     SpectrumElement,
     TemplateResult,
@@ -438,6 +437,30 @@ export class PickerBase extends SizedMixin(SpectrumElement, {
 
     public handleSlottableRequest = (_event: SlottableRequestEvent): void => {};
 
+    protected handleBeforetoggle = (
+        event: Event & {
+            target: Overlay;
+            newState: 'open' | 'closed';
+        }
+    ): void => {
+        if (event.composedPath()[0] !== event.target) {
+            return;
+        }
+        if (event.newState === 'closed') {
+            if (this.strategy?.preventNextToggle === 'no') {
+                this.open = false;
+            } else if (!this.strategy?.pointerdownState) {
+                // Prevent browser driven closure while opening the Picker
+                // and the expected event series has not completed.
+                this.overlayElement?.manuallyKeepOpen();
+            }
+        }
+        if (!this.open) {
+            this.optionsMenu.updateSelectedItemIndex();
+            this.optionsMenu.closeDescendentOverlays();
+        }
+    };
+
     protected renderLabelContent(content: Node[]): TemplateResult | Node[] {
         if (this.value && this.selectedItem) {
             return content;
@@ -577,14 +600,30 @@ export class PickerBase extends SizedMixin(SpectrumElement, {
     }
 
     protected renderOverlay(menu: TemplateResult): TemplateResult {
-        if (this.strategy?.overlay === undefined) {
-            return menu;
-        }
         const container = this.renderContainer(menu);
-        render(container, this.strategy?.overlay as unknown as HTMLElement, {
-            host: this,
-        });
-        return this.strategy?.overlay as unknown as TemplateResult;
+        this.dependencyManager.add('sp-overlay');
+        import('@spectrum-web-components/overlay/sp-overlay.js');
+        return html`
+            <sp-overlay
+                @slottable-request=${this.handleSlottableRequest}
+                @beforetoggle=${this.handleBeforetoggle}
+                .triggerElement=${this as HTMLElement}
+                .offset=${0}
+                ?open=${this.open && this.dependencyManager.loaded}
+                .placement=${this.isMobile.matches && !this.forcePopover
+                    ? undefined
+                    : this.placement}
+                .type=${this.isMobile.matches && !this.forcePopover
+                    ? 'modal'
+                    : 'auto'}
+                .receivesFocus=${'false'}
+                .willPreventClose=${this.strategy?.preventNextToggle !== 'no' &&
+                this.open &&
+                this.dependencyManager.loaded}
+            >
+                ${container}
+            </sp-overlay>
+        `;
     }
 
     protected get renderDescriptionSlot(): TemplateResult {
@@ -662,7 +701,7 @@ export class PickerBase extends SizedMixin(SpectrumElement, {
             // await the same here.
             this.shouldScheduleManageSelection();
         }
-        // Maybe it's finally time to remove this support?s
+        // Maybe it's finally time to remove this support?
         if (!this.hasUpdated) {
             this.deprecatedMenu = this.querySelector(':scope > sp-menu');
             this.deprecatedMenu?.toggleAttribute('ignore', true);
@@ -697,15 +736,26 @@ export class PickerBase extends SizedMixin(SpectrumElement, {
 
     protected override updated(changes: PropertyValues<this>): void {
         super.updated(changes);
-        if (changes.has('open')) {
-            this.strategy.open = this.open;
+        if (
+            changes.has('open') &&
+            this.overlayElement &&
+            !this.strategy.overlay
+        ) {
+            this.strategy.overlay = this.overlayElement;
         }
     }
 
-    protected override firstUpdated(changes: PropertyValues<this>): void {
+    protected override async firstUpdated(
+        changes: PropertyValues<this>
+    ): Promise<void> {
         super.firstUpdated(changes);
         this.bindButtonKeydownListener();
         this.bindEvents();
+
+        await this.updateComplete;
+        if (this.overlayElement && !this.strategy.overlay) {
+            this.strategy.overlay = this.overlayElement;
+        }
     }
 
     protected get dismissHelper(): TemplateResult {
@@ -815,7 +865,6 @@ export class PickerBase extends SizedMixin(SpectrumElement, {
                 ((event.target as HTMLElement).getRootNode() as ShadowRoot)
                     .host === this)
         ) {
-            //s set a flag to manage selection on the next frame
             this.willManageSelection = true;
             requestAnimationFrame(() => {
                 requestAnimationFrame(() => {
@@ -881,9 +930,6 @@ export class PickerBase extends SizedMixin(SpectrumElement, {
     protected override async getUpdateComplete(): Promise<boolean> {
         const complete = (await super.getUpdateComplete()) as boolean;
         await this.selectionPromise;
-        // if (this.overlayElement) {
-        //     await this.overlayElement.updateComplete;
-        // }
         return complete;
     }