Trusted domain dialog in web doesn't return focus to previously clicked element when canceled.

[alexr00]

This can be trivially reproduced with the GitHub Pull Requests and Issues extension:

1. Open a pull request on insiders.vscode.dev
2. In the pull request description, leave a comment with a link to an external domain.
3. If you've trusted the workspace, set "workbench.trustedDomains.promptInTrustedWorkspace": true
4. Tab to the link that you made in step 2.
5. Enter.
6. In the resulting trusted domain dialog, tab to cancel and cancel.
7. 🐛 Focus is not restored (this works property on desktop with the native dialog).

[alexr00]

Discovered by the accessibility testing team in microsoft/vscode-pull-request-github#4242

[fsteffi]

GitHubTags:#A11ySev3;#A11yTCS;#A11yMAS;#DesktopWeb;#Visual Studio Code Client;#WCAG2.4.3;#BM-VisualStudioCode-Win32-Nov2021;

primary bug:microsoft/vscode-pull-request-github#4242

[sbatten]

@alexr00 I'm not sure what I can do on my side. The custom dialog takes the active element before showing and restores it after the dialog is dismissed. In this case, the active element is the iframe for the webview. I think the webview content provider may need to hand focus coming back to the iframe and decide if it should restore focus to the previously active element.

[alexr00]

@sbatten I assumed that it was fixable by the custom dialog since the issue doesn't happen with the native dialog (and since it is something that will affect all extensions with webviews). Does this need to be fixed by individual extensions?

[sbatten]

The native dialog doesn't have this issue because it doesn't truly steal focus, its a separate window. In custom dialogs we try our best to restore focus to mimic the behaviour of a native dialog but since the focused element is inside a webview, we need a webview specific solution. After chatting with @mjbvz we propose that the PR extension handle this when the iframe is refocused. I'm not sure if the PR extension keeps some internal focus state to handle tab navigation. However, if there is a general webview solution, we can pull that up. At the dialog level it is impossible for me to properly restore focus to a specific element inside the iframe unfortunately.

[isidorn]

Thanks for starting this discussion, and a friendly reminder that we have to tackle this in April for compliance reasons.

@scottaohara what do you think? Somewhat similar to our previous webview focus discussion? If the custom dialog returns the focus to the WebView (outer element) would that be enough to reduce severity to 3? Otherwise VS Code can not pass focus back to the exact element as @sbatten explained above.

[sbatten]

We are already passing focus back to the outer webview at the dialog layer.

[isidorn]

Thanks. Let's see what @scottaohara says.

[scottaohara]

i imagine that'll be the best you can do, assuming these would be cross-origin iframes.... moving focus to the iframe of the webview and then letting the extension teams reset focus back to the last focusable element once focus is re-detected back in the iframe. based on the linked issue, as long as that is a common representation of this issue, it does seem like it'd be fine as a sev3, so long as focus was moved back to the iframe/iframe's container, and not to the browser chrome.

[alexr00]

I've verified that focus does return to the iframe, but sometimes tabbing from the iframe takes me to the browser chrome. It actually appears to exactly alternate:

1. Try to repro
2. See that focus is on the iframe
3. Tab
4. Focus is on browser chrome
5. Try to repro
6. See that focus is on the iframe
7. Tab
8. Focus is inside the webview.

@mjbvz do you know why tabbing from the outer iframe might do this?

[sbatten]

@mjbvz do you have any idea why that might be the case?

[sbatten]

@isidorn @scottaohara how do we get this bumped down to sev3 as discussed?

[scottaohara]

@fsteffi should be able to help here.

[fsteffi]

hi @sbatten severity has been reduced

[mjbvz]

@sbatten @alexr00 Possibly because the webview is structurally and the end of the dom (outside of the normal workbench dom). We need to do this so that the webview can be moved around without being recreated and destroyed. Not sure why it only sometimes happens though