Add optionnal external option to WindowService

In a browser, this makes little difference to open a new Theia tab or to open a link. But in Electron, we want to open a BrowserWindow for new Theia workspaces, and to open in the default browser that the user has configured on his machine. Signed-off-by: Paul Maréchal <[email protected]>
eclipse-theia · Nov 27, 2018 · 369dcff · 369dcff
1 parent c109deb
commit 369dcff
Show file tree

Hide file tree

Showing 8 changed files with 39 additions and 12 deletions.
diff --git a/dev-packages/application-manager/src/generator/frontend-generator.ts b/dev-packages/application-manager/src/generator/frontend-generator.ts
@@ -158,6 +158,9 @@ if (isMaster) {
         ipcMain.on('create-new-window', (event, url) => {
             createNewWindow(url);
         });
+        ipcMain.on('open-external', (event, url) => {
+            shell.openExternal(url);
+        });
 
         // Check whether we are in bundled application or development mode.
         // @ts-ignore

diff --git a/packages/core/src/browser/http-open-handler.ts b/packages/core/src/browser/http-open-handler.ts
@@ -14,21 +14,26 @@
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
 
-import { injectable } from 'inversify';
+import { injectable, inject } from 'inversify';
 import URI from '../common/uri';
 import { OpenHandler } from './opener-service';
+import { WindowService } from './window/window-service';
 
 @injectable()
 export class HttpOpenHandler implements OpenHandler {
 
     readonly id = 'http';
 
+    @inject(WindowService)
+    protected readonly windowService: WindowService;
+
     canHandle(uri: URI): number {
         return uri.scheme.startsWith('http') ? 500 : 0;
     }
 
-    open(uri: URI): Window | undefined {
-        return window.open(uri.toString(true)) || undefined;
+    open(uri: URI): undefined {
+        this.windowService.openNewWindow(uri.toString(true), { external: true });
+        return undefined;
     }
 
 }
diff --git a/packages/core/src/browser/storage-service.spec.ts b/packages/core/src/browser/storage-service.spec.ts
@@ -15,6 +15,8 @@
  ********************************************************************************/
 
 import { Container } from 'inversify';
+import { WindowService } from './window/window-service';
+import { MockWindowService } from './window/test/mock-window-service';
 import { LocalStorageService, StorageService } from './storage-service';
 import { expect } from 'chai';
 import { ILogger } from '../common/logger';
@@ -36,6 +38,7 @@ before(() => {
         return logger;
     });
     testContainer.bind(StorageService).to(LocalStorageService).inSingletonScope();
+    testContainer.bind(WindowService).to(MockWindowService).inSingletonScope();
     testContainer.bind(LocalStorageService).toSelf().inSingletonScope();
 
     testContainer.bind(MessageClient).toSelf().inSingletonScope();

diff --git a/packages/core/src/browser/storage-service.ts b/packages/core/src/browser/storage-service.ts
@@ -17,6 +17,7 @@
 import { inject, injectable, postConstruct } from 'inversify';
 import { ILogger } from '../common/logger';
 import { MessageService } from '../common/message-service';
+import { WindowService } from './window/window-service';
 
 export const StorageService = Symbol('IStorageService');
 /**
@@ -44,8 +45,10 @@ interface LocalStorage {
 @injectable()
 export class LocalStorageService implements StorageService {
     private storage: LocalStorage;
+
     @inject(ILogger) protected logger: ILogger;
     @inject(MessageService) protected readonly messageService: MessageService;
+    @inject(WindowService) protected readonly windowService: WindowService;
 
     @postConstruct()
     protected init() {
@@ -93,7 +96,7 @@ export class LocalStorageService implements StorageService {
         your browser's local storage or choose to clear all.`;
         this.messageService.warn(ERROR_MESSAGE, READ_INSTRUCTIONS_ACTION, CLEAR_STORAGE_ACTION).then(async selected => {
             if (READ_INSTRUCTIONS_ACTION === selected) {
-                window.open('https://github.com/theia-ide/theia/wiki/Cleaning-Local-Storage');
+                this.windowService.openNewWindow('https://github.com/theia-ide/theia/wiki/Cleaning-Local-Storage', { external: true });
             } else if (CLEAR_STORAGE_ACTION === selected) {
                 this.clearStorage();
             }

diff --git a/packages/core/src/browser/window/test/mock-window-service.ts b/packages/core/src/browser/window/test/mock-window-service.ts
@@ -18,6 +18,5 @@ import { WindowService } from '../window-service';
 
 @injectable()
 export class MockWindowService implements WindowService {
-
-    openNewWindow(url: string): void { }
+    openNewWindow(): void { }
 }
diff --git a/packages/core/src/browser/window/window-service.ts b/packages/core/src/browser/window/window-service.ts
@@ -16,6 +16,10 @@
 
 import { injectable } from 'inversify';
 
+export interface NewWindowOptions {
+    external?: boolean;
+}
+
 /**
  * Service for opening new browser windows.
  */
@@ -24,8 +28,10 @@ export interface WindowService {
 
     /**
      * Opens a new window and loads the content from the given URL.
+     * In a browser, opening a new Theia tab or open a link is the same thing.
+     * But in Electron, we want to open links in a browser, not in Electron.
      */
-    openNewWindow(url: string): void;
+    openNewWindow(url: string, options?: NewWindowOptions): void;
 
 }
 

diff --git a/packages/core/src/electron-browser/window/electron-window-service.ts b/packages/core/src/electron-browser/window/electron-window-service.ts
@@ -16,13 +16,17 @@
 
 import { injectable } from 'inversify';
 import { ipcRenderer } from 'electron';
-import { DefaultWindowService } from '../../browser/window/window-service';
+import { WindowService, NewWindowOptions } from '../../browser/window/window-service';
 
 @injectable()
-export class ElectronWindowService extends DefaultWindowService {
+export class ElectronWindowService implements WindowService {
 
-    openNewWindow(url: string): void {
-        ipcRenderer.send('create-new-window', url);
+    openNewWindow(url: string, { external }: NewWindowOptions = {}): void {
+        if (external) {
+            ipcRenderer.send('open-external', url);
+        } else {
+            ipcRenderer.send('create-new-window', url);
+        }
     }
 
 }
diff --git a/packages/cpp/src/browser/cpp-language-client-contribution.ts b/packages/cpp/src/browser/cpp-language-client-contribution.ts
@@ -26,6 +26,7 @@ import { MessageService } from '@theia/core/lib/common/message-service';
 import { CPP_LANGUAGE_ID, CPP_LANGUAGE_NAME, HEADER_AND_SOURCE_FILE_EXTENSIONS } from '../common';
 import { CppBuildConfigurationManager, CppBuildConfiguration } from './cpp-build-configurations';
 import { CppBuildConfigurationsStatusBarElement } from './cpp-build-configurations-statusbar-element';
+import { WindowService } from '@theia/core/lib/browser/window/window-service';
 
 /**
  * Clangd extension to set clangd-specific "initializationOptions" in the
@@ -48,6 +49,9 @@ export class CppLanguageClientContribution extends BaseLanguageClientContributio
     @inject(CppBuildConfigurationsStatusBarElement)
     protected readonly cppBuildConfigurationsStatusBarElement: CppBuildConfigurationsStatusBarElement;
 
+    @inject(WindowService)
+    protected readonly windowService: WindowService;
+
     constructor(
         @inject(Workspace) protected readonly workspace: Workspace,
         @inject(Languages) protected readonly languages: Languages,
@@ -117,7 +121,7 @@ export class CppLanguageClientContribution extends BaseLanguageClientContributio
                 'You can refer to the clangd page for instructions.';
             this.messageService.error(ERROR_MESSAGE, READ_INSTRUCTIONS_ACTION).then(selected => {
                 if (READ_INSTRUCTIONS_ACTION === selected) {
-                    window.open('https://clang.llvm.org/extra/clangd.html');
+                    this.windowService.openNewWindow('https://clang.llvm.org/extra/clangd.html', { external: true });
                 }
             });
             this.logger.error(ERROR_MESSAGE);