Skip to content

Remove the GHPR uri opening workaround in favor of external URI opener #334222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Alex Ross (alexr00) merged 2 commits into from
Sep 3, 2026
Merged

Remove the GHPR uri opening workaround in favor of external URI opener #334222

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
35651d9
Remove the GHPR uri opening workaround in favor of external URI opener
alexr00 Sep 3, 2026
11501f7
CCR feedback
alexr00 Sep 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 74 additions & 24 deletions src/vs/editor/browser/services/openerService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,13 @@ import { URI } from '../../../base/common/uri.js';
import { ICodeEditorService } from './codeEditorService.js';
import { ICommandService } from '../../../platform/commands/common/commands.js';
import { EditorOpenSource } from '../../../platform/editor/common/editor.js';
import { extractSelection, IExternalOpener, IExternalUriResolver, IOpener, IOpenerService, IResolvedExternalUri, IValidator, OpenOptions, ResolveExternalUriOptions } from '../../../platform/opener/common/opener.js';
import { defaultExternalUriOpenerId, extractSelection, IExternalOpener, IExternalUriResolver, IOpener, IOpenerService, IResolvedExternalUri, IValidator, OpenOptions, ResolveExternalUriOptions } from '../../../platform/opener/common/opener.js';

interface IExternalUriOpenTarget {
readonly sourceUri: URI;
readonly href: string;
readonly validationTarget: URI | string;
}

class CommandOpener implements IOpener {

Expand Down Expand Up @@ -98,6 +104,16 @@ class EditorOpener implements IOpener {
}
}

function shouldOpenExternal(target: URI | string, options: OpenOptions | undefined): boolean {
return !!options?.openExternal || matchesSomeScheme(target, Schemas.mailto, Schemas.http, Schemas.https, Schemas.vsls);
}

function shouldUseContributedExternalOpeners(target: URI | string, options: OpenOptions | undefined): boolean {
return !!options?.allowContributedOpeners
&& options.allowContributedOpeners !== defaultExternalUriOpenerId
&& shouldOpenExternal(target, options);
}

export class OpenerService implements IOpenerService {

declare readonly _serviceBrand: undefined;
Expand All @@ -109,6 +125,7 @@ export class OpenerService implements IOpenerService {

private _defaultExternalOpener: IExternalOpener;
private readonly _externalOpeners = new LinkedList<IExternalOpener>();
private readonly _externalResourceOpener: IOpener;

constructor(
@ICodeEditorService editorService: ICodeEditorService,
Expand All @@ -131,16 +148,17 @@ export class OpenerService implements IOpenerService {
};

// Default opener: any external, maito, http(s), command, and catch-all-editors
this._openers.push({
this._externalResourceOpener = {
open: async (target: URI | string, options?: OpenOptions) => {
if (options?.openExternal || matchesSomeScheme(target, Schemas.mailto, Schemas.http, Schemas.https, Schemas.vsls)) {
if (shouldOpenExternal(target, options)) {
// open externally
await this._doOpenExternal(target, options);
return true;
}
return false;
}
});
};
this._openers.push(this._externalResourceOpener);
this._openers.push(new CommandOpener(commandService));
this._openers.push(new EditorOpener(editorService));
}
Expand Down Expand Up @@ -177,18 +195,26 @@ export class OpenerService implements IOpenerService {
return false;
}

// check with contributed validators
let externalUriOpenTarget: IExternalUriOpenTarget | undefined;
if (shouldUseContributedExternalOpeners(target, options)) {
externalUriOpenTarget = await this._resolveExternalUriOpenTarget(target, options);
if (await this._openWithContributedExternalOpeners(externalUriOpenTarget, options)) {
return true;
}
}

if (!options?.skipValidation) {
const validationTarget = this._resolvedUriTargets.get(targetURI) ?? target; // validate against the original URI that this URI resolves to, if one exists
for (const validator of this._validators) {
if (!(await validator.shouldOpen(validationTarget, options))) {
return false;
}
const validationTarget = externalUriOpenTarget?.validationTarget ?? this._resolvedUriTargets.get(targetURI) ?? target;
if (!(await this._validate(validationTarget, options))) {
return false;
}
}

// check with contributed openers
for (const opener of this._openers) {
if (externalUriOpenTarget && opener === this._externalResourceOpener) {
return this._openDefaultExternal(externalUriOpenTarget);
}
const handled = await opener.open(target, options);
if (handled) {
return true;
Expand Down Expand Up @@ -216,8 +242,7 @@ export class OpenerService implements IOpenerService {
throw new Error('Could not resolve external URI: ' + resource.toString());
}

private async _doOpenExternal(resource: URI | string, options: OpenOptions | undefined): Promise<boolean> {

private async _resolveExternalUriOpenTarget(resource: URI | string, options: OpenOptions | undefined): Promise<IExternalUriOpenTarget> {
//todo@jrieken IExternalUriResolver should support `uri: URI | string`
const uri = typeof resource === 'string' ? URI.parse(resource) : resource;
let externalUri: URI;
Expand All @@ -228,29 +253,54 @@ export class OpenerService implements IOpenerService {
externalUri = uri;
}

const preserveOriginalString = typeof resource === 'string' && uri.toString() === externalUri.toString();
let href: string;
if (typeof resource === 'string' && uri.toString() === externalUri.toString()) {
if (preserveOriginalString) {
// open the url-string AS IS
href = resource;
} else {
// open URI using the toString(noEncode)+encodeURI-trick
href = encodeURI(externalUri.toString(true));
}

if (options?.allowContributedOpeners) {
const preferredOpenerId = typeof options?.allowContributedOpeners === 'string' ? options?.allowContributedOpeners : undefined;
for (const opener of this._externalOpeners) {
const didOpen = await opener.openExternal(href, {
sourceUri: uri,
preferredOpenerId,
}, CancellationToken.None);
if (didOpen) {
return true;
}
return {
sourceUri: uri,
href,
validationTarget: preserveOriginalString ? resource : externalUri,
};
}

private async _openWithContributedExternalOpeners(target: IExternalUriOpenTarget, options: OpenOptions | undefined): Promise<boolean> {
const preferredOpenerId = typeof options?.allowContributedOpeners === 'string' ? options.allowContributedOpeners : undefined;
for (const opener of this._externalOpeners) {
const didOpen = await opener.openExternal(target.href, {
sourceUri: target.sourceUri,
preferredOpenerId,
}, CancellationToken.None);
if (didOpen) {
return true;
}
}

return this._defaultExternalOpener.openExternal(href, { sourceUri: uri }, CancellationToken.None);
return false;
}

private _openDefaultExternal(target: IExternalUriOpenTarget): Promise<boolean> {
return this._defaultExternalOpener.openExternal(target.href, { sourceUri: target.sourceUri }, CancellationToken.None);
}

private async _doOpenExternal(resource: URI | string, options: OpenOptions | undefined): Promise<boolean> {
const target = await this._resolveExternalUriOpenTarget(resource, options);
return this._openDefaultExternal(target);
}

private async _validate(resource: URI | string, options: OpenOptions | undefined): Promise<boolean> {
for (const validator of this._validators) {
if (!(await validator.shouldOpen(resource, options))) {
return false;
}
}
return true;
}

dispose() {
Expand Down
Loading
Loading