From a0bb087ec1aee28ca2532cca5cd8e875d008aff0 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Wed, 29 Nov 2023 22:04:54 +0100 Subject: [PATCH] fix: webview zoom level persistence on navigation --- shell/browser/web_contents_zoom_controller.cc | 14 ++++++--- .../webview-zoom-change-persist-host.html | 31 +++++++++++++++++++ spec/node-spec.ts | 8 ++++- spec/webview-spec.ts | 25 +++++++++++++++ 4 files changed, 72 insertions(+), 6 deletions(-) create mode 100644 spec/fixtures/pages/webview-zoom-change-persist-host.html diff --git a/shell/browser/web_contents_zoom_controller.cc b/shell/browser/web_contents_zoom_controller.cc index b0fb6bee9d93b..a79e10fa7a787 100644 --- a/shell/browser/web_contents_zoom_controller.cc +++ b/shell/browser/web_contents_zoom_controller.cc @@ -74,6 +74,7 @@ bool WebContentsZoomController::SetZoomLevel(double level) { DCHECK_CURRENTLY_ON(BrowserThread::UI); content::NavigationEntry* entry = web_contents()->GetController().GetLastCommittedEntry(); + // Cannot zoom in disabled mode. Also, don't allow changing zoom level on // a crashed tab, an error page or an interstitial page. if (zoom_mode_ == ZOOM_MODE_DISABLED || @@ -90,7 +91,7 @@ bool WebContentsZoomController::SetZoomLevel(double level) { zoom_level_ = level; ZoomChangedEventData zoom_change_data(web_contents(), old_zoom_level, - zoom_level_, false /* temporary */, + zoom_level_, true /* temporary */, zoom_mode_); for (auto& observer : observers_) observer.OnZoomChanged(zoom_change_data); @@ -154,7 +155,7 @@ void WebContentsZoomController::SetTemporaryZoomLevel(double level) { // Notify observers of zoom level changes. ZoomChangedEventData zoom_change_data(web_contents(), zoom_level_, level, true /* temporary */, zoom_mode_); - for (WebContentsZoomObserver& observer : observers_) + for (auto& observer : observers_) observer.OnZoomChanged(zoom_change_data); } @@ -268,8 +269,10 @@ void WebContentsZoomController::ResetZoomModeOnNavigationIfNeeded( double old_zoom_level = zoom_map->GetZoomLevel(web_contents()); double new_zoom_level = zoom_map->GetZoomLevelForHostAndScheme( url.scheme(), net::GetHostOrSpecFromURL(url)); + event_data_ = std::make_unique( web_contents(), old_zoom_level, new_zoom_level, false, ZOOM_MODE_DEFAULT); + // The call to ClearTemporaryZoomLevel() doesn't generate any events from // HostZoomMap, but the call to UpdateState() at the end of // DidFinishNavigation will notify our observers. @@ -295,11 +298,12 @@ void WebContentsZoomController::DidFinishNavigation( if (!navigation_handle->IsSameDocument()) { ResetZoomModeOnNavigationIfNeeded(navigation_handle->GetURL()); SetZoomFactorOnNavigationIfNeeded(navigation_handle->GetURL()); + + // If the main frame's content has changed, the new page may have a + // different zoom level from the old one. + UpdateState(std::string()); } - // If the main frame's content has changed, the new page may have a different - // zoom level from the old one. - UpdateState(std::string()); DCHECK(!event_data_); } diff --git a/spec/fixtures/pages/webview-zoom-change-persist-host.html b/spec/fixtures/pages/webview-zoom-change-persist-host.html new file mode 100644 index 0000000000000..6dd7787a6b9ca --- /dev/null +++ b/spec/fixtures/pages/webview-zoom-change-persist-host.html @@ -0,0 +1,31 @@ + + + + + + + + \ No newline at end of file diff --git a/spec/node-spec.ts b/spec/node-spec.ts index e7e1d81e27219..2f8dd5218d567 100644 --- a/spec/node-spec.ts +++ b/spec/node-spec.ts @@ -873,7 +873,13 @@ describe('node feature', () => { }); }; - process.once('unhandledRejection', () => done(new Error('catch block is delayed to next tick'))); + let called = false; + process.once('unhandledRejection', () => { + if (called) return; + + done(new Error('catch block is delayed to next tick')); + called = true; + }); setTimeout(() => { f3().catch(() => done()); diff --git a/spec/webview-spec.ts b/spec/webview-spec.ts index 7563fef1a54c3..57770e797ca7b 100644 --- a/spec/webview-spec.ts +++ b/spec/webview-spec.ts @@ -353,6 +353,31 @@ describe(' tag', function () { expect(zoomLevel).to.equal(1); }); + it('maintains the zoom level for a given host in the same session after navigation', () => { + const w = new BrowserWindow({ + show: false, + webPreferences: { + webviewTag: true, + nodeIntegration: true, + contextIsolation: false + } + }); + + const zoomPromise = new Promise((resolve) => { + ipcMain.on('webview-zoom-persist-level', (_event, values) => { + resolve(values); + }); + }); + + w.loadFile(path.join(fixtures, 'pages', 'webview-zoom-change-persist-host.html')); + + expect(zoomPromise).to.eventually.deep.equal({ + initialZoomLevel: 2, + switchZoomLevel: 3, + finalZoomLevel: 2 + }); + }); + it('maintains zoom level on navigation', async () => { const w = new BrowserWindow({ show: false,