Browser: Fix scroll key binding conflict (#2239)

* Check for webview focus before running command on webview

* Fix browser path

* Hook up active tag management for the browser layer

* Fix lint issues

browser/src/Editor/BufferManager.ts

@@ -43,8 +43,14 @@ import { TokenColor } from "./../Services/TokenColors"
 
 import { IBufferLayer } from "./NeovimEditor/BufferLayerManager"
 
+/**
+ * Candidate API methods
+ */
 export interface IBuffer extends Oni.Buffer {
     setLanguage(lang: string): Promise<void>
+
+    getLayerById<T>(id: string): T
+
     getCursorPosition(): Promise<types.Position>
     handleInput(key: string): boolean
     detectIndentation(): Promise<BufferIndentationInfo>
@@ -143,10 +149,12 @@ export class Buffer implements IBuffer {
         this._actions.addBufferLayer(parseInt(this._id, 10), layer)
     }
 
-    public getLayerById<T>(id: string): T {
-        return (this._store
-            .getState()
-            .layers[parseInt(this._id, 10)].find(layer => layer.id === id) as any) as T
+    public getLayerById<T>(id: string): T | null {
+        return (
+            ((this._store
+                .getState()
+                .layers[parseInt(this._id, 10)].find(layer => layer.id === id) as any) as T) || null
+        )
     }
 
     public removeLayer(layer: IBufferLayer): void {

browser/src/Services/Browser/BrowserView.tsx

@@ -69,6 +69,9 @@ export interface IBrowserViewProps {
     scrollDown: IEvent<void>
     scrollLeft: IEvent<void>
     scrollRight: IEvent<void>
+
+    webviewRef?: (webviewTag: WebviewTag) => void
+    onFocusTag?: (tagName: string | null) => void
 }
 
 export interface IBrowserViewState {
@@ -324,6 +327,24 @@ export class BrowserView extends React.PureComponent<IBrowserViewProps, IBrowser
             this._webviewElement.addEventListener("blur", () => {
                 focusManager.popFocus(this._webviewElement)
             })
+
+            this._webviewElement.addEventListener("ipc-message", event => {
+                switch (event.channel) {
+                    case "focusin":
+                        if (this.props.onFocusTag) {
+                            this.props.onFocusTag(event.args[0])
+                        }
+                        return
+                    case "focusout":
+                        if (this.props.onFocusTag) {
+                            this.props.onFocusTag(null)
+                        }
+                }
+            })
+
+            if (this.props.webviewRef) {
+                this.props.webviewRef(this._webviewElement)
+            }
         }
     }
 }

browser/src/Services/Browser/index.tsx

@@ -4,15 +4,18 @@
  * Entry point for browser integration plugin
  */
 
-import { shell } from "electron"
+import { shell, WebviewTag } from "electron"
 import * as React from "react"
 
 import * as Oni from "oni-api"
 import { Event } from "oni-types"
 
+import { IBuffer } from "./../../Editor/BufferManager"
+
 import { CommandManager } from "./../CommandManager"
 import { Configuration } from "./../Configuration"
 import { EditorManager } from "./../EditorManager"
+import { focusManager } from "./../FocusManager"
 import {
     AchievementsManager,
     getInstance as getAchievementsInstance,
@@ -30,12 +33,23 @@ export class BrowserLayer implements Oni.BufferLayer {
     private _scrollRightEvent = new Event<void>()
     private _scrollLeftEvent = new Event<void>()
 
+    private _webview: WebviewTag | null = null
+    private _activeTagName: string | null = null
+
     constructor(private _url: string, private _configuration: Configuration) {}
 
     public get id(): string {
         return "oni.browser"
     }
 
+    public get webviewElement(): HTMLElement {
+        return this._webview
+    }
+
+    public get activeTagName(): string {
+        return this._activeTagName
+    }
+
     public render(): JSX.Element {
         return (
             <BrowserView
@@ -49,6 +63,8 @@ export class BrowserLayer implements Oni.BufferLayer {
                 scrollUp={this._scrollUpEvent}
                 scrollLeft={this._scrollLeftEvent}
                 scrollRight={this._scrollRightEvent}
+                webviewRef={webview => (this._webview = webview)}
+                onFocusTag={newTag => (this._activeTagName = newTag)}
             />
         )
     }
@@ -92,8 +108,6 @@ export const activate = (
 ) => {
     let count = 0
 
-    const activeLayers: { [bufferId: string]: BrowserLayer } = {}
-
     const browserEnabledSetting = configuration.registerSetting("browser.enabled", {
         requiresReload: false,
         description:
@@ -128,7 +142,6 @@ export const activate = (
 
             const layer = new BrowserLayer(url, configuration)
             buffer.addLayer(layer)
-            activeLayers[buffer.id] = layer
 
             const achievements = getAchievementsInstance()
             achievements.notifyGoal("oni.goal.openBrowser")
@@ -160,82 +173,122 @@ export const activate = (
         detail: null,
     })
 
+    const getLayerForBuffer = (buffer: Oni.Buffer): BrowserLayer => {
+        return (buffer as IBuffer).getLayerById<BrowserLayer>("oni.browser")
+    }
+
     const executeCommandForLayer = (callback: (browserLayer: BrowserLayer) => void) => () => {
         const activeBuffer = editorManager.activeEditor.activeBuffer
 
-        const browserLayer = activeLayers[activeBuffer.id]
+        const browserLayer = getLayerForBuffer(activeBuffer)
         if (browserLayer) {
             callback(browserLayer)
         }
     }
 
-    const isBrowserLayerActive = () =>
-        !!activeLayers[editorManager.activeEditor.activeBuffer.id] &&
-        browserEnabledSetting.getValue()
+    const isBrowserCommandEnabled = (): boolean => {
+        if (!browserEnabledSetting.getValue()) {
+            return false
+        }
+
+        const layer = getLayerForBuffer(editorManager.activeEditor.activeBuffer)
+        if (!layer) {
+            return false
+        }
+
+        // If the layer is open, but not focused, we shouldn't execute commands.
+        // This could happen if there is a pop-up menu, or if we're working with some
+        // non-webview UI in the browser (like the address bar)
+        if (layer.webviewElement !== focusManager.focusedElement) {
+            return false
+        }
+
+        return true
+    }
+
+    const isInputTag = (tagName: string): boolean => {
+        return tagName === "INPUT" || tagName === "TEXTAREA"
+    }
+
+    const isBrowserScrollCommandEnabled = (): boolean => {
+        if (!isBrowserCommandEnabled()) {
+            return false
+        }
+
+        const layer = getLayerForBuffer(editorManager.activeEditor.activeBuffer)
+
+        // Finally, if the webview _is_ focused, but something has focus, we'll
+        // skip our bindings and defer to the browser
+        if (isInputTag(layer.activeTagName)) {
+            return false
+        }
+
+        return true
+    }
 
     // Per-layer commands
     commandManager.registerCommand({
         command: "browser.debug",
         execute: executeCommandForLayer(browser => browser.openDebugger()),
         name: "Browser: Open DevTools",
         detail: "Open the devtools pane for the current browser window.",
-        enabled: isBrowserLayerActive,
+        enabled: isBrowserCommandEnabled,
     })
 
     commandManager.registerCommand({
         command: "browser.goBack",
         execute: executeCommandForLayer(browser => browser.goBack()),
         name: "Browser: Go back",
         detail: "",
-        enabled: isBrowserLayerActive,
+        enabled: isBrowserCommandEnabled,
     })
 
     commandManager.registerCommand({
         command: "browser.goForward",
         execute: executeCommandForLayer(browser => browser.goForward()),
         name: "Browser: Go forward",
         detail: "",
-        enabled: isBrowserLayerActive,
+        enabled: isBrowserCommandEnabled,
     })
 
     commandManager.registerCommand({
         command: "browser.reload",
         execute: executeCommandForLayer(browser => browser.reload()),
         name: "Browser: Reload",
         detail: "",
-        enabled: isBrowserLayerActive,
+        enabled: isBrowserCommandEnabled,
     })
 
     commandManager.registerCommand({
         command: "browser.scrollDown",
         execute: executeCommandForLayer(browser => browser.scrollDown()),
         name: "Browser: Scroll Down",
         detail: "",
-        enabled: isBrowserLayerActive,
+        enabled: isBrowserScrollCommandEnabled,
     })
 
     commandManager.registerCommand({
         command: "browser.scrollUp",
         execute: executeCommandForLayer(browser => browser.scrollUp()),
         name: "Browser: Scroll Up",
         detail: "",
-        enabled: isBrowserLayerActive,
+        enabled: isBrowserScrollCommandEnabled,
     })
 
     commandManager.registerCommand({
         command: "browser.scrollLeft",
         execute: executeCommandForLayer(browser => browser.scrollLeft()),
         name: "Browser: Scroll Left",
         detail: "",
-        enabled: isBrowserLayerActive,
+        enabled: isBrowserScrollCommandEnabled,
     })
 
     commandManager.registerCommand({
         command: "browser.scrollRight",
         execute: executeCommandForLayer(browser => browser.scrollRight()),
         name: "Browser: Scroll Right",
         detail: "",
-        enabled: isBrowserLayerActive,
+        enabled: isBrowserScrollCommandEnabled,
     })
 }
 

browser/src/Services/FocusManager.ts

@@ -7,6 +7,10 @@ import * as Log from "oni-core-logging"
 class FocusManager {
     private _focusElementStack: HTMLElement[] = []
 
+    public get focusedElement(): HTMLElement | null {
+        return this._focusElementStack.length > 0 ? this._focusElementStack[0] : null
+    }
+
     public pushFocus(element: HTMLElement) {
         this._focusElementStack = [element, ...this._focusElementStack]
 

test/CiTests.ts

@@ -8,6 +8,7 @@ import * as mkdirp from "mkdirp"
 import { IFailedTest, Oni, runInProcTest } from "./common"
 
 const LongTimeout = 5000
+
 const CiTests = [
     // Core functionality tests
     "Api.Buffer.AddLayer",
@@ -17,6 +18,8 @@ const CiTests = [
     "AutoCompletionTest-HTML",
     "AutoCompletionTest-TypeScript",
 
+    "Browser.LocationTest",
+
     "Configuration.JavaScriptEditorTest",
     "Configuration.TypeScriptEditor.NewConfigurationTest",
     "Configuration.TypeScriptEditor.CompletionTest",
@@ -87,7 +90,9 @@ const FGYELLOW = "\x1b[33m"
 describe("ci tests", function() {
     const tests = Platform.isWindows()
         ? [...CiTests, ...WindowsOnlyTests]
-        : Platform.isMac() ? [...CiTests, ...OSXOnlyTests] : CiTests
+        : Platform.isMac()
+            ? [...CiTests, ...OSXOnlyTests]
+            : CiTests
 
     const testFailures: IFailedTest[] = []
     tests.forEach(test => {

test/ci/Browser.LocationTest.ts

@@ -0,0 +1,64 @@
+/**
+ * Test scripts for Auto Complete for a Typescript file.
+ */
+
+import * as assert from "assert"
+
+import * as Oni from "oni-api"
+
+import { WebviewTag } from "electron"
+
+import { getElementsBySelector } from "./Common"
+
+export const test = async (oni: Oni.Plugin.Api) => {
+    await oni.automation.waitForEditors()
+
+    const getWebView = (): WebviewTag | null => {
+        const elems = getElementsBySelector("webview")
+        return elems.length > 0 ? elems[0] : null
+    }
+
+    const waitForWebViewUrl = (urlPart: string): boolean => {
+        const webview = getWebView()
+
+        if (!webview) {
+            return false
+        }
+
+        const url = webview.getURL()
+
+        return url.indexOf(urlPart) >= 0
+    }
+
+    oni.commands.executeCommand("browser.openUrl.verticalSplit", "https://github.com/onivim/oni")
+
+    await oni.automation.waitFor(() => getWebView() !== null)
+    await oni.automation.waitFor(() => waitForWebViewUrl("github.com"))
+
+    await oni.automation.sendKeys("<c-g>")
+    await oni.automation.sleep(500)
+
+    // We'll sneak to the browser address and load a new site
+    const anyOni = oni as any
+    const sneak = anyOni.sneak.getSneakMatchingTag("browser.address")
+
+    const keys: string = sneak.triggerKeys.toLowerCase()
+    await anyOni.automation.sendKeysV2(keys)
+
+    await oni.automation.sleep(500)
+
+    await anyOni.automation.sendKeysV2("https://www.onivim.io")
+
+    await oni.automation.sleep(500)
+
+    await anyOni.automation.sendKeysV2("<CR>")
+
+    await oni.automation.waitFor(() => waitForWebViewUrl("onivim.io"))
+
+    assert.ok(
+        getWebView()
+            .getURL()
+            .indexOf("onivim.io") >= 0,
+        "Successfully navigated to onivim.io",
+    )
+}

webview_preload/src/index.ts

@@ -5,9 +5,21 @@
  * https://electronjs.org/docs/api/webview-tag#preload
  */
 
+declare var require: any
 ;(() => {
     const __oni_win: any = window
 
+    const { ipcRenderer } = require("electron")
+
+    window.document.addEventListener("focusin", evt => {
+        const target = evt.target as HTMLElement
+        ipcRenderer.sendToHost("focusin", target ? target.tagName : null)
+    })
+
+    window.document.addEventListener("focusout", evt => {
+        ipcRenderer.sendToHost("focusout")
+    })
+
     interface Rectangle {
         x: number
         y: number