From ddac4a99bf89ec6d2db001073008b85bd168c45c Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Wed, 12 May 2021 11:42:22 -0700 Subject: [PATCH 01/14] fix: adjust initial webContents focus calculation --- lib/browser/api/browser-window.ts | 14 --------- .../browser/api/electron_api_web_contents.cc | 31 ++++++++++++++----- shell/browser/api/electron_api_web_contents.h | 5 ++- typings/internal-electron.d.ts | 1 - 4 files changed, 25 insertions(+), 26 deletions(-) diff --git a/lib/browser/api/browser-window.ts b/lib/browser/api/browser-window.ts index d01be9c795f0f..62630bfffb2f5 100644 --- a/lib/browser/api/browser-window.ts +++ b/lib/browser/api/browser-window.ts @@ -20,20 +20,6 @@ BrowserWindow.prototype._init = function (this: BWT) { nativeSetBounds.call(this, bounds, ...opts); }; - // Sometimes the webContents doesn't get focus when window is shown, so we - // have to force focusing on webContents in this case. The safest way is to - // focus it when we first start to load URL, if we do it earlier it won't - // have effect, if we do it later we might move focus in the page. - // - // Though this hack is only needed on macOS when the app is launched from - // Finder, we still do it on all platforms in case of other bugs we don't - // know. - if (this.webContents._initiallyShown) { - this.webContents.once('load-url' as any, function (this: WebContents) { - this.focus(); - }); - } - // Redirect focus/blur event to app instance too. this.on('blur', (event: Event) => { app.emit('browser-window-blur', event, this); diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc index 1f390e16b82ff..54108698ccd2c 100644 --- a/shell/browser/api/electron_api_web_contents.cc +++ b/shell/browser/api/electron_api_web_contents.cc @@ -702,8 +702,8 @@ WebContents::WebContents(v8::Isolate* isolate, // BrowserViews are not attached to a window initially so they should start // off as hidden. This is also important for compositor recycling. See: // https://github.com/electron/electron/pull/21372 - initially_shown_ = type_ != Type::kBrowserView; - options.Get(options::kShow, &initially_shown_); + bool initially_shown = type_ != Type::kBrowserView; + options.Get(options::kShow, &initially_shown); // Obtain the session. std::string partition; @@ -759,7 +759,7 @@ WebContents::WebContents(v8::Isolate* isolate, #endif } else { content::WebContents::CreateParams params(session->browser_context()); - params.initially_hidden = !initially_shown_; + params.initially_hidden = !initially_shown; web_contents = content::WebContents::Create(params); } @@ -1655,6 +1655,26 @@ void WebContents::DidRedirectNavigation( EmitNavigationEvent("did-redirect-navigation", navigation_handle); } +void WebContents::ReadyToCommitNavigation( + content::NavigationHandle* navigation_handle) { + // Don't focus content in an inactive window. + if (!owner_window()) + return; + if (!owner_window()->widget()->IsActive()) + return; + // Don't focus content after subframe navigations. + if (!navigation_handle->IsInMainFrame()) + return; + // Only focus for top-level contents. + if (type_ != Type::kBrowserWindow) + return; + // Only set the initial focus when navigating away from the + // blank page. + if (web_contents()->GetLastCommittedURL() != "about:blank") + return; + web_contents()->SetInitialFocus(); +} + void WebContents::DidFinishNavigation( content::NavigationHandle* navigation_handle) { if (!navigation_handle->HasCommitted()) @@ -3126,10 +3146,6 @@ v8::Local WebContents::Debugger(v8::Isolate* isolate) { return v8::Local::New(isolate, debugger_); } -bool WebContents::WasInitiallyShown() { - return initially_shown_; -} - content::RenderFrameHost* WebContents::MainFrame() { return web_contents()->GetMainFrame(); } @@ -3752,7 +3768,6 @@ v8::Local WebContents::FillObjectTemplate( .SetProperty("hostWebContents", &WebContents::HostWebContents) .SetProperty("devToolsWebContents", &WebContents::DevToolsWebContents) .SetProperty("debugger", &WebContents::Debugger) - .SetProperty("_initiallyShown", &WebContents::WasInitiallyShown) .SetProperty("mainFrame", &WebContents::MainFrame) .Build(); } diff --git a/shell/browser/api/electron_api_web_contents.h b/shell/browser/api/electron_api_web_contents.h index c8f0961651bf1..3f87f8b09dacd 100644 --- a/shell/browser/api/electron_api_web_contents.h +++ b/shell/browser/api/electron_api_web_contents.h @@ -330,7 +330,6 @@ class WebContents : public gin::Wrappable, content::WebContents* HostWebContents() const; v8::Local DevToolsWebContents(v8::Isolate* isolate); v8::Local Debugger(v8::Isolate* isolate); - bool WasInitiallyShown(); content::RenderFrameHost* MainFrame(); WebContentsZoomController* GetZoomController() { return zoom_controller_; } @@ -567,6 +566,8 @@ class WebContents : public gin::Wrappable, content::NavigationHandle* navigation_handle) override; void DidRedirectNavigation( content::NavigationHandle* navigation_handle) override; + void ReadyToCommitNavigation( + content::NavigationHandle* navigation_handle) override; void DidFinishNavigation( content::NavigationHandle* navigation_handle) override; bool OnMessageReceived(const IPC::Message& message) override; @@ -735,8 +736,6 @@ class WebContents : public gin::Wrappable, v8::Global pending_child_web_preferences_; - bool initially_shown_ = true; - // The window that this WebContents belongs to. base::WeakPtr owner_window_; diff --git a/typings/internal-electron.d.ts b/typings/internal-electron.d.ts index 91b4b4a1f4f31..38cd6cd17ec1b 100644 --- a/typings/internal-electron.d.ts +++ b/typings/internal-electron.d.ts @@ -67,7 +67,6 @@ declare namespace Electron { getLastWebPreferences(): Electron.WebPreferences; _getPreloadPaths(): string[]; equal(other: WebContents): boolean; - _initiallyShown: boolean; browserWindowOptions: BrowserWindowConstructorOptions; _windowOpenHandler: ((details: Electron.HandlerDetails) => any) | null; _callWindowOpenHandler(event: any, details: Electron.HandlerDetails): Electron.BrowserWindowConstructorOptions | null; From ca95653279b0005111437c5ca85ab95ba2d27a58 Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Tue, 18 May 2021 07:47:28 -0700 Subject: [PATCH 02/14] fix: active window check on mac --- shell/browser/api/electron_api_browser_window.cc | 1 + shell/browser/api/electron_api_web_contents.cc | 9 +++++---- shell/browser/native_window.h | 4 ++++ shell/browser/native_window_mac.h | 3 +++ shell/browser/native_window_mac.mm | 10 ++++++++++ spec-main/chromium-spec.ts | 1 - 6 files changed, 23 insertions(+), 5 deletions(-) diff --git a/shell/browser/api/electron_api_browser_window.cc b/shell/browser/api/electron_api_browser_window.cc index d9df95019a6d0..9817bb70b8dbd 100644 --- a/shell/browser/api/electron_api_browser_window.cc +++ b/shell/browser/api/electron_api_browser_window.cc @@ -301,6 +301,7 @@ void BrowserWindow::OnWindowIsKeyChanged(bool is_key) { auto* rwhv = web_contents()->GetRenderWidgetHostView(); if (rwhv) rwhv->SetActive(is_key); + window()->SetActive(is_key); #endif } diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc index 54108698ccd2c..168291c9c211e 100644 --- a/shell/browser/api/electron_api_web_contents.cc +++ b/shell/browser/api/electron_api_web_contents.cc @@ -1660,18 +1660,19 @@ void WebContents::ReadyToCommitNavigation( // Don't focus content in an inactive window. if (!owner_window()) return; +#if defined(OS_MAC) + if (!owner_window()->IsActive()) + return; +#else if (!owner_window()->widget()->IsActive()) return; +#endif // Don't focus content after subframe navigations. if (!navigation_handle->IsInMainFrame()) return; // Only focus for top-level contents. if (type_ != Type::kBrowserWindow) return; - // Only set the initial focus when navigating away from the - // blank page. - if (web_contents()->GetLastCommittedURL() != "about:blank") - return; web_contents()->SetInitialFocus(); } diff --git a/shell/browser/native_window.h b/shell/browser/native_window.h index 6314cc4318401..0ac798328e556 100644 --- a/shell/browser/native_window.h +++ b/shell/browser/native_window.h @@ -135,6 +135,10 @@ class NativeWindow : public base::SupportsUserData, virtual void Invalidate() = 0; virtual void SetTitle(const std::string& title) = 0; virtual std::string GetTitle() = 0; +#if defined(OS_MAC) + virtual void SetActive(bool is_key) = 0; + virtual bool IsActive() const = 0; +#endif // Ability to augment the window title for the screen readers. void SetAccessibleTitle(const std::string& title); diff --git a/shell/browser/native_window_mac.h b/shell/browser/native_window_mac.h index c8c4fe392cc4e..c0ba103985713 100644 --- a/shell/browser/native_window_mac.h +++ b/shell/browser/native_window_mac.h @@ -149,6 +149,8 @@ class NativeWindowMac : public NativeWindow, gfx::Rect WindowBoundsToContentBounds(const gfx::Rect& bounds) const override; void NotifyWindowEnterFullScreen() override; void NotifyWindowLeaveFullScreen() override; + void SetActive(bool is_key) override; + bool IsActive() const override; void NotifyWindowWillEnterFullScreen(); void NotifyWindowWillLeaveFullScreen(); @@ -270,6 +272,7 @@ class NativeWindowMac : public NativeWindow, bool is_simple_fullscreen_ = false; bool was_maximizable_ = false; bool was_movable_ = false; + bool is_active_ = false; NSRect original_frame_; NSInteger original_level_; NSUInteger simple_fullscreen_mask_; diff --git a/shell/browser/native_window_mac.mm b/shell/browser/native_window_mac.mm index 46be083762ea1..d4eb6b608aab2 100644 --- a/shell/browser/native_window_mac.mm +++ b/shell/browser/native_window_mac.mm @@ -1671,6 +1671,16 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) { UpdateVibrancyRadii(false); } +void NativeWindowMac::SetActive(bool is_key) { + if (is_key) + widget()->Activate(); + is_active_ = is_key; +} + +bool NativeWindowMac::IsActive() const { + return is_active_; +} + void NativeWindowMac::ReorderButtonsView() { if (buttons_view_) { [buttons_view_ removeFromSuperview]; diff --git a/spec-main/chromium-spec.ts b/spec-main/chromium-spec.ts index 4075569d61c83..cf29201b4d728 100644 --- a/spec-main/chromium-spec.ts +++ b/spec-main/chromium-spec.ts @@ -1665,7 +1665,6 @@ describe('navigator.clipboard', () => { let w: BrowserWindow; before(async () => { w = new BrowserWindow({ - show: false, webPreferences: { enableBlinkFeatures: 'Serial' } From 6743e852e1112b90c3abd99a7524997c2678ae36 Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Tue, 18 May 2021 22:45:03 -0700 Subject: [PATCH 03/14] fix: about:blank focus behavior --- spec-main/api-web-contents-spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec-main/api-web-contents-spec.ts b/spec-main/api-web-contents-spec.ts index 240c3a41e696b..6ca16511e414a 100644 --- a/spec-main/api-web-contents-spec.ts +++ b/spec-main/api-web-contents-spec.ts @@ -392,7 +392,7 @@ describe('webContents module', () => { const testFn = (process.platform === 'win32' && process.arch === 'arm64' ? it.skip : it); testFn('returns the focused web contents', async () => { const w = new BrowserWindow({ show: true }); - await w.loadURL('about:blank'); + await w.loadFile(path.join(__dirname, 'fixtures', 'blank.html')); expect(webContents.getFocusedWebContents().id).to.equal(w.webContents.id); const devToolsOpened = emittedOnce(w.webContents, 'devtools-opened'); From 6cf28806ca6133b412349cbce724d9ec2f9ab3fd Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Tue, 18 May 2021 22:45:23 -0700 Subject: [PATCH 04/14] chore: add spec --- spec-main/chromium-spec.ts | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/spec-main/chromium-spec.ts b/spec-main/chromium-spec.ts index cf29201b4d728..e0c50eddcc22e 100644 --- a/spec-main/chromium-spec.ts +++ b/spec-main/chromium-spec.ts @@ -1481,6 +1481,31 @@ describe('chromium features', () => { expect(pageExists).to.be.true(); }); }); + + describe('document.hasFocus', () => { + it('has correct value when multiple windows are opened', async () => { + const w1 = new BrowserWindow({ show: true }); + const w2 = new BrowserWindow({ show: true }); + const w3 = new BrowserWindow({ show: false }); + await w1.loadFile(path.join(__dirname, 'fixtures', 'blank.html')); + await w2.loadFile(path.join(__dirname, 'fixtures', 'blank.html')); + await w3.loadFile(path.join(__dirname, 'fixtures', 'blank.html')); + expect(webContents.getFocusedWebContents().id).to.equal(w2.webContents.id); + let focus = false; + focus = await w1.webContents.executeJavaScript( + 'document.hasFocus()' + ); + expect(focus).to.be.false(); + focus = await w2.webContents.executeJavaScript( + 'document.hasFocus()' + ); + expect(focus).to.be.true(); + focus = await w3.webContents.executeJavaScript( + 'document.hasFocus()' + ); + expect(focus).to.be.false(); + }); + }); }); describe('font fallback', () => { From 0afceccdb82b2f40e278e3d38861d4d2f932e34f Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Wed, 23 Jun 2021 00:19:19 -0700 Subject: [PATCH 05/14] fix: window ordering on mac --- shell/browser/native_window_mac.mm | 2 -- spec-main/api-browser-window-spec.ts | 12 ++++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/shell/browser/native_window_mac.mm b/shell/browser/native_window_mac.mm index d4eb6b608aab2..a93ca7ae2fa75 100644 --- a/shell/browser/native_window_mac.mm +++ b/shell/browser/native_window_mac.mm @@ -1672,8 +1672,6 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) { } void NativeWindowMac::SetActive(bool is_key) { - if (is_key) - widget()->Activate(); is_active_ = is_key; } diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts index bda6c8c1c3886..5467ff777e39f 100644 --- a/spec-main/api-browser-window-spec.ts +++ b/spec-main/api-browser-window-spec.ts @@ -4311,6 +4311,18 @@ describe('BrowserWindow module', () => { await leaveFullScreen; expect(w.isFullScreen()).to.be.false('isFullScreen'); }); + + it('multiple windows inherit correct fullscreen state', async () => { + const w = new BrowserWindow(); + const enterFullScreen = emittedOnce(w, 'enter-full-screen'); + w.setFullScreen(true); + await enterFullScreen; + expect(w.isFullScreen()).to.be.true('isFullScreen'); + await delay(); + const w2 = new BrowserWindow(); + await delay(); + expect(w2.isFullScreen()).to.be.true('isFullScreen'); + }); }); describe('closable state', () => { From abff3c87b96c4d4c4f2db31ad7a331d402287163 Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Thu, 1 Jul 2021 00:50:55 -0700 Subject: [PATCH 06/14] chore: focus after navigation --- spec/webview-spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/webview-spec.js b/spec/webview-spec.js index cbe2989e0c92c..a094cc69e386f 100644 --- a/spec/webview-spec.js +++ b/spec/webview-spec.js @@ -977,12 +977,12 @@ describe(' tag', function () { it('emits when a request is made', async () => { const didFinishLoad = waitForEvent(webview, 'did-finish-load'); loadWebView(webview, { src: `file://${fixtures}/pages/content.html` }); + await didFinishLoad; // TODO(deepak1556): With https://codereview.chromium.org/2836973002 // focus of the webContents is required when triggering the api. // Remove this workaround after determining the cause for // incorrect focus. webview.focus(); - await didFinishLoad; const activeMatchOrdinal = []; From ece14c99a9838eda2c9673791c309e7087c6ef6c Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Sun, 11 Jul 2021 21:54:43 -0700 Subject: [PATCH 07/14] chore: fix flaky fullscreen inheritance test --- spec-main/api-browser-window-spec.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts index 5467ff777e39f..21b7981cbfec4 100644 --- a/spec-main/api-browser-window-spec.ts +++ b/spec-main/api-browser-window-spec.ts @@ -4319,8 +4319,10 @@ describe('BrowserWindow module', () => { await enterFullScreen; expect(w.isFullScreen()).to.be.true('isFullScreen'); await delay(); - const w2 = new BrowserWindow(); - await delay(); + const w2 = new BrowserWindow({ show: false }); + const enterFullScreen2 = emittedOnce(w2, 'enter-full-screen'); + w2.show(); + await enterFullScreen2; expect(w2.isFullScreen()).to.be.true('isFullScreen'); }); }); From 360f6313b92cc2106e4c7b5dbfce2de656d9826e Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Mon, 12 Jul 2021 00:31:34 -0700 Subject: [PATCH 08/14] chore: disable fullscreen test on mac arm --- spec-main/api-browser-window-spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts index 21b7981cbfec4..16efa54773165 100644 --- a/spec-main/api-browser-window-spec.ts +++ b/spec-main/api-browser-window-spec.ts @@ -4312,7 +4312,7 @@ describe('BrowserWindow module', () => { expect(w.isFullScreen()).to.be.false('isFullScreen'); }); - it('multiple windows inherit correct fullscreen state', async () => { + ifit(process.arch === 'x64')('multiple windows inherit correct fullscreen state', async () => { const w = new BrowserWindow(); const enterFullScreen = emittedOnce(w, 'enter-full-screen'); w.setFullScreen(true); From 78cb54edd1900db6644cf1075ada0f92fa7ca52e Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Mon, 12 Jul 2021 22:05:06 -0700 Subject: [PATCH 09/14] chore: simplify found-in-page spec --- spec/webview-spec.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/webview-spec.js b/spec/webview-spec.js index a094cc69e386f..3106788691837 100644 --- a/spec/webview-spec.js +++ b/spec/webview-spec.js @@ -975,9 +975,7 @@ describe(' tag', function () { describe('found-in-page event', () => { it('emits when a request is made', async () => { - const didFinishLoad = waitForEvent(webview, 'did-finish-load'); - loadWebView(webview, { src: `file://${fixtures}/pages/content.html` }); - await didFinishLoad; + await loadWebView(webview, { src: `file://${fixtures}/pages/content.html` }); // TODO(deepak1556): With https://codereview.chromium.org/2836973002 // focus of the webContents is required when triggering the api. // Remove this workaround after determining the cause for @@ -985,11 +983,13 @@ describe(' tag', function () { webview.focus(); const activeMatchOrdinal = []; + let isFirstRequest = true; for (;;) { const foundInPage = waitForEvent(webview, 'found-in-page'); - const requestId = webview.findInPage('virtual'); + const requestId = webview.findInPage('virtual', {findNext: isFirstRequest}); const event = await foundInPage; + isFirstRequest = false; expect(event.result.requestId).to.equal(requestId); expect(event.result.matches).to.equal(3); From ceff2c76fd4f90ba3d67eb0788061527c0000ebe Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Mon, 12 Jul 2021 22:13:21 -0700 Subject: [PATCH 10/14] chore: fix lint --- spec/webview-spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/webview-spec.js b/spec/webview-spec.js index 3106788691837..f2036eb395a31 100644 --- a/spec/webview-spec.js +++ b/spec/webview-spec.js @@ -987,7 +987,7 @@ describe(' tag', function () { for (;;) { const foundInPage = waitForEvent(webview, 'found-in-page'); - const requestId = webview.findInPage('virtual', {findNext: isFirstRequest}); + const requestId = webview.findInPage('virtual', { findNext: isFirstRequest }); const event = await foundInPage; isFirstRequest = false; From 58943a6be5d1e3c7e302536f78957e8866de404f Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Tue, 13 Jul 2021 01:52:39 -0700 Subject: [PATCH 11/14] chore: move found-in-page spec to main process --- spec-main/webview-spec.ts | 42 +++++++++++++++++++++++++++++++++++++++ spec/webview-spec.js | 33 ------------------------------ 2 files changed, 42 insertions(+), 33 deletions(-) diff --git a/spec-main/webview-spec.ts b/spec-main/webview-spec.ts index 5d576e48e2623..8cc488d35bfc8 100644 --- a/spec-main/webview-spec.ts +++ b/spec-main/webview-spec.ts @@ -783,4 +783,46 @@ describe(' tag', function () { })`); }); }); + + describe('found-in-page event', () => { + let w: BrowserWindow; + beforeEach(async () => { + w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true, webviewTag: true, contextIsolation: false } }); + await w.loadURL('about:blank'); + }); + afterEach(closeAllWindows); + + it('emits when a request is made', async () => { + loadWebView(w.webContents, { + src: `file://${fixtures}/pages/content.html` + }); + const [, webViewContents] = await emittedOnce(app, 'web-contents-created'); + // TODO(deepak1556): With https://codereview.chromium.org/2836973002 + // focus of the webContents is required when triggering the api. + // Remove this workaround after determining the cause for + // incorrect focus. + webViewContents.focus(); + const activeMatchOrdinal = []; + let isFirstRequest = true; + + for (;;) { + const foundInPage = emittedOnce(webViewContents, 'found-in-page'); + const requestId = webViewContents.findInPage('virtual', {findNext: isFirstRequest}); + const [, result] = await foundInPage; + isFirstRequest = false; + + expect(result.requestId).to.equal(requestId); + expect(result.matches).to.equal(3); + + activeMatchOrdinal.push(result.activeMatchOrdinal); + + if (result.activeMatchOrdinal === result.matches) { + break; + } + } + + expect(activeMatchOrdinal).to.deep.equal([1, 2, 3]); + webViewContents.stopFindInPage('clearSelection'); + }); + }); }); diff --git a/spec/webview-spec.js b/spec/webview-spec.js index f2036eb395a31..852f1d9591759 100644 --- a/spec/webview-spec.js +++ b/spec/webview-spec.js @@ -973,39 +973,6 @@ describe(' tag', function () { }); }); - describe('found-in-page event', () => { - it('emits when a request is made', async () => { - await loadWebView(webview, { src: `file://${fixtures}/pages/content.html` }); - // TODO(deepak1556): With https://codereview.chromium.org/2836973002 - // focus of the webContents is required when triggering the api. - // Remove this workaround after determining the cause for - // incorrect focus. - webview.focus(); - - const activeMatchOrdinal = []; - let isFirstRequest = true; - - for (;;) { - const foundInPage = waitForEvent(webview, 'found-in-page'); - const requestId = webview.findInPage('virtual', { findNext: isFirstRequest }); - const event = await foundInPage; - isFirstRequest = false; - - expect(event.result.requestId).to.equal(requestId); - expect(event.result.matches).to.equal(3); - - activeMatchOrdinal.push(event.result.activeMatchOrdinal); - - if (event.result.activeMatchOrdinal === event.result.matches) { - break; - } - } - - expect(activeMatchOrdinal).to.deep.equal([1, 2, 3]); - webview.stopFindInPage('clearSelection'); - }); - }); - describe('.getWebContentsId', () => { it('can return the WebContents ID', async () => { const src = 'about:blank'; From 7a89f76ffa584a7209bd8b10d806f6da9a42b9ec Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Tue, 13 Jul 2021 02:00:32 -0700 Subject: [PATCH 12/14] chore: fix lint --- spec-main/webview-spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec-main/webview-spec.ts b/spec-main/webview-spec.ts index 8cc488d35bfc8..1ad272be053c6 100644 --- a/spec-main/webview-spec.ts +++ b/spec-main/webview-spec.ts @@ -807,7 +807,7 @@ describe(' tag', function () { for (;;) { const foundInPage = emittedOnce(webViewContents, 'found-in-page'); - const requestId = webViewContents.findInPage('virtual', {findNext: isFirstRequest}); + const requestId = webViewContents.findInPage('virtual', { findNext: isFirstRequest }); const [, result] = await foundInPage; isFirstRequest = false; From 637fc07c94dc72ff1eb8a3d132ded419c6fcf817 Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Tue, 13 Jul 2021 04:43:53 -0700 Subject: [PATCH 13/14] chore: remove focus workaround --- spec-main/webview-spec.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/spec-main/webview-spec.ts b/spec-main/webview-spec.ts index 1ad272be053c6..4a068a99322de 100644 --- a/spec-main/webview-spec.ts +++ b/spec-main/webview-spec.ts @@ -797,11 +797,6 @@ describe(' tag', function () { src: `file://${fixtures}/pages/content.html` }); const [, webViewContents] = await emittedOnce(app, 'web-contents-created'); - // TODO(deepak1556): With https://codereview.chromium.org/2836973002 - // focus of the webContents is required when triggering the api. - // Remove this workaround after determining the cause for - // incorrect focus. - webViewContents.focus(); const activeMatchOrdinal = []; let isFirstRequest = true; From 1674a4dd5230783812dc8a005bc84bb4f288e9ee Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Wed, 14 Jul 2021 01:47:10 -0700 Subject: [PATCH 14/14] chore: improve found-in-page spec --- spec-main/webview-spec.ts | 50 +++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/spec-main/webview-spec.ts b/spec-main/webview-spec.ts index 4a068a99322de..de1153d76dd6e 100644 --- a/spec-main/webview-spec.ts +++ b/spec-main/webview-spec.ts @@ -787,37 +787,47 @@ describe(' tag', function () { describe('found-in-page event', () => { let w: BrowserWindow; beforeEach(async () => { - w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true, webviewTag: true, contextIsolation: false } }); + w = new BrowserWindow({ show: false, webPreferences: { webviewTag: true, contextIsolation: false } }); await w.loadURL('about:blank'); }); afterEach(closeAllWindows); it('emits when a request is made', async () => { - loadWebView(w.webContents, { + await loadWebView(w.webContents, { src: `file://${fixtures}/pages/content.html` }); - const [, webViewContents] = await emittedOnce(app, 'web-contents-created'); - const activeMatchOrdinal = []; - let isFirstRequest = true; + const result = await w.webContents.executeJavaScript(`new Promise((resolve, reject) => { + const webview = document.querySelector('webview') - for (;;) { - const foundInPage = emittedOnce(webViewContents, 'found-in-page'); - const requestId = webViewContents.findInPage('virtual', { findNext: isFirstRequest }); - const [, result] = await foundInPage; - isFirstRequest = false; - - expect(result.requestId).to.equal(requestId); - expect(result.matches).to.equal(3); - - activeMatchOrdinal.push(result.activeMatchOrdinal); + const waitForEvent = (target, eventName) => { + return new Promise(resolve => { + target.addEventListener(eventName, resolve, { once: true }) + }) + } - if (result.activeMatchOrdinal === result.matches) { - break; + async function startFind() { + const activeMatchOrdinal = [] + const foundInPage = waitForEvent(webview, 'found-in-page') + webview.findInPage('virtual', { findNext: true }) + const event = await foundInPage + if (event.result.matches) { + activeMatchOrdinal.push(event.result.activeMatchOrdinal) + + for (let i=1; i < event.result.matches; i++) { + const foundInPage = waitForEvent(webview, 'found-in-page') + webview.findInPage('virtual', { findNext: false }) + const event = await foundInPage + activeMatchOrdinal.push(event.result.activeMatchOrdinal) + } + } + webview.stopFindInPage('clearSelection') + resolve(activeMatchOrdinal) } - } - expect(activeMatchOrdinal).to.deep.equal([1, 2, 3]); - webViewContents.stopFindInPage('clearSelection'); + webview.focus() + startFind() + })`); + expect(result).to.deep.equal([1, 2, 3]); }); }); });