Open sponsor link should ask user if they want to open an external URL

[isidorn]

Testing #150748

1. code-insiders with new user data dir
2. Search for my extension isidor-tryout https://marketplace.visualstudio.com/items?itemName=inikolic.isidor-tryout
3. Click on the sponsor link -> notice how the patreon link just opens

I would have expected that VS Code would ask me if I am sure I want to open an external link.
I think we should just re-use the service that @rzhao271 introduced

[sandy081]

I use IOpenerService to open the URL and I expect that it asks for confirmation

vscode/src/vs/workbench/contrib/extensions/browser/extensionsActions.ts

Line 908 in 0e131b5

return this.openerService.open(this.extension.publisherSponsorLink);

But, it seems the trusted domains validator is not showing the dialog

Assigning to @sbatten because I assume the workspace trust change might have broken this

vscode/src/vs/workbench/contrib/url/browser/trustedDomainsValidator.ts

Lines 72 to 74 in 757b708

	if (this._workspaceTrustService.isWorkspaceTrusted() && !this._configurationService.getValue('workbench.trustedDomains.promptInTrustedWorkspace')) {
	return true;
	}

[sbatten]

This was introduced via #124325 by @JacksonKearl. The logic is that the workspace is trusted and therefore we don't need to prompt, but I'm not sure we considered extension provided links.

[isidorn]

Yes, that assumption might be flawed.
Adding an option to the openerService sounds like a potential path forward, however then it is up to the implementor to know if a link is extension provided which is error prone.
This is a special case because the extension can even be not installed. If the extension is installed we have bigger problems...

@sbatten what do you suggest we do here?

[sbatten]

an option to the opener service was what I was originally thinking. I would expect it to be opt-in (i.e. you have to say you want to rely on workspace trust) otherwise by default, the link prompts. This would likely lead to a few cases where you are prompted erroneously, but its safer than the alternative. I'm not sure if the option makes sense on the opener service though, so I defer to that knowledge.

[sandy081]

Good idea. I like that by default we show the dialog if the domain is not trusted. One can override that if they depend on workspace trust.

[sandy081]

Is it possible to get this into this milestone?

[isidorn]

This idea works for me.
+1 if possible for this milestone since I see Jackson's change is from this milestone and I worry this might cause some msrc cases to get opened potentially.

[sbatten]

Jackon's change was actually introduced a year ago (May '21). Should this change come from @bpasero @lramos15 ?

[isidorn]

Oh wrong May haha
Then it might be fine to do this in debt week. I leave it up to you to decide :)

[bpasero]

I suggest that this change is done by someone on the workspace trust team because the motivation for the change in the first place comes from you and you know best how you want to treat trust and link opening I would say. I can review the change though.

[sbatten]

I see 3 parts here:

1. The code in the opener service that correlates workspace trust and trusted domains
2. The opener service options bag that tells us where it should be correlated
3. The adoption of others to opt into this behavior

I can do 2 and update 1 if that's a reasonable property on the openerservice api

[sbatten]

sorry I didn't get to this in June, will tackle next week