From 9c783f53ba7ba4158de7c6a48aafc64071b4eebc Mon Sep 17 00:00:00 2001
From: Pedro Pontes <ppontes@gmail.com>
Date: Wed, 19 Dec 2018 07:44:51 +0100
Subject: [PATCH] fix: do not allow the window to grab focus when tabbing /
 shift+tabbing (#16042)

* fix: do not allow the window to grab focus when tabbing / shift+tabbing

* test: add tests.
---
 atom/browser/common_web_contents_delegate.cc  |  13 +++
 atom/browser/common_web_contents_delegate.h   |   1 +
 spec/chromium-spec.js                         | 105 ++++++++++++++++++
 .../pages/tab-focus-loop-elements-wv.html     |  25 +++++
 .../pages/tab-focus-loop-elements.html        |  27 +++++
 5 files changed, 171 insertions(+)
 create mode 100644 spec/fixtures/pages/tab-focus-loop-elements-wv.html
 create mode 100644 spec/fixtures/pages/tab-focus-loop-elements.html

diff --git a/atom/browser/common_web_contents_delegate.cc b/atom/browser/common_web_contents_delegate.cc
index 591013af8b27..01f8c8b735f0 100644
--- a/atom/browser/common_web_contents_delegate.cc
+++ b/atom/browser/common_web_contents_delegate.cc
@@ -346,6 +346,19 @@ blink::WebSecurityStyle CommonWebContentsDelegate::GetSecurityStyle(
                                           security_style_explanations);
 }
 
+bool CommonWebContentsDelegate::TakeFocus(content::WebContents* source,
+                                          bool reverse) {
+  if (source && source->GetOutermostWebContents() == source) {
+    // If this is the outermost web contents and the user has tabbed or
+    // shift + tabbed through all the elements, reset the focus back to
+    // the first or last element so that it doesn't stay in the body.
+    source->FocusThroughTabTraversal(reverse);
+    return true;
+  }
+
+  return false;
+}
+
 void CommonWebContentsDelegate::DevToolsSaveToFile(const std::string& url,
                                                    const std::string& content,
                                                    bool save_as) {
diff --git a/atom/browser/common_web_contents_delegate.h b/atom/browser/common_web_contents_delegate.h
index 96cbee75c225..d4729808ee8d 100644
--- a/atom/browser/common_web_contents_delegate.h
+++ b/atom/browser/common_web_contents_delegate.h
@@ -98,6 +98,7 @@ class CommonWebContentsDelegate : public content::WebContentsDelegate,
   blink::WebSecurityStyle GetSecurityStyle(
       content::WebContents* web_contents,
       content::SecurityStyleExplanations* explanations) override;
+  bool TakeFocus(content::WebContents* source, bool reverse) override;
   void HandleKeyboardEvent(
       content::WebContents* source,
       const content::NativeWebKeyboardEvent& event) override;
diff --git a/spec/chromium-spec.js b/spec/chromium-spec.js
index be86cd8d6daa..7f7b9cb9242a 100644
--- a/spec/chromium-spec.js
+++ b/spec/chromium-spec.js
@@ -1410,6 +1410,111 @@ describe('chromium feature', () => {
       await new Promise((resolve) => { utter.onend = resolve })
     })
   })
