From e24fefe0a89205ce519efd8bd4ee53bde4bfe90c Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Thu, 14 Nov 2019 09:34:30 +0100 Subject: [PATCH 01/15] add URI.from(url)-overload --- src/vs/base/common/uri.ts | 9 ++++++++- src/vs/monaco.d.ts | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/vs/base/common/uri.ts b/src/vs/base/common/uri.ts index f6a8959a04c..29cd09a479e 100644 --- a/src/vs/base/common/uri.ts +++ b/src/vs/base/common/uri.ts @@ -323,7 +323,14 @@ export class URI implements UriComponents { return new _URI('file', authority, path, _empty, _empty); } - static from(components: { scheme: string; authority?: string; path?: string; query?: string; fragment?: string }): URI { + static from(components: { scheme: string; authority?: string; path?: string; query?: string; fragment?: string } | URL): URI { + + if (components instanceof URL) { + const value = <_URI>_URI.parse(components.href); + value._formatted = components.href; + return value; + } + return new _URI( components.scheme, components.authority, diff --git a/src/vs/monaco.d.ts b/src/vs/monaco.d.ts index 2fa74f0ac35..ff535f4bfbc 100644 --- a/src/vs/monaco.d.ts +++ b/src/vs/monaco.d.ts @@ -160,7 +160,7 @@ declare namespace monaco { path?: string; query?: string; fragment?: string; - }): Uri; + } | URL): Uri; /** * Creates a string representation for this Uri. It's guaranteed that calling * `Uri.parse` with the result of this function creates an Uri which is equal From 9106843dc2f3a6d419c19c191e16a1da80299700 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Thu, 14 Nov 2019 10:08:03 +0100 Subject: [PATCH 02/15] IOpener must accept URI and URL --- .../editor/browser/services/openerService.ts | 174 ++++++++++-------- .../browser/services/openerService.test.ts | 26 +-- src/vs/platform/opener/common/opener.ts | 3 +- .../url/electron-browser/urlService.ts | 11 +- 4 files changed, 124 insertions(+), 90 deletions(-) diff --git a/src/vs/editor/browser/services/openerService.ts b/src/vs/editor/browser/services/openerService.ts index 48175adfb42..ff7c67ebbbb 100644 --- a/src/vs/editor/browser/services/openerService.ts +++ b/src/vs/editor/browser/services/openerService.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as dom from 'vs/base/browser/dom'; -import { Disposable, IDisposable } from 'vs/base/common/lifecycle'; +import { IDisposable } from 'vs/base/common/lifecycle'; import { LinkedList } from 'vs/base/common/linkedList'; import { parse } from 'vs/base/common/marshalling'; import { Schemas } from 'vs/base/common/network'; @@ -16,46 +16,123 @@ import { CommandsRegistry, ICommandService } from 'vs/platform/commands/common/c import { IOpener, IOpenerService, IValidator, IExternalUriResolver, OpenOptions, ResolveExternalUriOptions, IResolvedExternalUri, IExternalOpener } from 'vs/platform/opener/common/opener'; import { EditorOpenContext } from 'vs/platform/editor/common/editor'; -export class OpenerService extends Disposable implements IOpenerService { +function hasScheme(target: URI | URL, scheme: string) { + if (URI.isUri(target)) { + return equalsIgnoreCase(target.scheme, scheme); + } else { + return equalsIgnoreCase(target.protocol, scheme + ':'); + } +} + +export class OpenerService implements IOpenerService { _serviceBrand: undefined; private readonly _openers = new LinkedList(); private readonly _validators = new LinkedList(); private readonly _resolvers = new LinkedList(); + private _externalOpener: IExternalOpener; + private _openerAsExternal: IOpener; + private _openerAsCommand: IOpener; + private _openerAsEditor: IOpener; constructor( - @ICodeEditorService private readonly _editorService: ICodeEditorService, - @ICommandService private readonly _commandService: ICommandService, + @ICodeEditorService editorService: ICodeEditorService, + @ICommandService commandService: ICommandService, ) { - super(); - // Default external opener is going through window.open() this._externalOpener = { openExternal: href => { dom.windowOpenNoOpener(href); - return Promise.resolve(true); } }; + + // Default opener: maito, http(s), command, and catch-all-editors + this._openerAsExternal = { + open: async (target: URI | URL, options?: OpenOptions) => { + if (options?.openExternal || hasScheme(target, Schemas.mailto) || hasScheme(target, Schemas.http) || hasScheme(target, Schemas.https)) { + // open externally + await this._doOpenExternal(target, options); + return true; + } + return false; + } + }; + + this._openerAsCommand = { + open: async (target) => { + if (!hasScheme(target, Schemas.command)) { + return false; + } + // run command or bail out if command isn't known + if (!URI.isUri(target)) { + target = URI.from(target); + } + if (!CommandsRegistry.getCommand(target.path)) { + throw new Error(`command '${target.path}' NOT known`); + } + // execute as command + let args: any = []; + try { + args = parse(target.query); + if (!Array.isArray(args)) { + args = [args]; + } + } catch (e) { + // ignore error + } + await commandService.executeCommand(target.path, ...args); + return true; + } + }; + + this._openerAsEditor = { + open: async (target, options: OpenOptions) => { + if (!URI.isUri(target)) { + target = URI.from(target); + } + let selection: { startLineNumber: number; startColumn: number; } | undefined = undefined; + const match = /^L?(\d+)(?:,(\d+))?/.exec(target.fragment); + if (match) { + // support file:///some/file.js#73,84 + // support file:///some/file.js#L73 + selection = { + startLineNumber: parseInt(match[1]), + startColumn: match[2] ? parseInt(match[2]) : 1 + }; + // remove fragment + target = target.with({ fragment: '' }); + } + + if (target.scheme === Schemas.file) { + target = resources.normalizePath(target); // workaround for non-normalized paths (https://github.com/Microsoft/vscode/issues/12954) + } + + await editorService.openCodeEditor( + { resource: target, options: { selection, context: options?.fromUserGesture ? EditorOpenContext.USER : EditorOpenContext.API } }, + editorService.getFocusedCodeEditor(), + options?.openToSide + ); + + return true; + } + }; } registerOpener(opener: IOpener): IDisposable { const remove = this._openers.push(opener); - return { dispose: remove }; } registerValidator(validator: IValidator): IDisposable { const remove = this._validators.push(validator); - return { dispose: remove }; } registerExternalUriResolver(resolver: IExternalUriResolver): IDisposable { const remove = this._resolvers.push(resolver); - return { dispose: remove }; } @@ -86,7 +163,12 @@ export class OpenerService extends Disposable implements IOpenerService { } // use default openers - return this._doOpen(resource, options); + for (const opener of [this._openerAsExternal, this._openerAsCommand, this._openerAsEditor]) { + if (await opener.open(resource, options)) { + break; + } + } + return true; } async resolveExternalUri(resource: URI, options?: ResolveExternalUriOptions): Promise { @@ -100,68 +182,16 @@ export class OpenerService extends Disposable implements IOpenerService { return { resolved: resource, dispose: () => { } }; } - private async _doOpen(resource: URI, options: OpenOptions | undefined): Promise { - const { scheme, path, query, fragment } = resource; - - if (options?.openExternal || equalsIgnoreCase(scheme, Schemas.mailto) || equalsIgnoreCase(scheme, Schemas.http) || equalsIgnoreCase(scheme, Schemas.https)) { - // open externally - return this._doOpenExternal(resource, options); + private async _doOpenExternal(resource: URI | URL, options: OpenOptions | undefined): Promise { + if (URI.isUri(resource)) { + const { resolved } = await this.resolveExternalUri(resource, options); + // TODO@Jo neither encodeURI nor toString(true) should be needed + // once we go with URL and not URI + return this._externalOpener.openExternal(encodeURI(resolved.toString(true))); + } else { + //todo@joh what about resolveExternalUri? + return this._externalOpener.openExternal(resource.href); } - - if (equalsIgnoreCase(scheme, Schemas.command)) { - // run command or bail out if command isn't known - if (!CommandsRegistry.getCommand(path)) { - throw new Error(`command '${path}' NOT known`); - } - // execute as command - let args: any = []; - try { - args = parse(query); - if (!Array.isArray(args)) { - args = [args]; - } - } catch (e) { - // ignore error - } - - await this._commandService.executeCommand(path, ...args); - - return true; - } - - // finally open in editor - let selection: { startLineNumber: number; startColumn: number; } | undefined = undefined; - const match = /^L?(\d+)(?:,(\d+))?/.exec(fragment); - if (match) { - // support file:///some/file.js#73,84 - // support file:///some/file.js#L73 - selection = { - startLineNumber: parseInt(match[1]), - startColumn: match[2] ? parseInt(match[2]) : 1 - }; - // remove fragment - resource = resource.with({ fragment: '' }); - } - - if (resource.scheme === Schemas.file) { - resource = resources.normalizePath(resource); // workaround for non-normalized paths (https://github.com/Microsoft/vscode/issues/12954) - } - - await this._editorService.openCodeEditor( - { resource, options: { selection, context: options?.fromUserGesture ? EditorOpenContext.USER : EditorOpenContext.API } }, - this._editorService.getFocusedCodeEditor(), - options?.openToSide - ); - - return true; - } - - private async _doOpenExternal(resource: URI, options: OpenOptions | undefined): Promise { - const { resolved } = await this.resolveExternalUri(resource, options); - - // TODO@Jo neither encodeURI nor toString(true) should be needed - // once we go with URL and not URI - return this._externalOpener.openExternal(encodeURI(resolved.toString(true))); } dispose() { diff --git a/src/vs/editor/test/browser/services/openerService.test.ts b/src/vs/editor/test/browser/services/openerService.test.ts index 6f03b911e37..4081b6fc5f8 100644 --- a/src/vs/editor/test/browser/services/openerService.test.ts +++ b/src/vs/editor/test/browser/services/openerService.test.ts @@ -28,27 +28,27 @@ suite('OpenerService', function () { lastCommand = undefined; }); - test('delegate to editorService, scheme:///fff', function () { + test('delegate to editorService, scheme:///fff', async function () { const openerService = new OpenerService(editorService, NullCommandService); - openerService.open(URI.parse('another:///somepath')); + await openerService.open(URI.parse('another:///somepath')); assert.equal(editorService.lastInput!.options!.selection, undefined); }); - test('delegate to editorService, scheme:///fff#L123', function () { + test('delegate to editorService, scheme:///fff#L123', async function () { const openerService = new OpenerService(editorService, NullCommandService); - openerService.open(URI.parse('file:///somepath#L23')); + await openerService.open(URI.parse('file:///somepath#L23')); assert.equal(editorService.lastInput!.options!.selection!.startLineNumber, 23); assert.equal(editorService.lastInput!.options!.selection!.startColumn, 1); assert.equal(editorService.lastInput!.options!.selection!.endLineNumber, undefined); assert.equal(editorService.lastInput!.options!.selection!.endColumn, undefined); assert.equal(editorService.lastInput!.resource.fragment, ''); - openerService.open(URI.parse('another:///somepath#L23')); + await openerService.open(URI.parse('another:///somepath#L23')); assert.equal(editorService.lastInput!.options!.selection!.startLineNumber, 23); assert.equal(editorService.lastInput!.options!.selection!.startColumn, 1); - openerService.open(URI.parse('another:///somepath#L23,45')); + await openerService.open(URI.parse('another:///somepath#L23,45')); assert.equal(editorService.lastInput!.options!.selection!.startLineNumber, 23); assert.equal(editorService.lastInput!.options!.selection!.startColumn, 45); assert.equal(editorService.lastInput!.options!.selection!.endLineNumber, undefined); @@ -56,17 +56,17 @@ suite('OpenerService', function () { assert.equal(editorService.lastInput!.resource.fragment, ''); }); - test('delegate to editorService, scheme:///fff#123,123', function () { + test('delegate to editorService, scheme:///fff#123,123', async function () { const openerService = new OpenerService(editorService, NullCommandService); - openerService.open(URI.parse('file:///somepath#23')); + await openerService.open(URI.parse('file:///somepath#23')); assert.equal(editorService.lastInput!.options!.selection!.startLineNumber, 23); assert.equal(editorService.lastInput!.options!.selection!.startColumn, 1); assert.equal(editorService.lastInput!.options!.selection!.endLineNumber, undefined); assert.equal(editorService.lastInput!.options!.selection!.endColumn, undefined); assert.equal(editorService.lastInput!.resource.fragment, ''); - openerService.open(URI.parse('file:///somepath#23,45')); + await openerService.open(URI.parse('file:///somepath#23,45')); assert.equal(editorService.lastInput!.options!.selection!.startLineNumber, 23); assert.equal(editorService.lastInput!.options!.selection!.startColumn, 45); assert.equal(editorService.lastInput!.options!.selection!.endLineNumber, undefined); @@ -74,22 +74,22 @@ suite('OpenerService', function () { assert.equal(editorService.lastInput!.resource.fragment, ''); }); - test('delegate to commandsService, command:someid', function () { + test('delegate to commandsService, command:someid', async function () { const openerService = new OpenerService(editorService, commandService); const id = `aCommand${Math.random()}`; CommandsRegistry.registerCommand(id, function () { }); - openerService.open(URI.parse('command:' + id)); + await openerService.open(URI.parse('command:' + id)); assert.equal(lastCommand!.id, id); assert.equal(lastCommand!.args.length, 0); - openerService.open(URI.parse('command:' + id).with({ query: '123' })); + await openerService.open(URI.parse('command:' + id).with({ query: '123' })); assert.equal(lastCommand!.id, id); assert.equal(lastCommand!.args.length, 1); assert.equal(lastCommand!.args[0], '123'); - openerService.open(URI.parse('command:' + id).with({ query: JSON.stringify([12, true]) })); + await openerService.open(URI.parse('command:' + id).with({ query: JSON.stringify([12, true]) })); assert.equal(lastCommand!.id, id); assert.equal(lastCommand!.args.length, 2); assert.equal(lastCommand!.args[0], 12); diff --git a/src/vs/platform/opener/common/opener.ts b/src/vs/platform/opener/common/opener.ts index 3b60521677a..8b9ce48f621 100644 --- a/src/vs/platform/opener/common/opener.ts +++ b/src/vs/platform/opener/common/opener.ts @@ -35,8 +35,7 @@ export interface IResolvedExternalUri extends IDisposable { } export interface IOpener { - open(resource: URI, options?: OpenInternalOptions): Promise; - open(resource: URI, options?: OpenExternalOptions): Promise; + open(resource: URI | URL, options?: OpenInternalOptions | OpenExternalOptions): Promise; } export interface IExternalOpener { diff --git a/src/vs/workbench/services/url/electron-browser/urlService.ts b/src/vs/workbench/services/url/electron-browser/urlService.ts index 73fa15e1f8e..c46c1948c45 100644 --- a/src/vs/workbench/services/url/electron-browser/urlService.ts +++ b/src/vs/workbench/services/url/electron-browser/urlService.ts @@ -8,7 +8,7 @@ import { URI, UriComponents } from 'vs/base/common/uri'; import { IMainProcessService } from 'vs/platform/ipc/electron-browser/mainProcessService'; import { URLHandlerChannel } from 'vs/platform/url/common/urlIpc'; import { URLService } from 'vs/platform/url/node/urlService'; -import { IOpenerService } from 'vs/platform/opener/common/opener'; +import { IOpenerService, IOpener } from 'vs/platform/opener/common/opener'; import product from 'vs/platform/product/common/product'; import { registerSingleton } from 'vs/platform/instantiation/common/extensions'; import { IElectronEnvironmentService } from 'vs/workbench/services/electron/electron-browser/electronEnvironmentService'; @@ -20,7 +20,7 @@ export interface IRelayOpenURLOptions extends IOpenURLOptions { openExternal?: boolean; } -export class RelayURLService extends URLService implements IURLHandler { +export class RelayURLService extends URLService implements IURLHandler, IOpener { private urlService: IURLService; @@ -51,7 +51,12 @@ export class RelayURLService extends URLService implements IURLHandler { return uri.with({ query }); } - async open(resource: URI, options?: IRelayOpenURLOptions): Promise { + async open(resource: URI | URL, options?: IRelayOpenURLOptions): Promise { + + if (resource instanceof URL) { + resource = URI.from(resource); + } + if (resource.scheme !== product.urlProtocol) { return false; } From 3816ecfe24d7835c319d89f2b9aeb9e5836a9f2c Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Thu, 14 Nov 2019 10:29:58 +0100 Subject: [PATCH 03/15] IOpenerService#open accepts URI and URL --- src/vs/editor/browser/services/openerService.ts | 9 ++++++--- src/vs/platform/opener/common/opener.ts | 3 +-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/vs/editor/browser/services/openerService.ts b/src/vs/editor/browser/services/openerService.ts index ff7c67ebbbb..dab6794e8ca 100644 --- a/src/vs/editor/browser/services/openerService.ts +++ b/src/vs/editor/browser/services/openerService.ts @@ -140,13 +140,16 @@ export class OpenerService implements IOpenerService { this._externalOpener = externalOpener; } - async open(resource: URI, options?: OpenOptions): Promise { + async open(target: URI | URL, options?: OpenOptions): Promise { + + const resource = URI.isUri(target) ? target : URI.from(target); // no scheme ?!? if (!resource.scheme) { return Promise.resolve(false); } + //todo@joh adopt validator // check with contributed validators for (const validator of this._validators.toArray()) { if (!(await validator.shouldOpen(resource))) { @@ -156,7 +159,7 @@ export class OpenerService implements IOpenerService { // check with contributed openers for (const opener of this._openers.toArray()) { - const handled = await opener.open(resource, options); + const handled = await opener.open(target, options); if (handled) { return true; } @@ -164,7 +167,7 @@ export class OpenerService implements IOpenerService { // use default openers for (const opener of [this._openerAsExternal, this._openerAsCommand, this._openerAsEditor]) { - if (await opener.open(resource, options)) { + if (await opener.open(target, options)) { break; } } diff --git a/src/vs/platform/opener/common/opener.ts b/src/vs/platform/opener/common/opener.ts index 8b9ce48f621..a0caf1463e2 100644 --- a/src/vs/platform/opener/common/opener.ts +++ b/src/vs/platform/opener/common/opener.ts @@ -82,8 +82,7 @@ export interface IOpenerService { * @param resource A resource * @return A promise that resolves when the opening is done. */ - open(resource: URI, options?: OpenInternalOptions): Promise; - open(resource: URI, options?: OpenExternalOptions): Promise; + open(resource: URI | URL, options?: OpenInternalOptions | OpenExternalOptions): Promise; /** * Resolve a resource to its external form. From d891b0d3493a3180cfdf2ea53f46b41445151bb4 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Thu, 14 Nov 2019 10:52:36 +0100 Subject: [PATCH 04/15] editor link detector uses URL --- src/vs/editor/contrib/links/getLinks.ts | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/vs/editor/contrib/links/getLinks.ts b/src/vs/editor/contrib/links/getLinks.ts index df34eed5600..e7a66f48234 100644 --- a/src/vs/editor/contrib/links/getLinks.ts +++ b/src/vs/editor/contrib/links/getLinks.ts @@ -14,6 +14,18 @@ import { CommandsRegistry } from 'vs/platform/commands/common/commands'; import { isDisposable, Disposable } from 'vs/base/common/lifecycle'; import { coalesce } from 'vs/base/common/arrays'; +// in IE11 there is URL but a constructor +// https://developer.mozilla.org/en-US/docs/Web/API/URL/URL#Browser_compatibility +const canUseUrl = (function () { + try { + // tslint:disable-next-line: no-unused-expression + new URL('some://thing'); + return true; + } catch { + return false; + } +})(); + export class Link implements ILink { private _link: ILink; @@ -44,13 +56,15 @@ export class Link implements ILink { return this._link.tooltip; } - resolve(token: CancellationToken): Promise { + async resolve(token: CancellationToken): Promise { if (this._link.url) { try { - if (typeof this._link.url === 'string') { - return Promise.resolve(URI.parse(this._link.url)); + if (URI.isUri(this._link.url)) { + return this._link.url; + } else if (!canUseUrl) { + return URI.parse(this._link.url); } else { - return Promise.resolve(this._link.url); + return new URL(this._link.url); } } catch (e) { return Promise.reject(new Error('invalid')); From 1603d3e6e52c5e6d93d274e3b322097df1afb1c1 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Thu, 14 Nov 2019 11:03:05 +0100 Subject: [PATCH 05/15] validator must support URI and URL, also extract utils for re-use --- src/vs/base/common/resources.ts | 22 ++++++++++++++++ .../editor/browser/services/openerService.ts | 26 ++++--------------- src/vs/editor/contrib/links/getLinks.ts | 15 ++--------- src/vs/platform/opener/common/opener.ts | 2 +- .../url/common/trustedDomainsValidator.ts | 13 ++++++---- 5 files changed, 38 insertions(+), 40 deletions(-) diff --git a/src/vs/base/common/resources.ts b/src/vs/base/common/resources.ts index bea40bbdc62..6a9dcd13095 100644 --- a/src/vs/base/common/resources.ts +++ b/src/vs/base/common/resources.ts @@ -361,3 +361,25 @@ export function toLocalResource(resource: URI, authority: string | undefined): U return resource.with({ scheme: Schemas.file }); } + +export function matchesScheme(target: URI | URL, scheme: string) { + if (URI.isUri(target)) { + return equalsIgnoreCase(target.scheme, scheme); + } else { + return equalsIgnoreCase(target.protocol, scheme + ':'); + } +} + +/** + * `true` when urls can be constructed via `new URL("string")`, should only be `false` in IE: + * https://developer.mozilla.org/en-US/docs/Web/API/URL/URL#Browser_compatibility + */ +export const hasURLCtor = (function () { + try { + // tslint:disable-next-line: no-unused-expression + new URL('some://thing'); + return true; + } catch { + return false; + } +})(); diff --git a/src/vs/editor/browser/services/openerService.ts b/src/vs/editor/browser/services/openerService.ts index dab6794e8ca..be0bcb1dcf8 100644 --- a/src/vs/editor/browser/services/openerService.ts +++ b/src/vs/editor/browser/services/openerService.ts @@ -8,21 +8,13 @@ import { IDisposable } from 'vs/base/common/lifecycle'; import { LinkedList } from 'vs/base/common/linkedList'; import { parse } from 'vs/base/common/marshalling'; import { Schemas } from 'vs/base/common/network'; -import * as resources from 'vs/base/common/resources'; -import { equalsIgnoreCase } from 'vs/base/common/strings'; +import { matchesScheme, normalizePath } from 'vs/base/common/resources'; import { URI } from 'vs/base/common/uri'; import { ICodeEditorService } from 'vs/editor/browser/services/codeEditorService'; import { CommandsRegistry, ICommandService } from 'vs/platform/commands/common/commands'; import { IOpener, IOpenerService, IValidator, IExternalUriResolver, OpenOptions, ResolveExternalUriOptions, IResolvedExternalUri, IExternalOpener } from 'vs/platform/opener/common/opener'; import { EditorOpenContext } from 'vs/platform/editor/common/editor'; -function hasScheme(target: URI | URL, scheme: string) { - if (URI.isUri(target)) { - return equalsIgnoreCase(target.scheme, scheme); - } else { - return equalsIgnoreCase(target.protocol, scheme + ':'); - } -} export class OpenerService implements IOpenerService { @@ -52,7 +44,7 @@ export class OpenerService implements IOpenerService { // Default opener: maito, http(s), command, and catch-all-editors this._openerAsExternal = { open: async (target: URI | URL, options?: OpenOptions) => { - if (options?.openExternal || hasScheme(target, Schemas.mailto) || hasScheme(target, Schemas.http) || hasScheme(target, Schemas.https)) { + if (options?.openExternal || matchesScheme(target, Schemas.mailto) || matchesScheme(target, Schemas.http) || matchesScheme(target, Schemas.https)) { // open externally await this._doOpenExternal(target, options); return true; @@ -63,7 +55,7 @@ export class OpenerService implements IOpenerService { this._openerAsCommand = { open: async (target) => { - if (!hasScheme(target, Schemas.command)) { + if (!matchesScheme(target, Schemas.command)) { return false; } // run command or bail out if command isn't known @@ -107,7 +99,7 @@ export class OpenerService implements IOpenerService { } if (target.scheme === Schemas.file) { - target = resources.normalizePath(target); // workaround for non-normalized paths (https://github.com/Microsoft/vscode/issues/12954) + target = normalizePath(target); // workaround for non-normalized paths (https://github.com/Microsoft/vscode/issues/12954) } await editorService.openCodeEditor( @@ -142,17 +134,9 @@ export class OpenerService implements IOpenerService { async open(target: URI | URL, options?: OpenOptions): Promise { - const resource = URI.isUri(target) ? target : URI.from(target); - - // no scheme ?!? - if (!resource.scheme) { - return Promise.resolve(false); - } - - //todo@joh adopt validator // check with contributed validators for (const validator of this._validators.toArray()) { - if (!(await validator.shouldOpen(resource))) { + if (!(await validator.shouldOpen(target))) { return false; } } diff --git a/src/vs/editor/contrib/links/getLinks.ts b/src/vs/editor/contrib/links/getLinks.ts index e7a66f48234..081d8b4adde 100644 --- a/src/vs/editor/contrib/links/getLinks.ts +++ b/src/vs/editor/contrib/links/getLinks.ts @@ -13,18 +13,7 @@ import { IModelService } from 'vs/editor/common/services/modelService'; import { CommandsRegistry } from 'vs/platform/commands/common/commands'; import { isDisposable, Disposable } from 'vs/base/common/lifecycle'; import { coalesce } from 'vs/base/common/arrays'; - -// in IE11 there is URL but a constructor -// https://developer.mozilla.org/en-US/docs/Web/API/URL/URL#Browser_compatibility -const canUseUrl = (function () { - try { - // tslint:disable-next-line: no-unused-expression - new URL('some://thing'); - return true; - } catch { - return false; - } -})(); +import { hasURLCtor } from 'vs/base/common/resources'; export class Link implements ILink { @@ -61,7 +50,7 @@ export class Link implements ILink { try { if (URI.isUri(this._link.url)) { return this._link.url; - } else if (!canUseUrl) { + } else if (!hasURLCtor) { return URI.parse(this._link.url); } else { return new URL(this._link.url); diff --git a/src/vs/platform/opener/common/opener.ts b/src/vs/platform/opener/common/opener.ts index a0caf1463e2..5cdaebc93ce 100644 --- a/src/vs/platform/opener/common/opener.ts +++ b/src/vs/platform/opener/common/opener.ts @@ -43,7 +43,7 @@ export interface IExternalOpener { } export interface IValidator { - shouldOpen(resource: URI): Promise; + shouldOpen(resource: URI | URL): Promise; } export interface IExternalUriResolver { diff --git a/src/vs/workbench/contrib/url/common/trustedDomainsValidator.ts b/src/vs/workbench/contrib/url/common/trustedDomainsValidator.ts index f930d68bccf..aabcd4405b2 100644 --- a/src/vs/workbench/contrib/url/common/trustedDomainsValidator.ts +++ b/src/vs/workbench/contrib/url/common/trustedDomainsValidator.ts @@ -5,7 +5,6 @@ import { Schemas } from 'vs/base/common/network'; import Severity from 'vs/base/common/severity'; -import { equalsIgnoreCase } from 'vs/base/common/strings'; import { URI } from 'vs/base/common/uri'; import { localize } from 'vs/nls'; import { IDialogService } from 'vs/platform/dialogs/common/dialogs'; @@ -20,6 +19,7 @@ import { } from 'vs/workbench/contrib/url/common/trustedDomains'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import { IClipboardService } from 'vs/platform/clipboard/common/clipboardService'; +import { matchesScheme } from 'vs/base/common/resources'; export class OpenerValidatorContributions implements IWorkbenchContribution { constructor( @@ -34,13 +34,16 @@ export class OpenerValidatorContributions implements IWorkbenchContribution { this._openerService.registerValidator({ shouldOpen: r => this.validateLink(r) }); } - async validateLink(resource: URI): Promise { - const { scheme, authority, path, query, fragment } = resource; - - if (!equalsIgnoreCase(scheme, Schemas.http) && !equalsIgnoreCase(scheme, Schemas.https)) { + async validateLink(resource: URI | URL): Promise { + if (!matchesScheme(resource, Schemas.http) && !matchesScheme(resource, Schemas.https)) { return true; } + if (!URI.isUri(resource)) { + resource = URI.from(resource); + } + const { scheme, authority, path, query, fragment } = resource; + const domainToOpen = `${scheme}://${authority}`; const { defaultTrustedDomains, trustedDomains } = readTrustedDomains(this._storageService, this._productService); const allTrustedDomains = [...defaultTrustedDomains, ...trustedDomains]; From 65159d001a60c01985666dbdde7b14ba24262399 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Thu, 14 Nov 2019 11:55:52 +0100 Subject: [PATCH 06/15] terminal link handler using URL, #74604 --- .../workbench/contrib/terminal/browser/terminalLinkHandler.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/browser/terminalLinkHandler.ts b/src/vs/workbench/contrib/terminal/browser/terminalLinkHandler.ts index 544cabcde0d..2699cf8ab8a 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalLinkHandler.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalLinkHandler.ts @@ -266,8 +266,8 @@ export class TerminalLinkHandler { } private _handleHypertextLink(url: string): void { - const uri = URI.parse(url); - this._openerService.open(uri, { allowTunneling: !!(this._processManager && this._processManager.remoteAuthority) }); + const urlObj = new URL(url); + this._openerService.open(urlObj, { allowTunneling: !!(this._processManager && this._processManager.remoteAuthority) }); } private _isLinkActivationModifierDown(event: MouseEvent): boolean { From 481485b354b6a32eac31101daaf8cd1cc81787da Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Thu, 14 Nov 2019 15:05:10 +0100 Subject: [PATCH 07/15] use string instead of URL, help with IE11 and with unwanted URL magic --- src/vs/base/common/resources.ts | 22 ------------------- .../editor/browser/services/openerService.ts | 20 ++++++++--------- src/vs/editor/contrib/links/getLinks.ts | 15 ++----------- .../browser/services/openerService.test.ts | 15 +++++++++++++ src/vs/platform/opener/common/opener.ts | 15 ++++++++++--- .../terminal/browser/terminalLinkHandler.ts | 3 +-- .../url/common/trustedDomainsValidator.ts | 10 ++++----- .../url/electron-browser/urlService.ts | 13 +++++------ 8 files changed, 51 insertions(+), 62 deletions(-) diff --git a/src/vs/base/common/resources.ts b/src/vs/base/common/resources.ts index 6a9dcd13095..bea40bbdc62 100644 --- a/src/vs/base/common/resources.ts +++ b/src/vs/base/common/resources.ts @@ -361,25 +361,3 @@ export function toLocalResource(resource: URI, authority: string | undefined): U return resource.with({ scheme: Schemas.file }); } - -export function matchesScheme(target: URI | URL, scheme: string) { - if (URI.isUri(target)) { - return equalsIgnoreCase(target.scheme, scheme); - } else { - return equalsIgnoreCase(target.protocol, scheme + ':'); - } -} - -/** - * `true` when urls can be constructed via `new URL("string")`, should only be `false` in IE: - * https://developer.mozilla.org/en-US/docs/Web/API/URL/URL#Browser_compatibility - */ -export const hasURLCtor = (function () { - try { - // tslint:disable-next-line: no-unused-expression - new URL('some://thing'); - return true; - } catch { - return false; - } -})(); diff --git a/src/vs/editor/browser/services/openerService.ts b/src/vs/editor/browser/services/openerService.ts index be0bcb1dcf8..7e1d50546aa 100644 --- a/src/vs/editor/browser/services/openerService.ts +++ b/src/vs/editor/browser/services/openerService.ts @@ -8,11 +8,11 @@ import { IDisposable } from 'vs/base/common/lifecycle'; import { LinkedList } from 'vs/base/common/linkedList'; import { parse } from 'vs/base/common/marshalling'; import { Schemas } from 'vs/base/common/network'; -import { matchesScheme, normalizePath } from 'vs/base/common/resources'; +import { normalizePath } from 'vs/base/common/resources'; import { URI } from 'vs/base/common/uri'; import { ICodeEditorService } from 'vs/editor/browser/services/codeEditorService'; import { CommandsRegistry, ICommandService } from 'vs/platform/commands/common/commands'; -import { IOpener, IOpenerService, IValidator, IExternalUriResolver, OpenOptions, ResolveExternalUriOptions, IResolvedExternalUri, IExternalOpener } from 'vs/platform/opener/common/opener'; +import { IOpener, IOpenerService, IValidator, IExternalUriResolver, OpenOptions, ResolveExternalUriOptions, IResolvedExternalUri, IExternalOpener, matchesScheme } from 'vs/platform/opener/common/opener'; import { EditorOpenContext } from 'vs/platform/editor/common/editor'; @@ -43,7 +43,7 @@ export class OpenerService implements IOpenerService { // Default opener: maito, http(s), command, and catch-all-editors this._openerAsExternal = { - open: async (target: URI | URL, options?: OpenOptions) => { + open: async (target: URI | string, options?: OpenOptions) => { if (options?.openExternal || matchesScheme(target, Schemas.mailto) || matchesScheme(target, Schemas.http) || matchesScheme(target, Schemas.https)) { // open externally await this._doOpenExternal(target, options); @@ -59,8 +59,8 @@ export class OpenerService implements IOpenerService { return false; } // run command or bail out if command isn't known - if (!URI.isUri(target)) { - target = URI.from(target); + if (typeof target === 'string') { + target = URI.parse(target); } if (!CommandsRegistry.getCommand(target.path)) { throw new Error(`command '${target.path}' NOT known`); @@ -82,8 +82,8 @@ export class OpenerService implements IOpenerService { this._openerAsEditor = { open: async (target, options: OpenOptions) => { - if (!URI.isUri(target)) { - target = URI.from(target); + if (typeof target === 'string') { + target = URI.parse(target); } let selection: { startLineNumber: number; startColumn: number; } | undefined = undefined; const match = /^L?(\d+)(?:,(\d+))?/.exec(target.fragment); @@ -132,7 +132,7 @@ export class OpenerService implements IOpenerService { this._externalOpener = externalOpener; } - async open(target: URI | URL, options?: OpenOptions): Promise { + async open(target: URI | string, options?: OpenOptions): Promise { // check with contributed validators for (const validator of this._validators.toArray()) { @@ -169,7 +169,7 @@ export class OpenerService implements IOpenerService { return { resolved: resource, dispose: () => { } }; } - private async _doOpenExternal(resource: URI | URL, options: OpenOptions | undefined): Promise { + private async _doOpenExternal(resource: URI | string, options: OpenOptions | undefined): Promise { if (URI.isUri(resource)) { const { resolved } = await this.resolveExternalUri(resource, options); // TODO@Jo neither encodeURI nor toString(true) should be needed @@ -177,7 +177,7 @@ export class OpenerService implements IOpenerService { return this._externalOpener.openExternal(encodeURI(resolved.toString(true))); } else { //todo@joh what about resolveExternalUri? - return this._externalOpener.openExternal(resource.href); + return this._externalOpener.openExternal(resource); } } diff --git a/src/vs/editor/contrib/links/getLinks.ts b/src/vs/editor/contrib/links/getLinks.ts index 081d8b4adde..a85e7f8c942 100644 --- a/src/vs/editor/contrib/links/getLinks.ts +++ b/src/vs/editor/contrib/links/getLinks.ts @@ -13,7 +13,6 @@ import { IModelService } from 'vs/editor/common/services/modelService'; import { CommandsRegistry } from 'vs/platform/commands/common/commands'; import { isDisposable, Disposable } from 'vs/base/common/lifecycle'; import { coalesce } from 'vs/base/common/arrays'; -import { hasURLCtor } from 'vs/base/common/resources'; export class Link implements ILink { @@ -45,19 +44,9 @@ export class Link implements ILink { return this._link.tooltip; } - async resolve(token: CancellationToken): Promise { + async resolve(token: CancellationToken): Promise { if (this._link.url) { - try { - if (URI.isUri(this._link.url)) { - return this._link.url; - } else if (!hasURLCtor) { - return URI.parse(this._link.url); - } else { - return new URL(this._link.url); - } - } catch (e) { - return Promise.reject(new Error('invalid')); - } + return this._link.url; } if (typeof this._provider.resolveLink === 'function') { diff --git a/src/vs/editor/test/browser/services/openerService.test.ts b/src/vs/editor/test/browser/services/openerService.test.ts index 4081b6fc5f8..697dc99402d 100644 --- a/src/vs/editor/test/browser/services/openerService.test.ts +++ b/src/vs/editor/test/browser/services/openerService.test.ts @@ -8,6 +8,7 @@ import { URI } from 'vs/base/common/uri'; import { OpenerService } from 'vs/editor/browser/services/openerService'; import { TestCodeEditorService } from 'vs/editor/test/browser/editorTestServices'; import { CommandsRegistry, ICommandService, NullCommandService } from 'vs/platform/commands/common/commands'; +import { matchesScheme } from 'vs/platform/opener/common/opener'; suite('OpenerService', function () { const editorService = new TestCodeEditorService(); @@ -199,4 +200,18 @@ suite('OpenerService', function () { assert.equal(v1, 2); assert.equal(v2, 0); }); + + test('matchesScheme', function () { + assert.ok(matchesScheme('https://microsoft.com', 'https')); + assert.ok(matchesScheme('http://microsoft.com', 'http')); + assert.ok(matchesScheme('hTTPs://microsoft.com', 'https')); + assert.ok(matchesScheme('httP://microsoft.com', 'http')); + assert.ok(matchesScheme(URI.parse('https://microsoft.com'), 'https')); + assert.ok(matchesScheme(URI.parse('http://microsoft.com'), 'http')); + assert.ok(matchesScheme(URI.parse('hTTPs://microsoft.com'), 'https')); + assert.ok(matchesScheme(URI.parse('httP://microsoft.com'), 'http')); + assert.ok(!matchesScheme(URI.parse('https://microsoft.com'), 'http')); + assert.ok(!matchesScheme(URI.parse('htt://microsoft.com'), 'http')); + assert.ok(!matchesScheme(URI.parse('z://microsoft.com'), 'http')); + }); }); diff --git a/src/vs/platform/opener/common/opener.ts b/src/vs/platform/opener/common/opener.ts index 5cdaebc93ce..8f7921d1581 100644 --- a/src/vs/platform/opener/common/opener.ts +++ b/src/vs/platform/opener/common/opener.ts @@ -6,6 +6,7 @@ import { URI } from 'vs/base/common/uri'; import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; import { IDisposable, Disposable } from 'vs/base/common/lifecycle'; +import { equalsIgnoreCase, startsWithIgnoreCase } from 'vs/base/common/strings'; export const IOpenerService = createDecorator('openerService'); @@ -35,7 +36,7 @@ export interface IResolvedExternalUri extends IDisposable { } export interface IOpener { - open(resource: URI | URL, options?: OpenInternalOptions | OpenExternalOptions): Promise; + open(resource: URI | string, options?: OpenInternalOptions | OpenExternalOptions): Promise; } export interface IExternalOpener { @@ -43,7 +44,7 @@ export interface IExternalOpener { } export interface IValidator { - shouldOpen(resource: URI | URL): Promise; + shouldOpen(resource: URI | string): Promise; } export interface IExternalUriResolver { @@ -82,7 +83,7 @@ export interface IOpenerService { * @param resource A resource * @return A promise that resolves when the opening is done. */ - open(resource: URI | URL, options?: OpenInternalOptions | OpenExternalOptions): Promise; + open(resource: URI | string, options?: OpenInternalOptions | OpenExternalOptions): Promise; /** * Resolve a resource to its external form. @@ -99,3 +100,11 @@ export const NullOpenerService: IOpenerService = Object.freeze({ async open() { return false; }, async resolveExternalUri(uri: URI) { return { resolved: uri, dispose() { } }; }, }); + +export function matchesScheme(target: URI | string, scheme: string) { + if (URI.isUri(target)) { + return equalsIgnoreCase(target.scheme, scheme); + } else { + return startsWithIgnoreCase(target, scheme + ':'); + } +} diff --git a/src/vs/workbench/contrib/terminal/browser/terminalLinkHandler.ts b/src/vs/workbench/contrib/terminal/browser/terminalLinkHandler.ts index 2699cf8ab8a..6cb08ae0b26 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalLinkHandler.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalLinkHandler.ts @@ -266,8 +266,7 @@ export class TerminalLinkHandler { } private _handleHypertextLink(url: string): void { - const urlObj = new URL(url); - this._openerService.open(urlObj, { allowTunneling: !!(this._processManager && this._processManager.remoteAuthority) }); + this._openerService.open(url, { allowTunneling: !!(this._processManager && this._processManager.remoteAuthority) }); } private _isLinkActivationModifierDown(event: MouseEvent): boolean { diff --git a/src/vs/workbench/contrib/url/common/trustedDomainsValidator.ts b/src/vs/workbench/contrib/url/common/trustedDomainsValidator.ts index aabcd4405b2..d8b03dc04d0 100644 --- a/src/vs/workbench/contrib/url/common/trustedDomainsValidator.ts +++ b/src/vs/workbench/contrib/url/common/trustedDomainsValidator.ts @@ -8,7 +8,7 @@ import Severity from 'vs/base/common/severity'; import { URI } from 'vs/base/common/uri'; import { localize } from 'vs/nls'; import { IDialogService } from 'vs/platform/dialogs/common/dialogs'; -import { IOpenerService } from 'vs/platform/opener/common/opener'; +import { IOpenerService, matchesScheme } from 'vs/platform/opener/common/opener'; import { IProductService } from 'vs/platform/product/common/productService'; import { IQuickInputService } from 'vs/platform/quickinput/common/quickInput'; import { IStorageService } from 'vs/platform/storage/common/storage'; @@ -19,7 +19,7 @@ import { } from 'vs/workbench/contrib/url/common/trustedDomains'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import { IClipboardService } from 'vs/platform/clipboard/common/clipboardService'; -import { matchesScheme } from 'vs/base/common/resources'; + export class OpenerValidatorContributions implements IWorkbenchContribution { constructor( @@ -34,13 +34,13 @@ export class OpenerValidatorContributions implements IWorkbenchContribution { this._openerService.registerValidator({ shouldOpen: r => this.validateLink(r) }); } - async validateLink(resource: URI | URL): Promise { + async validateLink(resource: URI | string): Promise { if (!matchesScheme(resource, Schemas.http) && !matchesScheme(resource, Schemas.https)) { return true; } - if (!URI.isUri(resource)) { - resource = URI.from(resource); + if (typeof resource === 'string') { + resource = URI.parse(resource); } const { scheme, authority, path, query, fragment } = resource; diff --git a/src/vs/workbench/services/url/electron-browser/urlService.ts b/src/vs/workbench/services/url/electron-browser/urlService.ts index c46c1948c45..c1485a6ec59 100644 --- a/src/vs/workbench/services/url/electron-browser/urlService.ts +++ b/src/vs/workbench/services/url/electron-browser/urlService.ts @@ -8,7 +8,7 @@ import { URI, UriComponents } from 'vs/base/common/uri'; import { IMainProcessService } from 'vs/platform/ipc/electron-browser/mainProcessService'; import { URLHandlerChannel } from 'vs/platform/url/common/urlIpc'; import { URLService } from 'vs/platform/url/node/urlService'; -import { IOpenerService, IOpener } from 'vs/platform/opener/common/opener'; +import { IOpenerService, IOpener, matchesScheme } from 'vs/platform/opener/common/opener'; import product from 'vs/platform/product/common/product'; import { registerSingleton } from 'vs/platform/instantiation/common/extensions'; import { IElectronEnvironmentService } from 'vs/workbench/services/electron/electron-browser/electronEnvironmentService'; @@ -51,16 +51,15 @@ export class RelayURLService extends URLService implements IURLHandler, IOpener return uri.with({ query }); } - async open(resource: URI | URL, options?: IRelayOpenURLOptions): Promise { + async open(resource: URI | string, options?: IRelayOpenURLOptions): Promise { - if (resource instanceof URL) { - resource = URI.from(resource); - } - - if (resource.scheme !== product.urlProtocol) { + if (!matchesScheme(resource, product.urlProtocol)) { return false; } + if (typeof resource === 'string') { + resource = URI.parse(resource); + } return await this.urlService.open(resource, options); } From 953cd2e6a21c9750456023928ba4ccea224f549a Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Thu, 14 Nov 2019 15:47:32 +0100 Subject: [PATCH 08/15] use string in markdown rendering - in most cases --- src/vs/base/browser/markdownRenderer.ts | 10 +++++----- src/vs/editor/contrib/hover/modesContentHover.ts | 2 +- src/vs/editor/contrib/hover/modesGlyphHover.ts | 2 +- src/vs/editor/contrib/markdown/markdownRenderer.ts | 13 ++----------- 4 files changed, 9 insertions(+), 18 deletions(-) diff --git a/src/vs/base/browser/markdownRenderer.ts b/src/vs/base/browser/markdownRenderer.ts index 57144d67ea6..971addf8f00 100644 --- a/src/vs/base/browser/markdownRenderer.ts +++ b/src/vs/base/browser/markdownRenderer.ts @@ -50,19 +50,19 @@ export function renderMarkdown(markdown: IMarkdownString, options: MarkdownRende const _href = function (href: string, isDomUri: boolean): string { const data = markdown.uris && markdown.uris[href]; if (!data) { - return href; + return href; // no uri exists } let uri = URI.revive(data); + if (URI.parse(href).toString() === uri.toString()) { + return href; // no tranformation performed + } if (isDomUri) { uri = DOM.asDomUri(uri); } if (uri.query) { uri = uri.with({ query: _uriMassage(uri.query) }); } - if (data) { - href = uri.toString(true); - } - return href; + return uri.toString(); }; // signal to code-block render that the diff --git a/src/vs/editor/contrib/hover/modesContentHover.ts b/src/vs/editor/contrib/hover/modesContentHover.ts index f37254d776f..ca710263b24 100644 --- a/src/vs/editor/contrib/hover/modesContentHover.ts +++ b/src/vs/editor/contrib/hover/modesContentHover.ts @@ -207,7 +207,7 @@ export class ModesContentHoverWidget extends ContentHoverWidget { private readonly _themeService: IThemeService, private readonly _keybindingService: IKeybindingService, private readonly _modeService: IModeService, - private readonly _openerService: IOpenerService | null = NullOpenerService, + private readonly _openerService: IOpenerService = NullOpenerService, ) { super(ModesContentHoverWidget.ID, editor); diff --git a/src/vs/editor/contrib/hover/modesGlyphHover.ts b/src/vs/editor/contrib/hover/modesGlyphHover.ts index 85dece77b64..32c6cf14656 100644 --- a/src/vs/editor/contrib/hover/modesGlyphHover.ts +++ b/src/vs/editor/contrib/hover/modesGlyphHover.ts @@ -97,7 +97,7 @@ export class ModesGlyphHoverWidget extends GlyphHoverWidget { constructor( editor: ICodeEditor, modeService: IModeService, - openerService: IOpenerService | null = NullOpenerService, + openerService: IOpenerService = NullOpenerService, ) { super(ModesGlyphHoverWidget.ID, editor); diff --git a/src/vs/editor/contrib/markdown/markdownRenderer.ts b/src/vs/editor/contrib/markdown/markdownRenderer.ts index eb17228cfa8..b4bfe8ddd9e 100644 --- a/src/vs/editor/contrib/markdown/markdownRenderer.ts +++ b/src/vs/editor/contrib/markdown/markdownRenderer.ts @@ -7,7 +7,6 @@ import { IMarkdownString } from 'vs/base/common/htmlContent'; import { renderMarkdown, MarkdownRenderOptions } from 'vs/base/browser/markdownRenderer'; import { IOpenerService, NullOpenerService } from 'vs/platform/opener/common/opener'; import { IModeService } from 'vs/editor/common/services/modeService'; -import { URI } from 'vs/base/common/uri'; import { onUnexpectedError } from 'vs/base/common/errors'; import { tokenizeToString } from 'vs/editor/common/modes/textToHtmlTokenizer'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; @@ -29,7 +28,7 @@ export class MarkdownRenderer extends Disposable { constructor( private readonly _editor: ICodeEditor, @IModeService private readonly _modeService: IModeService, - @optional(IOpenerService) private readonly _openerService: IOpenerService | null = NullOpenerService, + @optional(IOpenerService) private readonly _openerService: IOpenerService = NullOpenerService, ) { super(); } @@ -64,15 +63,7 @@ export class MarkdownRenderer extends Disposable { codeBlockRenderCallback: () => this._onDidRenderCodeBlock.fire(), actionHandler: { callback: (content) => { - let uri: URI | undefined; - try { - uri = URI.parse(content); - } catch { - // ignore - } - if (uri && this._openerService) { - this._openerService.open(uri, { fromUserGesture: true }).catch(onUnexpectedError); - } + this._openerService.open(content, { fromUserGesture: true }).catch(onUnexpectedError); }, disposeables } From 705973999095bc2c67e9d03cf0341409135915fc Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Thu, 14 Nov 2019 15:52:12 +0100 Subject: [PATCH 09/15] simpler markdown link handling --- .../contrib/comments/browser/commentsTreeViewer.ts | 8 +------- .../contrib/preferences/browser/settingsTree.ts | 11 +---------- 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/src/vs/workbench/contrib/comments/browser/commentsTreeViewer.ts b/src/vs/workbench/contrib/comments/browser/commentsTreeViewer.ts index 299a7271b2b..c499a52051e 100644 --- a/src/vs/workbench/contrib/comments/browser/commentsTreeViewer.ts +++ b/src/vs/workbench/contrib/comments/browser/commentsTreeViewer.ts @@ -8,7 +8,6 @@ import * as nls from 'vs/nls'; import { renderMarkdown } from 'vs/base/browser/markdownRenderer'; import { onUnexpectedError } from 'vs/base/common/errors'; import { IDisposable, DisposableStore } from 'vs/base/common/lifecycle'; -import { URI } from 'vs/base/common/uri'; import { IOpenerService } from 'vs/platform/opener/common/opener'; import { IResourceLabel, ResourceLabels } from 'vs/workbench/browser/labels'; import { CommentNode, CommentsModel, ResourceWithCommentThreads } from 'vs/workbench/contrib/comments/common/commentModel'; @@ -127,12 +126,7 @@ export class CommentNodeRenderer implements IListRenderer inline: true, actionHandler: { callback: (content) => { - try { - const uri = URI.parse(content); - this.openerService.open(uri).catch(onUnexpectedError); - } catch (err) { - // ignore - } + this.openerService.open(content).catch(onUnexpectedError); }, disposeables: disposables } diff --git a/src/vs/workbench/contrib/preferences/browser/settingsTree.ts b/src/vs/workbench/contrib/preferences/browser/settingsTree.ts index 329d660bca6..d026d0da526 100644 --- a/src/vs/workbench/contrib/preferences/browser/settingsTree.ts +++ b/src/vs/workbench/contrib/preferences/browser/settingsTree.ts @@ -28,7 +28,6 @@ import { KeyCode } from 'vs/base/common/keyCodes'; import { dispose, IDisposable, Disposable, DisposableStore } from 'vs/base/common/lifecycle'; import { ISpliceable } from 'vs/base/common/sequence'; import { escapeRegExpCharacters, startsWith } from 'vs/base/common/strings'; -import { URI } from 'vs/base/common/uri'; import { localize } from 'vs/nls'; import { IClipboardService } from 'vs/platform/clipboard/common/clipboardService'; import { ICommandService } from 'vs/platform/commands/common/commands'; @@ -485,15 +484,7 @@ export abstract class AbstractSettingRenderer extends Disposable implements ITre }; this._onDidClickSettingLink.fire(e); } else { - let uri: URI | undefined; - try { - uri = URI.parse(content); - } catch (err) { - // ignore - } - if (uri) { - this._openerService.open(uri).catch(onUnexpectedError); - } + this._openerService.open(content).catch(onUnexpectedError); } }, disposeables From 67ac71161cc2295441b70d3d772af77e9b0bdfaa Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Thu, 14 Nov 2019 16:42:35 +0100 Subject: [PATCH 10/15] openExternal uses resolver for URI and string --- src/vs/editor/browser/services/openerService.ts | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/vs/editor/browser/services/openerService.ts b/src/vs/editor/browser/services/openerService.ts index 7e1d50546aa..b92275c5cf0 100644 --- a/src/vs/editor/browser/services/openerService.ts +++ b/src/vs/editor/browser/services/openerService.ts @@ -170,14 +170,17 @@ export class OpenerService implements IOpenerService { } private async _doOpenExternal(resource: URI | string, options: OpenOptions | undefined): Promise { - if (URI.isUri(resource)) { - const { resolved } = await this.resolveExternalUri(resource, options); - // TODO@Jo neither encodeURI nor toString(true) should be needed - // once we go with URL and not URI - return this._externalOpener.openExternal(encodeURI(resolved.toString(true))); - } else { - //todo@joh what about resolveExternalUri? + + //todo@joh IExternalUriResolver should support `uri: URI | string` + const uri = typeof resource === 'string' ? URI.parse(resource) : resource; + const { resolved } = await this.resolveExternalUri(uri, options); + + if (typeof resource === 'string' && uri.toString() === resolved.toString()) { + // open the url-string AS IS return this._externalOpener.openExternal(resource); + } else { + // open URI using the toString(noEncode)+encodeURI-trick + return this._externalOpener.openExternal(encodeURI(resolved.toString(true))); } } From 0083c78aa4c906d9ac0b8fe91e1a548313dc9678 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Fri, 15 Nov 2019 09:28:21 +0100 Subject: [PATCH 11/15] Revert "add URI.from(url)-overload" This reverts commit e24fefe0a89205ce519efd8bd4ee53bde4bfe90c. --- src/vs/base/common/uri.ts | 9 +-------- src/vs/monaco.d.ts | 2 +- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/vs/base/common/uri.ts b/src/vs/base/common/uri.ts index 3df1ee149e2..19f89eec626 100644 --- a/src/vs/base/common/uri.ts +++ b/src/vs/base/common/uri.ts @@ -323,14 +323,7 @@ export class URI implements UriComponents { return new _URI('file', authority, path, _empty, _empty); } - static from(components: { scheme: string; authority?: string; path?: string; query?: string; fragment?: string } | URL): URI { - - if (components instanceof URL) { - const value = <_URI>_URI.parse(components.href); - value._formatted = components.href; - return value; - } - + static from(components: { scheme: string; authority?: string; path?: string; query?: string; fragment?: string }): URI { return new _URI( components.scheme, components.authority, diff --git a/src/vs/monaco.d.ts b/src/vs/monaco.d.ts index ff535f4bfbc..2fa74f0ac35 100644 --- a/src/vs/monaco.d.ts +++ b/src/vs/monaco.d.ts @@ -160,7 +160,7 @@ declare namespace monaco { path?: string; query?: string; fragment?: string; - } | URL): Uri; + }): Uri; /** * Creates a string representation for this Uri. It's guaranteed that calling * `Uri.parse` with the result of this function creates an Uri which is equal From f651ccac7e2806d8f8c57c47429293c2435c2200 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Fri, 15 Nov 2019 10:13:48 +0100 Subject: [PATCH 12/15] extract openers --- .../editor/browser/services/openerService.ts | 143 +++++++++--------- 1 file changed, 71 insertions(+), 72 deletions(-) diff --git a/src/vs/editor/browser/services/openerService.ts b/src/vs/editor/browser/services/openerService.ts index b92275c5cf0..ead9bd31bfb 100644 --- a/src/vs/editor/browser/services/openerService.ts +++ b/src/vs/editor/browser/services/openerService.ts @@ -16,6 +16,71 @@ import { IOpener, IOpenerService, IValidator, IExternalUriResolver, OpenOptions, import { EditorOpenContext } from 'vs/platform/editor/common/editor'; +class CommandOpener implements IOpener { + + constructor(@ICommandService private readonly _commandService: ICommandService) { } + + async open(target: URI | string) { + if (!matchesScheme(target, Schemas.command)) { + return false; + } + // run command or bail out if command isn't known + if (typeof target === 'string') { + target = URI.parse(target); + } + if (!CommandsRegistry.getCommand(target.path)) { + throw new Error(`command '${target.path}' NOT known`); + } + // execute as command + let args: any = []; + try { + args = parse(target.query); + if (!Array.isArray(args)) { + args = [args]; + } + } catch (e) { + // ignore error + } + await this._commandService.executeCommand(target.path, ...args); + return true; + } +} + +class EditorOpener implements IOpener { + + constructor(@ICodeEditorService private readonly _editorService: ICodeEditorService) { } + + async open(target: URI | string, options: OpenOptions) { + if (typeof target === 'string') { + target = URI.parse(target); + } + let selection: { startLineNumber: number; startColumn: number; } | undefined = undefined; + const match = /^L?(\d+)(?:,(\d+))?/.exec(target.fragment); + if (match) { + // support file:///some/file.js#73,84 + // support file:///some/file.js#L73 + selection = { + startLineNumber: parseInt(match[1]), + startColumn: match[2] ? parseInt(match[2]) : 1 + }; + // remove fragment + target = target.with({ fragment: '' }); + } + + if (target.scheme === Schemas.file) { + target = normalizePath(target); // workaround for non-normalized paths (https://github.com/Microsoft/vscode/issues/12954) + } + + await this._editorService.openCodeEditor( + { resource: target, options: { selection, context: options?.fromUserGesture ? EditorOpenContext.USER : EditorOpenContext.API } }, + this._editorService.getFocusedCodeEditor(), + options?.openToSide + ); + + return true; + } +} + export class OpenerService implements IOpenerService { _serviceBrand: undefined; @@ -25,9 +90,6 @@ export class OpenerService implements IOpenerService { private readonly _resolvers = new LinkedList(); private _externalOpener: IExternalOpener; - private _openerAsExternal: IOpener; - private _openerAsCommand: IOpener; - private _openerAsEditor: IOpener; constructor( @ICodeEditorService editorService: ICodeEditorService, @@ -42,7 +104,7 @@ export class OpenerService implements IOpenerService { }; // Default opener: maito, http(s), command, and catch-all-editors - this._openerAsExternal = { + this._openers.push({ open: async (target: URI | string, options?: OpenOptions) => { if (options?.openExternal || matchesScheme(target, Schemas.mailto) || matchesScheme(target, Schemas.http) || matchesScheme(target, Schemas.https)) { // open externally @@ -51,70 +113,13 @@ export class OpenerService implements IOpenerService { } return false; } - }; - - this._openerAsCommand = { - open: async (target) => { - if (!matchesScheme(target, Schemas.command)) { - return false; - } - // run command or bail out if command isn't known - if (typeof target === 'string') { - target = URI.parse(target); - } - if (!CommandsRegistry.getCommand(target.path)) { - throw new Error(`command '${target.path}' NOT known`); - } - // execute as command - let args: any = []; - try { - args = parse(target.query); - if (!Array.isArray(args)) { - args = [args]; - } - } catch (e) { - // ignore error - } - await commandService.executeCommand(target.path, ...args); - return true; - } - }; - - this._openerAsEditor = { - open: async (target, options: OpenOptions) => { - if (typeof target === 'string') { - target = URI.parse(target); - } - let selection: { startLineNumber: number; startColumn: number; } | undefined = undefined; - const match = /^L?(\d+)(?:,(\d+))?/.exec(target.fragment); - if (match) { - // support file:///some/file.js#73,84 - // support file:///some/file.js#L73 - selection = { - startLineNumber: parseInt(match[1]), - startColumn: match[2] ? parseInt(match[2]) : 1 - }; - // remove fragment - target = target.with({ fragment: '' }); - } - - if (target.scheme === Schemas.file) { - target = normalizePath(target); // workaround for non-normalized paths (https://github.com/Microsoft/vscode/issues/12954) - } - - await editorService.openCodeEditor( - { resource: target, options: { selection, context: options?.fromUserGesture ? EditorOpenContext.USER : EditorOpenContext.API } }, - editorService.getFocusedCodeEditor(), - options?.openToSide - ); - - return true; - } - }; + }); + this._openers.push(new CommandOpener(commandService)); + this._openers.push(new EditorOpener(editorService)); } registerOpener(opener: IOpener): IDisposable { - const remove = this._openers.push(opener); + const remove = this._openers.unshift(opener); return { dispose: remove }; } @@ -149,13 +154,7 @@ export class OpenerService implements IOpenerService { } } - // use default openers - for (const opener of [this._openerAsExternal, this._openerAsCommand, this._openerAsEditor]) { - if (await opener.open(target, options)) { - break; - } - } - return true; + return false; } async resolveExternalUri(resource: URI, options?: ResolveExternalUriOptions): Promise { From 544b0abf5bc125d63a8c141d3268aa36d8cd4f92 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Fri, 15 Nov 2019 10:28:03 +0100 Subject: [PATCH 13/15] allow $openUri to accept a URI and string, adopt consumer but keep the API as is --- .../workbench/api/browser/mainThreadWindow.ts | 24 ++++++++++++++++--- .../workbench/api/common/extHost.protocol.ts | 2 +- .../api/common/extHostRequireInterceptor.ts | 12 ++++++---- src/vs/workbench/api/common/extHostWindow.ts | 12 ---------- 4 files changed, 29 insertions(+), 21 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadWindow.ts b/src/vs/workbench/api/browser/mainThreadWindow.ts index bbce19a3388..e03411f1823 100644 --- a/src/vs/workbench/api/browser/mainThreadWindow.ts +++ b/src/vs/workbench/api/browser/mainThreadWindow.ts @@ -10,6 +10,8 @@ import { IOpenerService } from 'vs/platform/opener/common/opener'; import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers'; import { ExtHostContext, ExtHostWindowShape, IExtHostContext, IOpenUriOptions, MainContext, MainThreadWindowShape } from '../common/extHost.protocol'; import { IHostService } from 'vs/workbench/services/host/browser/host'; +import { Schemas } from 'vs/base/common/network'; +import { isFalsyOrWhitespace } from 'vs/base/common/strings'; @extHostNamedCustomer(MainContext.MainThreadWindow) export class MainThreadWindow implements MainThreadWindowShape { @@ -42,9 +44,25 @@ export class MainThreadWindow implements MainThreadWindowShape { return Promise.resolve(this.hostService.hasFocus); } - async $openUri(uriComponents: UriComponents, options: IOpenUriOptions): Promise { - const uri = URI.from(uriComponents); - return this.openerService.open(uri, { openExternal: true, allowTunneling: options.allowTunneling }); + async $openUri(stringOrComp: UriComponents | string, options: IOpenUriOptions): Promise { + + const uri = typeof stringOrComp === 'string' + ? URI.parse(stringOrComp) + : URI.revive(stringOrComp); + + // validate + if (isFalsyOrWhitespace(uri.scheme)) { + return Promise.reject('Invalid scheme - cannot be empty'); + } else if (uri.scheme === Schemas.command) { + return Promise.reject(`Invalid scheme '${uri.scheme}'`); + } + + // open AS-IS, keep string alive + if (typeof stringOrComp === 'string') { + return this.openerService.open(stringOrComp, { openExternal: true, allowTunneling: options.allowTunneling }); + } else { + return this.openerService.open(uri, { openExternal: true, allowTunneling: options.allowTunneling }); + } } async $asExternalUri(uriComponents: UriComponents, options: IOpenUriOptions): Promise { diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 9bc433a1727..3ce80496914 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -759,7 +759,7 @@ export interface IOpenUriOptions { export interface MainThreadWindowShape extends IDisposable { $getWindowVisibility(): Promise; - $openUri(uri: UriComponents, options: IOpenUriOptions): Promise; + $openUri(uri: UriComponents | string, options: IOpenUriOptions): Promise; $asExternalUri(uri: UriComponents, options: IOpenUriOptions): Promise; } diff --git a/src/vs/workbench/api/common/extHostRequireInterceptor.ts b/src/vs/workbench/api/common/extHostRequireInterceptor.ts index b6f94048fbd..6f8fc58d0b8 100644 --- a/src/vs/workbench/api/common/extHostRequireInterceptor.ts +++ b/src/vs/workbench/api/common/extHostRequireInterceptor.ts @@ -19,6 +19,8 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti import { IExtHostExtensionService } from 'vs/workbench/api/common/extHostExtensionService'; import { platform } from 'vs/base/common/process'; import { ILogService } from 'vs/platform/log/common/log'; +import { matchesScheme } from 'vs/platform/opener/common/opener'; +import { Schemas } from 'vs/base/common/network'; interface LoadFunction { @@ -239,15 +241,15 @@ class OpenNodeModuleFactory implements INodeModuleFactory { const mainThreadWindow = rpcService.getProxy(MainContext.MainThreadWindow); this._impl = (target, options) => { - const uri: URI = URI.parse(target); // If we have options use the original method. if (options) { return this.callOriginal(target, options); } - if (uri.scheme === 'http' || uri.scheme === 'https') { - return mainThreadWindow.$openUri(uri, { allowTunneling: true }); - } else if (uri.scheme === 'mailto' || uri.scheme === this._appUriScheme) { - return mainThreadWindow.$openUri(uri, {}); + if (matchesScheme(target, Schemas.http) || matchesScheme(target, Schemas.https)) { + return mainThreadWindow.$openUri(target, { allowTunneling: true }); + + } else if (matchesScheme(target, Schemas.mailto) || matchesScheme(target, this._appUriScheme)) { + return mainThreadWindow.$openUri(target, {}); } return this.callOriginal(target, options); }; diff --git a/src/vs/workbench/api/common/extHostWindow.ts b/src/vs/workbench/api/common/extHostWindow.ts index 8ce82ac8b2d..d86a9203f3c 100644 --- a/src/vs/workbench/api/common/extHostWindow.ts +++ b/src/vs/workbench/api/common/extHostWindow.ts @@ -39,18 +39,6 @@ export class ExtHostWindow implements ExtHostWindowShape { } openUri(stringOrUri: string | URI, options: IOpenUriOptions): Promise { - if (typeof stringOrUri === 'string') { - try { - stringOrUri = URI.parse(stringOrUri); - } catch (e) { - return Promise.reject(`Invalid uri - '${stringOrUri}'`); - } - } - if (isFalsyOrWhitespace(stringOrUri.scheme)) { - return Promise.reject('Invalid scheme - cannot be empty'); - } else if (stringOrUri.scheme === Schemas.command) { - return Promise.reject(`Invalid scheme '${stringOrUri.scheme}'`); - } return this._proxy.$openUri(stringOrUri, options); } From e51ef85bcbcd1d4f9f9af274235de79890c09504 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Fri, 15 Nov 2019 10:29:29 +0100 Subject: [PATCH 14/15] Revert "allow $openUri to accept a URI and string, adopt consumer but keep the API as is" This reverts commit 544b0abf5bc125d63a8c141d3268aa36d8cd4f92. --- .../workbench/api/browser/mainThreadWindow.ts | 24 +++---------------- .../workbench/api/common/extHost.protocol.ts | 2 +- .../api/common/extHostRequireInterceptor.ts | 12 ++++------ src/vs/workbench/api/common/extHostWindow.ts | 12 ++++++++++ 4 files changed, 21 insertions(+), 29 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadWindow.ts b/src/vs/workbench/api/browser/mainThreadWindow.ts index e03411f1823..bbce19a3388 100644 --- a/src/vs/workbench/api/browser/mainThreadWindow.ts +++ b/src/vs/workbench/api/browser/mainThreadWindow.ts @@ -10,8 +10,6 @@ import { IOpenerService } from 'vs/platform/opener/common/opener'; import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers'; import { ExtHostContext, ExtHostWindowShape, IExtHostContext, IOpenUriOptions, MainContext, MainThreadWindowShape } from '../common/extHost.protocol'; import { IHostService } from 'vs/workbench/services/host/browser/host'; -import { Schemas } from 'vs/base/common/network'; -import { isFalsyOrWhitespace } from 'vs/base/common/strings'; @extHostNamedCustomer(MainContext.MainThreadWindow) export class MainThreadWindow implements MainThreadWindowShape { @@ -44,25 +42,9 @@ export class MainThreadWindow implements MainThreadWindowShape { return Promise.resolve(this.hostService.hasFocus); } - async $openUri(stringOrComp: UriComponents | string, options: IOpenUriOptions): Promise { - - const uri = typeof stringOrComp === 'string' - ? URI.parse(stringOrComp) - : URI.revive(stringOrComp); - - // validate - if (isFalsyOrWhitespace(uri.scheme)) { - return Promise.reject('Invalid scheme - cannot be empty'); - } else if (uri.scheme === Schemas.command) { - return Promise.reject(`Invalid scheme '${uri.scheme}'`); - } - - // open AS-IS, keep string alive - if (typeof stringOrComp === 'string') { - return this.openerService.open(stringOrComp, { openExternal: true, allowTunneling: options.allowTunneling }); - } else { - return this.openerService.open(uri, { openExternal: true, allowTunneling: options.allowTunneling }); - } + async $openUri(uriComponents: UriComponents, options: IOpenUriOptions): Promise { + const uri = URI.from(uriComponents); + return this.openerService.open(uri, { openExternal: true, allowTunneling: options.allowTunneling }); } async $asExternalUri(uriComponents: UriComponents, options: IOpenUriOptions): Promise { diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 3ce80496914..9bc433a1727 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -759,7 +759,7 @@ export interface IOpenUriOptions { export interface MainThreadWindowShape extends IDisposable { $getWindowVisibility(): Promise; - $openUri(uri: UriComponents | string, options: IOpenUriOptions): Promise; + $openUri(uri: UriComponents, options: IOpenUriOptions): Promise; $asExternalUri(uri: UriComponents, options: IOpenUriOptions): Promise; } diff --git a/src/vs/workbench/api/common/extHostRequireInterceptor.ts b/src/vs/workbench/api/common/extHostRequireInterceptor.ts index 6f8fc58d0b8..b6f94048fbd 100644 --- a/src/vs/workbench/api/common/extHostRequireInterceptor.ts +++ b/src/vs/workbench/api/common/extHostRequireInterceptor.ts @@ -19,8 +19,6 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti import { IExtHostExtensionService } from 'vs/workbench/api/common/extHostExtensionService'; import { platform } from 'vs/base/common/process'; import { ILogService } from 'vs/platform/log/common/log'; -import { matchesScheme } from 'vs/platform/opener/common/opener'; -import { Schemas } from 'vs/base/common/network'; interface LoadFunction { @@ -241,15 +239,15 @@ class OpenNodeModuleFactory implements INodeModuleFactory { const mainThreadWindow = rpcService.getProxy(MainContext.MainThreadWindow); this._impl = (target, options) => { + const uri: URI = URI.parse(target); // If we have options use the original method. if (options) { return this.callOriginal(target, options); } - if (matchesScheme(target, Schemas.http) || matchesScheme(target, Schemas.https)) { - return mainThreadWindow.$openUri(target, { allowTunneling: true }); - - } else if (matchesScheme(target, Schemas.mailto) || matchesScheme(target, this._appUriScheme)) { - return mainThreadWindow.$openUri(target, {}); + if (uri.scheme === 'http' || uri.scheme === 'https') { + return mainThreadWindow.$openUri(uri, { allowTunneling: true }); + } else if (uri.scheme === 'mailto' || uri.scheme === this._appUriScheme) { + return mainThreadWindow.$openUri(uri, {}); } return this.callOriginal(target, options); }; diff --git a/src/vs/workbench/api/common/extHostWindow.ts b/src/vs/workbench/api/common/extHostWindow.ts index d86a9203f3c..8ce82ac8b2d 100644 --- a/src/vs/workbench/api/common/extHostWindow.ts +++ b/src/vs/workbench/api/common/extHostWindow.ts @@ -39,6 +39,18 @@ export class ExtHostWindow implements ExtHostWindowShape { } openUri(stringOrUri: string | URI, options: IOpenUriOptions): Promise { + if (typeof stringOrUri === 'string') { + try { + stringOrUri = URI.parse(stringOrUri); + } catch (e) { + return Promise.reject(`Invalid uri - '${stringOrUri}'`); + } + } + if (isFalsyOrWhitespace(stringOrUri.scheme)) { + return Promise.reject('Invalid scheme - cannot be empty'); + } else if (stringOrUri.scheme === Schemas.command) { + return Promise.reject(`Invalid scheme '${stringOrUri.scheme}'`); + } return this._proxy.$openUri(stringOrUri, options); } From e80c62bdcb0a1d7ce0d3d826241df0ef20162417 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Fri, 15 Nov 2019 10:53:04 +0100 Subject: [PATCH 15/15] allow $openUri to accept URI and string --- src/vs/workbench/api/browser/mainThreadWindow.ts | 12 ++++++++++-- src/vs/workbench/api/common/extHost.protocol.ts | 2 +- .../api/common/extHostRequireInterceptor.ts | 4 ++-- src/vs/workbench/api/common/extHostWindow.ts | 4 +++- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadWindow.ts b/src/vs/workbench/api/browser/mainThreadWindow.ts index bbce19a3388..2a431111e93 100644 --- a/src/vs/workbench/api/browser/mainThreadWindow.ts +++ b/src/vs/workbench/api/browser/mainThreadWindow.ts @@ -42,9 +42,17 @@ export class MainThreadWindow implements MainThreadWindowShape { return Promise.resolve(this.hostService.hasFocus); } - async $openUri(uriComponents: UriComponents, options: IOpenUriOptions): Promise { + async $openUri(uriComponents: UriComponents, uriString: string | undefined, options: IOpenUriOptions): Promise { const uri = URI.from(uriComponents); - return this.openerService.open(uri, { openExternal: true, allowTunneling: options.allowTunneling }); + let target: URI | string; + if (uriString && URI.parse(uriString).toString() === uri.toString()) { + // called with string and no transformation happened -> keep string + target = uriString; + } else { + // called with URI or transformed -> use uri + target = uri; + } + return this.openerService.open(target, { openExternal: true, allowTunneling: options.allowTunneling }); } async $asExternalUri(uriComponents: UriComponents, options: IOpenUriOptions): Promise { diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 9bc433a1727..5df456ab6c7 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -759,7 +759,7 @@ export interface IOpenUriOptions { export interface MainThreadWindowShape extends IDisposable { $getWindowVisibility(): Promise; - $openUri(uri: UriComponents, options: IOpenUriOptions): Promise; + $openUri(uri: UriComponents, uriString: string | undefined, options: IOpenUriOptions): Promise; $asExternalUri(uri: UriComponents, options: IOpenUriOptions): Promise; } diff --git a/src/vs/workbench/api/common/extHostRequireInterceptor.ts b/src/vs/workbench/api/common/extHostRequireInterceptor.ts index b6f94048fbd..c06e86c8305 100644 --- a/src/vs/workbench/api/common/extHostRequireInterceptor.ts +++ b/src/vs/workbench/api/common/extHostRequireInterceptor.ts @@ -245,9 +245,9 @@ class OpenNodeModuleFactory implements INodeModuleFactory { return this.callOriginal(target, options); } if (uri.scheme === 'http' || uri.scheme === 'https') { - return mainThreadWindow.$openUri(uri, { allowTunneling: true }); + return mainThreadWindow.$openUri(uri, target, { allowTunneling: true }); } else if (uri.scheme === 'mailto' || uri.scheme === this._appUriScheme) { - return mainThreadWindow.$openUri(uri, {}); + return mainThreadWindow.$openUri(uri, target, {}); } return this.callOriginal(target, options); }; diff --git a/src/vs/workbench/api/common/extHostWindow.ts b/src/vs/workbench/api/common/extHostWindow.ts index 8ce82ac8b2d..f8c5d5fa3d7 100644 --- a/src/vs/workbench/api/common/extHostWindow.ts +++ b/src/vs/workbench/api/common/extHostWindow.ts @@ -39,7 +39,9 @@ export class ExtHostWindow implements ExtHostWindowShape { } openUri(stringOrUri: string | URI, options: IOpenUriOptions): Promise { + let uriAsString: string | undefined; if (typeof stringOrUri === 'string') { + uriAsString = stringOrUri; try { stringOrUri = URI.parse(stringOrUri); } catch (e) { @@ -51,7 +53,7 @@ export class ExtHostWindow implements ExtHostWindowShape { } else if (stringOrUri.scheme === Schemas.command) { return Promise.reject(`Invalid scheme '${stringOrUri.scheme}'`); } - return this._proxy.$openUri(stringOrUri, options); + return this._proxy.$openUri(stringOrUri, uriAsString, options); } async asExternalUri(uri: URI, options: IOpenUriOptions): Promise {