Skip to content

Commit

Permalink
debt - prefer opener service over window.open(). Block window.open()
Browse files Browse the repository at this point in the history
  • Loading branch information
bpasero committed Aug 20, 2019
1 parent 6aa2a75 commit b0d58e4
Show file tree
Hide file tree
Showing 17 changed files with 117 additions and 67 deletions.
6 changes: 4 additions & 2 deletions src/vs/workbench/contrib/debug/browser/debugSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { IViewletService } from 'vs/workbench/services/viewlet/browser/viewlet';
import { ReplModel } from 'vs/workbench/contrib/debug/common/replModel';
import { onUnexpectedError } from 'vs/base/common/errors';
import { INotificationService } from 'vs/platform/notification/common/notification';
import { IOpenerService } from 'vs/platform/opener/common/opener';

export class DebugSession implements IDebugSession {

Expand Down Expand Up @@ -66,7 +67,8 @@ export class DebugSession implements IDebugSession {
@IWorkspaceContextService private readonly workspaceContextService: IWorkspaceContextService,
@INotificationService private readonly notificationService: INotificationService,
@IProductService private readonly productService: IProductService,
@IWindowsService private readonly windowsService: IWindowsService
@IWindowsService private readonly windowsService: IWindowsService,
@IOpenerService private readonly openerService: IOpenerService
) {
this.id = generateUuid();
this.repl = new ReplModel(this);
Expand Down Expand Up @@ -167,7 +169,7 @@ export class DebugSession implements IDebugSession {

return dbgr.createDebugAdapter(this).then(debugAdapter => {

this.raw = new RawDebugSession(debugAdapter, dbgr, this.telemetryService, customTelemetryService, this.windowsService);
this.raw = new RawDebugSession(debugAdapter, dbgr, this.telemetryService, customTelemetryService, this.windowsService, this.openerService);

return this.raw.start().then(() => {

Expand Down
6 changes: 4 additions & 2 deletions src/vs/workbench/contrib/debug/browser/rawDebugSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { IWindowsService } from 'vs/platform/windows/common/windows';
import { URI } from 'vs/base/common/uri';
import { IProcessEnvironment } from 'vs/base/common/platform';
import { env as processEnv } from 'vs/base/common/process';
import { IOpenerService } from 'vs/platform/opener/common/opener';

/**
* This interface represents a single command line argument split into a "prefix" and a "path" half.
Expand Down Expand Up @@ -74,7 +75,8 @@ export class RawDebugSession {
dbgr: IDebugger,
private readonly telemetryService: ITelemetryService,
public readonly customTelemetryService: ITelemetryService | undefined,
private readonly windowsService: IWindowsService
private readonly windowsService: IWindowsService,
private readonly openerService: IOpenerService

) {
this.debugAdapter = debugAdapter;
Expand Down Expand Up @@ -652,7 +654,7 @@ export class RawDebugSession {
const label = error.urlLabel ? error.urlLabel : nls.localize('moreInfo', "More Info");
return createErrorWithActions(userMessage, {
actions: [new Action('debug.moreInfo', label, undefined, true, () => {
window.open(error.url);
this.openerService.open(URI.parse(error.url));
return Promise.resolve(null);
})]
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ import { Source } from 'vs/workbench/contrib/debug/common/debugSource';
import { DebugSession } from 'vs/workbench/contrib/debug/browser/debugSession';
import { ReplModel } from 'vs/workbench/contrib/debug/common/replModel';
import { IBreakpointUpdateData } from 'vs/workbench/contrib/debug/common/debug';
import { NullOpenerService } from 'vs/platform/opener/common/opener';

function createMockSession(model: DebugModel, name = 'mockSession', parentSession?: DebugSession | undefined): DebugSession {
return new DebugSession({ resolved: { name, type: 'node', request: 'launch' }, unresolved: undefined }, undefined!, model, parentSession, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!);
return new DebugSession({ resolved: { name, type: 'node', request: 'launch' }, unresolved: undefined }, undefined!, model, parentSession, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, NullOpenerService);
}

suite('Debug - Model', () => {
Expand Down Expand Up @@ -427,7 +428,7 @@ suite('Debug - Model', () => {
// Repl output

test('repl output', () => {
const session = new DebugSession({ resolved: { name: 'mockSession', type: 'node', request: 'launch' }, unresolved: undefined }, undefined!, model, undefined, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!);
const session = new DebugSession({ resolved: { name: 'mockSession', type: 'node', request: 'launch' }, unresolved: undefined }, undefined!, model, undefined, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, NullOpenerService);
const repl = new ReplModel(session);
repl.appendToRepl('first line\n', severity.Error);
repl.appendToRepl('second line ', severity.Error);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,17 @@ import { IExtensionsViewlet } from 'vs/workbench/contrib/extensions/common/exten
import { IWorkbenchContribution } from 'vs/workbench/common/contributions';
import { Disposable } from 'vs/base/common/lifecycle';
import { language } from 'vs/base/common/platform';
import { IOpenerService } from 'vs/platform/opener/common/opener';
import { URI } from 'vs/base/common/uri';

export class ExperimentalPrompts extends Disposable implements IWorkbenchContribution {

constructor(
@IExperimentService private readonly experimentService: IExperimentService,
@IViewletService private readonly viewletService: IViewletService,
@INotificationService private readonly notificationService: INotificationService,
@ITelemetryService private readonly telemetryService: ITelemetryService
@ITelemetryService private readonly telemetryService: ITelemetryService,
@IOpenerService private readonly openerService: IOpenerService

) {
super();
Expand Down Expand Up @@ -65,7 +68,7 @@ export class ExperimentalPrompts extends Disposable implements IWorkbenchContrib
run: () => {
logTelemetry(commandText);
if (command.externalLink) {
window.open(command.externalLink);
this.openerService.open(URI.parse(command.externalLink));
} else if (command.curatedExtensionsKey && Array.isArray(command.curatedExtensionsList)) {
this.viewletService.openViewlet('workbench.view.extensions', true)
.then(viewlet => viewlet as IExtensionsViewlet)
Expand Down
11 changes: 6 additions & 5 deletions src/vs/workbench/contrib/extensions/browser/extensionEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import { IWebviewService, Webview } from 'vs/workbench/contrib/webview/browser/w
import { StandardKeyboardEvent } from 'vs/base/browser/keyboardEvent';
import { generateUuid } from 'vs/base/common/uuid';
import { platform } from 'vs/base/common/process';
import { URI } from 'vs/base/common/uri';

function removeEmbeddedSVGs(documentContent: string): string {
const newDocument = new DOMParser().parseFromString(documentContent, 'text/html');
Expand Down Expand Up @@ -185,7 +186,7 @@ export class ExtensionEditor extends BaseEditor {
@IStorageService storageService: IStorageService,
@IExtensionService private readonly extensionService: IExtensionService,
@IWorkbenchThemeService private readonly workbenchThemeService: IWorkbenchThemeService,
@IWebviewService private readonly webviewService: IWebviewService,
@IWebviewService private readonly webviewService: IWebviewService
) {
super(ExtensionEditor.ID, telemetryService, themeService, storageService);
this.extensionReadme = null;
Expand Down Expand Up @@ -354,16 +355,16 @@ export class ExtensionEditor extends BaseEditor {
toggleClass(template.publisher, 'clickable', !!extension.url);
toggleClass(template.rating, 'clickable', !!extension.url);
if (extension.url) {
this.transientDisposables.add(this.onClick(template.name, () => window.open(extension.url)));
this.transientDisposables.add(this.onClick(template.rating, () => window.open(`${extension.url}#review-details`)));
this.transientDisposables.add(this.onClick(template.name, () => this.openerService.open(URI.parse(extension.url!))));
this.transientDisposables.add(this.onClick(template.rating, () => this.openerService.open(URI.parse(`${extension.url}#review-details`))));
this.transientDisposables.add(this.onClick(template.publisher, () => {
this.viewletService.openViewlet(VIEWLET_ID, true)
.then(viewlet => viewlet as IExtensionsViewlet)
.then(viewlet => viewlet.search(`publisher:"${extension.publisherDisplayName}"`));
}));

if (extension.licenseUrl) {
this.transientDisposables.add(this.onClick(template.license, () => window.open(extension.licenseUrl)));
this.transientDisposables.add(this.onClick(template.license, () => this.openerService.open(URI.parse(extension.licenseUrl!))));
template.license.style.display = 'initial';
} else {
template.license.style.display = 'none';
Expand All @@ -373,7 +374,7 @@ export class ExtensionEditor extends BaseEditor {
}

if (extension.repository) {
this.transientDisposables.add(this.onClick(template.repository, () => window.open(extension.repository)));
this.transientDisposables.add(this.onClick(template.repository, () => this.openerService.open(URI.parse(extension.repository!))));
template.repository.style.display = 'initial';
}
else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { join } from 'vs/base/common/path';
import { onUnexpectedError } from 'vs/base/common/errors';
import { IDialogService } from 'vs/platform/dialogs/common/dialogs';
import Severity from 'vs/base/common/severity';
import { IOpenerService } from 'vs/platform/opener/common/opener';

abstract class RepoInfo {
abstract get base(): string;
Expand Down Expand Up @@ -117,6 +118,7 @@ class ReportExtensionSlowAction extends Action {
readonly repoInfo: RepoInfo,
readonly profile: IExtensionHostProfile,
@IDialogService private readonly _dialogService: IDialogService,
@IOpenerService private readonly _openerService: IOpenerService
) {
super('report.slow', localize('cmd.report', "Report Issue"));
}
Expand All @@ -140,7 +142,7 @@ class ReportExtensionSlowAction extends Action {
- VSCode version: \`${pkg.version}\`\n\n${message}`);

const url = `${this.repoInfo.base}/${this.repoInfo.owner}/${this.repoInfo.repo}/issues/new/?body=${body}&title=${title}`;
window.open(url);
this._openerService.open(URI.parse(url));

this._dialogService.show(
Severity.Info,
Expand All @@ -158,6 +160,7 @@ class ShowExtensionSlowAction extends Action {
readonly repoInfo: RepoInfo,
readonly profile: IExtensionHostProfile,
@IDialogService private readonly _dialogService: IDialogService,
@IOpenerService private readonly _openerService: IOpenerService
) {
super('show.slow', localize('cmd.show', "Show Issues"));
}
Expand All @@ -172,7 +175,7 @@ class ShowExtensionSlowAction extends Action {

// show issues
const url = `${this.repoInfo.base}/${this.repoInfo.owner}/${this.repoInfo.repo}/issues?utf8=✓&q=is%3Aissue+state%3Aopen+%22Extension+causes+high+cpu+load%22`;
window.open(url);
this._openerService.open(URI.parse(url));

this._dialogService.show(
Severity.Info,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ import { ExtensionIdentifier, ExtensionType, IExtensionDescription } from 'vs/pl
import { REMOTE_HOST_SCHEME } from 'vs/platform/remote/common/remoteHosts';
import { SlowExtensionAction } from 'vs/workbench/contrib/extensions/electron-browser/extensionsSlowActions';
import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService';
import { IOpenerService } from 'vs/platform/opener/common/opener';
import { URI } from 'vs/base/common/uri';

export const IExtensionHostProfileService = createDecorator<IExtensionHostProfileService>('extensionHostProfileService');
export const CONTEXT_PROFILE_SESSION_STATE = new RawContextKey<string>('profileSessionState', 'none');
Expand Down Expand Up @@ -120,7 +122,8 @@ export class RuntimeExtensionsEditor extends BaseEditor {
@IExtensionHostProfileService private readonly _extensionHostProfileService: IExtensionHostProfileService,
@IStorageService storageService: IStorageService,
@ILabelService private readonly _labelService: ILabelService,
@IWorkbenchEnvironmentService private readonly _environmentService: IWorkbenchEnvironmentService
@IWorkbenchEnvironmentService private readonly _environmentService: IWorkbenchEnvironmentService,
@IOpenerService private readonly _openerService: IOpenerService
) {
super(RuntimeExtensionsEditor.ID, telemetryService, themeService, storageService);

Expand Down Expand Up @@ -312,7 +315,7 @@ export class RuntimeExtensionsEditor extends BaseEditor {
data.actionbar.push(this._instantiationService.createInstance(SlowExtensionAction, element.description, element.unresponsiveProfile), { icon: true, label: true });
}
if (isNonEmptyArray(element.status.runtimeErrors)) {
data.actionbar.push(new ReportExtensionIssueAction(element), { icon: true, label: true });
data.actionbar.push(new ReportExtensionIssueAction(element, this._openerService), { icon: true, label: true });
}

let title: string;
Expand Down Expand Up @@ -416,7 +419,7 @@ export class RuntimeExtensionsEditor extends BaseEditor {

const actions: IAction[] = [];

actions.push(new ReportExtensionIssueAction(e.element));
actions.push(new ReportExtensionIssueAction(e.element, this._openerService));
actions.push(new Separator());

if (e.element.marketplaceInfo) {
Expand Down Expand Up @@ -480,7 +483,7 @@ export class ReportExtensionIssueAction extends Action {
marketplaceInfo: IExtension;
status?: IExtensionsStatus;
unresponsiveProfile?: IExtensionHostProfile
}) {
}, @IOpenerService private readonly openerService: IOpenerService) {
super(ReportExtensionIssueAction._id, ReportExtensionIssueAction._label, 'extension-action report-issue');
this.enabled = extension.marketplaceInfo
&& extension.marketplaceInfo.type === ExtensionType.User
Expand All @@ -490,7 +493,7 @@ export class ReportExtensionIssueAction extends Action {
}

async run(): Promise<void> {
window.open(this._url);
this.openerService.open(URI.parse(this._url));
}

private static _generateNewIssueUrl(extension: {
Expand Down
8 changes: 5 additions & 3 deletions src/vs/workbench/contrib/feedback/browser/feedback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,15 @@ import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
import { WorkbenchActionExecutedEvent, WorkbenchActionExecutedClassification } from 'vs/base/common/actions';
import { IStatusbarService } from 'vs/platform/statusbar/common/statusbar';
import { IProductService } from 'vs/platform/product/common/product';
import { IOpenerService } from 'vs/platform/opener/common/opener';

export interface IFeedback {
feedback: string;
sentiment: number;
}

export interface IFeedbackDelegate {
submitFeedback(feedback: IFeedback): void;
submitFeedback(feedback: IFeedback, openerService: IOpenerService): void;
getCharacterLimit(sentiment: number): number;
}

Expand Down Expand Up @@ -66,7 +67,8 @@ export class FeedbackDropdown extends Dropdown {
@IIntegrityService private readonly integrityService: IIntegrityService,
@IThemeService private readonly themeService: IThemeService,
@IStatusbarService private readonly statusbarService: IStatusbarService,
@IProductService productService: IProductService
@IProductService productService: IProductService,
@IOpenerService private readonly openerService: IOpenerService
) {
super(container, options);

Expand Down Expand Up @@ -415,7 +417,7 @@ export class FeedbackDropdown extends Dropdown {
this.feedbackDelegate.submitFeedback({
feedback: this.feedbackDescriptionInput.value,
sentiment: this.sentiment
});
}, this.openerService);

this.hide();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import { IWorkbenchContribution } from 'vs/workbench/common/contributions';
import { IStatusbarService, StatusbarAlignment, IStatusbarEntry, IStatusbarEntryAccessor } from 'vs/platform/statusbar/common/statusbar';
import { localize } from 'vs/nls';
import { CommandsRegistry } from 'vs/platform/commands/common/commands';
import { IOpenerService } from 'vs/platform/opener/common/opener';
import { URI } from 'vs/base/common/uri';

class TwitterFeedbackService implements IFeedbackDelegate {

Expand All @@ -23,11 +25,11 @@ class TwitterFeedbackService implements IFeedbackDelegate {
return TwitterFeedbackService.HASHTAGS.join(',');
}

submitFeedback(feedback: IFeedback): void {
submitFeedback(feedback: IFeedback, openerService: IOpenerService): void {
const queryString = `?${feedback.sentiment === 1 ? `hashtags=${this.combineHashTagsAsString()}&` : null}ref_src=twsrc%5Etfw&related=twitterapi%2Ctwitter&text=${encodeURIComponent(feedback.feedback)}&tw_p=tweetbutton&via=${TwitterFeedbackService.VIA_NAME}`;
const url = TwitterFeedbackService.TWITTER_URL + queryString;

window.open(url);
openerService.open(URI.parse(url));
}

getCharacterLimit(sentiment: number): number {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { PerfviewInput } from 'vs/workbench/contrib/performance/electron-browser
import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions';
import { IClipboardService } from 'vs/platform/clipboard/common/clipboardService';
import { URI } from 'vs/base/common/uri';
import { IOpenerService } from 'vs/platform/opener/common/opener';

export class StartupProfiler implements IWorkbenchContribution {

Expand All @@ -28,6 +29,7 @@ export class StartupProfiler implements IWorkbenchContribution {
@IClipboardService private readonly _clipboardService: IClipboardService,
@ILifecycleService lifecycleService: ILifecycleService,
@IExtensionService extensionService: IExtensionService,
@IOpenerService private readonly _openerService: IOpenerService
) {
// wait for everything to be ready
Promise.all([
Expand Down Expand Up @@ -116,6 +118,6 @@ export class StartupProfiler implements IWorkbenchContribution {
const baseUrl = product.reportIssueUrl;
const queryStringPrefix = baseUrl.indexOf('?') === -1 ? '?' : '&';

window.open(`${baseUrl}${queryStringPrefix}body=${encodeURIComponent(body)}`);
this._openerService.open(URI.parse(`${baseUrl}${queryStringPrefix}body=${encodeURIComponent(body)}`));
}
}
4 changes: 3 additions & 1 deletion src/vs/workbench/contrib/search/browser/searchView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ import { ViewletPanel, IViewletPanelOptions } from 'vs/workbench/browser/parts/v
import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
import { Memento, MementoObject } from 'vs/workbench/common/memento';
import { IStorageService, StorageScope } from 'vs/platform/storage/common/storage';
import { IOpenerService } from 'vs/platform/opener/common/opener';

const $ = dom.$;

Expand Down Expand Up @@ -153,6 +154,7 @@ export class SearchView extends ViewletPanel {
@IAccessibilityService private readonly accessibilityService: IAccessibilityService,
@IKeybindingService keybindingService: IKeybindingService,
@IStorageService storageService: IStorageService,
@IOpenerService private readonly openerService: IOpenerService
) {
super({ ...(options as IViewletPanelOptions), id: VIEW_ID, ariaHeaderLabel: nls.localize('searchView', "Search") }, keybindingService, contextMenuService, configurationService, contextKeyService);

Expand Down Expand Up @@ -1473,7 +1475,7 @@ export class SearchView extends ViewletPanel {
private onLearnMore = (e: MouseEvent): void => {
dom.EventHelper.stop(e, false);

window.open('https://go.microsoft.com/fwlink/?linkid=853977');
this.openerService.open(URI.parse('https://go.microsoft.com/fwlink/?linkid=853977'));
}

private updateSearchResultCount(disregardExcludesAndIgnores?: boolean): void {
Expand Down
Loading

0 comments on commit b0d58e4

Please sign in to comment.