+
+  describe('focus handling', () => {
+    let webviewContents = null
+
+    beforeEach(async () => {
+      w = new BrowserWindow({
+        show: true
+      })
+
+      const webviewReady = emittedOnce(w.webContents, 'did-attach-webview')
+      await w.loadFile(path.join(fixtures, 'pages', 'tab-focus-loop-elements.html'))
+      const [, wvContents] = await webviewReady
+      webviewContents = wvContents
+      await emittedOnce(webviewContents, 'did-finish-load')
+      w.focus()
+    })
+
+    afterEach(() => {
+      webviewContents = null
+    })
+
+    const expectFocusChange = async () => {
+      const [, focusedElementId] = await emittedOnce(ipcMain, 'focus-changed')
+      return focusedElementId
+    }
+
+    describe('a TAB press', () => {
+      const tabPressEvent = {
+        type: 'keyDown',
+        keyCode: 'Tab'
+      }
+
+      it('moves focus to the next focusable item', async () => {
+        let focusChange = expectFocusChange()
+        w.webContents.sendInputEvent(tabPressEvent)
+        let focusedElementId = await focusChange
+        assert.strictEqual(focusedElementId, 'BUTTON-element-1', `should start focused in element-1, it's instead in ${focusedElementId}`)
+
+        focusChange = expectFocusChange()
+        w.webContents.sendInputEvent(tabPressEvent)
+        focusedElementId = await focusChange
+        assert.strictEqual(focusedElementId, 'BUTTON-element-2', `focus should've moved to element-2, it's instead in ${focusedElementId}`)
+
+        focusChange = expectFocusChange()
+        w.webContents.sendInputEvent(tabPressEvent)
+        focusedElementId = await focusChange
+        assert.strictEqual(focusedElementId, 'BUTTON-wv-element-1', `focus should've moved to the webview's element-1, it's instead in ${focusedElementId}`)
+
+        focusChange = expectFocusChange()
+        webviewContents.sendInputEvent(tabPressEvent)
+        focusedElementId = await focusChange
+        assert.strictEqual(focusedElementId, 'BUTTON-wv-element-2', `focus should've moved to the webview's element-2, it's instead in ${focusedElementId}`)
+
+        focusChange = expectFocusChange()
+        webviewContents.sendInputEvent(tabPressEvent)
+        focusedElementId = await focusChange
+        assert.strictEqual(focusedElementId, 'BUTTON-element-3', `focus should've moved to element-3, it's instead in ${focusedElementId}`)
+
+        focusChange = expectFocusChange()
+        w.webContents.sendInputEvent(tabPressEvent)
+        focusedElementId = await focusChange
+        assert.strictEqual(focusedElementId, 'BUTTON-element-1', `focus should've looped back to element-1, it's instead in ${focusedElementId}`)
+      })
+    })
+
+    describe('a SHIFT + TAB press', () => {
+      const shiftTabPressEvent = {
+        type: 'keyDown',
+        modifiers: ['Shift'],
+        keyCode: 'Tab'
+      }
+
+      it('moves focus to the previous focusable item', async () => {
+        let focusChange = expectFocusChange()
+        w.webContents.sendInputEvent(shiftTabPressEvent)
+        let focusedElementId = await focusChange
+        assert.strictEqual(focusedElementId, 'BUTTON-element-3', `should start focused in element-3, it's instead in ${focusedElementId}`)
+
+        focusChange = expectFocusChange()
+        w.webContents.sendInputEvent(shiftTabPressEvent)
+        focusedElementId = await focusChange
+        assert.strictEqual(focusedElementId, 'BUTTON-wv-element-2', `focus should've moved to the webview's element-2, it's instead in ${focusedElementId}`)
+
+        focusChange = expectFocusChange()
+        webviewContents.sendInputEvent(shiftTabPressEvent)
+        focusedElementId = await focusChange
+        assert.strictEqual(focusedElementId, 'BUTTON-wv-element-1', `focus should've moved to the webview's element-1, it's instead in ${focusedElementId}`)
+
+        focusChange = expectFocusChange()
+        webviewContents.sendInputEvent(shiftTabPressEvent)
+        focusedElementId = await focusChange
+        assert.strictEqual(focusedElementId, 'BUTTON-element-2', `focus should've moved to element-2, it's instead in ${focusedElementId}`)
+
+        focusChange = expectFocusChange()
+        w.webContents.sendInputEvent(shiftTabPressEvent)
+        focusedElementId = await focusChange
+        assert.strictEqual(focusedElementId, 'BUTTON-element-1', `focus should've moved to element-1, it's instead in ${focusedElementId}`)
+
+        focusChange = expectFocusChange()
+        w.webContents.sendInputEvent(shiftTabPressEvent)
+        focusedElementId = await focusChange
+        assert.strictEqual(focusedElementId, 'BUTTON-element-3', `focus should've looped back to element-3, it's instead in ${focusedElementId}`)
+      })
+    })
+  })
 })
 
 describe('font fallback', () => {
diff --git a/spec/fixtures/pages/tab-focus-loop-elements-wv.html b/spec/fixtures/pages/tab-focus-loop-elements-wv.html
new file mode 100644
index 000000000000..b2daec0fc17e
--- /dev/null
+++ b/spec/fixtures/pages/tab-focus-loop-elements-wv.html
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title></title>
+  <body>
+    <div>
+      <button id="wv-element-1">Button 1</button>
+      <button id="wv-element-2">Button 2</button>
+    </div>
+    <script>
+      const { ipcRenderer } = require('electron')
+
+      function handleFocusChange(event) {
+        if (event.target.tagName) {
+          const elementId = event.target.id ? `-${event.target.id}` : ''
+          const elementIdentifier = `${event.target.tagName}${elementId}`
+          ipcRenderer.send('focus-changed', elementIdentifier)
+        }
+      }
+
+      addEventListener('focus', handleFocusChange, true)
+    </script>
+  </body>
+</html>
diff --git a/spec/fixtures/pages/tab-focus-loop-elements.html b/spec/fixtures/pages/tab-focus-loop-elements.html
new file mode 100644
index 000000000000..b10b6cba8079
--- /dev/null
+++ b/spec/fixtures/pages/tab-focus-loop-elements.html
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title></title>
+  <script>
+    const { ipcRenderer } = require('electron')
+
+    function handleFocusChange(event) {
+      if (event.target.tagName && event.target.tagName !== 'WEBVIEW') {
+        const elementId = event.target.id ? `-${event.target.id}` : ''
+        const elementIdentifier = `${event.target.tagName}${elementId}`
+        ipcRenderer.send('focus-changed', elementIdentifier)
+      }
+    }
+
+    addEventListener('focus', handleFocusChange, true)
+  </script>
+  <body>
+    <div>
+      <button id="element-1">Button 1</button>
+      <button id="element-2">Button 2</button>
+      <webview src="tab-focus-loop-elements-wv.html" nodeintegration></webview>
+      <button id="element-3">Button 3</button>
+    </div>
+  </body>
+</html>