fix: move BrowserWindow's WebContentsView to be a child of rootview by nornagon · Pull Request #41256 · electron/electron

nornagon · 2024-02-06T22:00:30Z

Description of Change

Previously, BrowserWindow#contentView was a reference to the BrowserWindow's
WebContentsView, and any child views that were added would be children of the
WebContentsView. This caused some strange behavior, including BrowserViews
sometimes being painted behind the BrowserWindow's web contents.

This PR has BrowserWindow's WebContentsView be a sibling to the content view,
inserted before it, so that the BrowserWindow's WebContents will always display
behind its children as intended.

It also prevents users from doing dangerous, unsupported and probably entirely
broken things like swapping out the BrowserWindow's contentView for another
view.

Checklist

PR description included and stakeholders cc'd
npm test passes
tests are changed or added
PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes: Fixed an issue where child views in a BrowserWindow could sometimes be
painted behind the main web content.

shell/browser/api/electron_api_browser_window.cc

samuelmaddock

API LGTM

samuelmaddock · 2024-02-07T03:56:43Z

Looks like the menu bar is expecting a different amount of children after this PR

electron/shell/browser/ui/views/root_view.cc

Lines 92 to 98 in 4367c5a

    
           if (visible) { 
        
             DCHECK_EQ(children().size(), 1ul); 
        
             AddChildView(menu_bar_.get()); 
        
           } else { 
        
             DCHECK_EQ(children().size(), 2ul); 
        
             RemoveChildView(menu_bar_.get()); 
        
           }

zcbenz

API LGTM

shell/browser/api/electron_api_browser_window.cc

zcbenz · 2024-02-28T07:30:57Z

The WoA CI consistently fails with a [5812:0228/072758.443:FATAL:frame_subscriber.cc(45)] Check failed: !size.IsEmpty(). crash, can you try merging latest main?

zcbenz · 2024-03-04T00:33:11Z

WoA still crashes, maybe just remove the line?

…bcontentsview

release-clerk · 2024-04-08T17:30:26Z

Release Notes Persisted

Fixed an issue where child views in a BrowserWindow could sometimes be

trop · 2024-04-08T17:30:41Z

I was unable to backport this PR to "30-x-y" cleanly;
you will need to perform this backport manually.

dsanders11 · 2024-04-08T19:54:34Z

/trop run backport-to 30-x-y

trop · 2024-04-08T19:54:37Z

The backport process for this PR has been manually initiated - sending your PR to 30-x-y!

…41256) Co-authored-by: Jeremy Rose <jeremya@chromium.org>

trop · 2024-04-08T19:55:00Z

I was unable to backport this PR to "30-x-y" cleanly;
you will need to perform this backport manually.

…41802) fix: move BrowserWindow's WebContentsView to be a child of rootview (#41256) Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Jeremy Rose <jeremya@chromium.org>

Revert "fix: move BrowserWindow's WebContentsView to be a child of rootview (electron#41256)" This reverts commit 76f7bbb. Patch-Filename: fix_revert_change_that_broke_nvda_screen_reader.patch

Revert "fix: move BrowserWindow's WebContentsView to be a child of rootview (electron#41256)" This reverts commit 76f7bbb. Additionally reverts commit 7d045dc to support prior revert. Patch-Filename: fix_revert_change_that_broke_nvda_screen_reader.patch

fix: hide BrowserWindow's WebContentsView

a5084fc

electron-cation bot added the new-pr 🌱 PR opened recently label Feb 6, 2024

nornagon added semver/minor backwards-compatible functionality no-backport labels Feb 6, 2024

electron-cation bot added the api-review/requested 🗳 label Feb 6, 2024

samuelmaddock reviewed Feb 6, 2024

View reviewed changes

shell/browser/api/electron_api_browser_window.cc Show resolved Hide resolved

shell/browser/api/electron_api_browser_window.cc Show resolved Hide resolved

samuelmaddock approved these changes Feb 7, 2024

View reviewed changes

force layout

48f89cb

nornagon mentioned this pull request Feb 7, 2024

[Bug]: BrowserView not rendering after WebContentsView PR got merged #40792

Closed

3 tasks

nornagon changed the title ~~fix: hide BrowserWindow's WebContentsView~~ fix: move BrowserWindow's WebContentsView to be a child of rootview Feb 7, 2024

tentative

b2f16cf

zcbenz approved these changes Feb 8, 2024

View reviewed changes

shell/browser/api/electron_api_browser_window.cc Show resolved Hide resolved

nornagon added 3 commits February 8, 2024 22:00

fix build

3e33135

fix

0b0221f

added comment

b3d6c53

codebytere mentioned this pull request Feb 12, 2024

[Bug]: Frameless browserview gets bigger size while it is set to fullscreen #34342

Open

3 tasks

electron-cation bot added api-review/approved ✅ and removed new-pr 🌱 PR opened recently api-review/requested 🗳 labels Feb 13, 2024

nornagon force-pushed the hide-browserwindow-webcontentsview branch from 03b1e5e to 3d3af28 Compare February 20, 2024 00:58

Merge main

aa66ce7

nornagon force-pushed the hide-browserwindow-webcontentsview branch from 3d3af28 to aa66ce7 Compare February 20, 2024 01:11

Merge main

36aa723

nornagon added 4 commits April 1, 2024 11:45

Merge remote-tracking branch 'origin/main' into hide-browserwindow-we…

187f0e8

…bcontentsview

fix layout issues on win/linux

5a6fd18

fix view hierarchy

c717f3c

correctly compute menu bar height

2f92cde

trop bot added needs-manual-bp/30-x-y and removed target/30-x-y PR should also be added to the "30-x-y" branch. labels Apr 8, 2024

dsanders11 added target/30-x-y PR should also be added to the "30-x-y" branch. needs-manual-bp/30-x-y and removed needs-manual-bp/30-x-y target/30-x-y PR should also be added to the "30-x-y" branch. labels Apr 8, 2024

trop bot added a commit that referenced this pull request Apr 8, 2024

fix: move BrowserWindow's WebContentsView to be a child of rootview (#…

1789d34

…41256) Co-authored-by: Jeremy Rose <jeremya@chromium.org>

trop bot mentioned this pull request Apr 8, 2024

fix: move BrowserWindow's WebContentsView to be a child of rootview #41802

Merged

nornagon added in-flight/30-x-y merged/30-x-y PR was merged to the "30-x-y" branch. and removed needs-manual-bp/30-x-y in-flight/30-x-y labels Apr 10, 2024

Tuditi mentioned this pull request May 15, 2024

🐞 - Transak white screen when buying IOTA bloomwalletio/bloom#2497

Closed

2 tasks

codebytere mentioned this pull request May 17, 2024

[Bug]: setBrowserView broken in v30 on macOS, works on Windows #42202

Closed

3 tasks

Hans-Halverson mentioned this pull request May 24, 2024

[Bug]: BrowserView's content is visually wiped when vibrancy is set on Electron version 30 and up #41979

Closed

3 tasks

markh-discord mentioned this pull request Jul 17, 2024

[Bug]: NVDA screen reader no longer works since 30.0.0-beta.8 #42945

Open

3 tasks

georgexu99 mentioned this pull request Sep 18, 2024

Mouse tracking breaks on electron apps nvaccess/nvda#17190

Open

codebytere mentioned this pull request Oct 29, 2024

[Bug] unable to get context-menu event when using -webkit-app-region: drag #44386

Closed

3 tasks

codebytere mentioned this pull request Nov 20, 2024

fix: ensure context-menu emitted for draggable regions #44761

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: move BrowserWindow's WebContentsView to be a child of rootview#41256

fix: move BrowserWindow's WebContentsView to be a child of rootview#41256
nornagon merged 12 commits intomainfrom
hide-browserwindow-webcontentsview

nornagon commented Feb 6, 2024

Uh oh!

Uh oh!

Uh oh!

samuelmaddock left a comment

Uh oh!

samuelmaddock commented Feb 7, 2024

Uh oh!

zcbenz left a comment

Uh oh!

Uh oh!

zcbenz commented Feb 28, 2024

Uh oh!

zcbenz commented Mar 4, 2024

Uh oh!

release-clerk bot commented Apr 8, 2024

Uh oh!

trop bot commented Apr 8, 2024

Uh oh!

dsanders11 commented Apr 8, 2024

Uh oh!

trop bot commented Apr 8, 2024

Uh oh!

trop bot commented Apr 8, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

nornagon commented Feb 6, 2024

Description of Change

Checklist

Release Notes

Uh oh!

Uh oh!

Uh oh!

samuelmaddock left a comment

Choose a reason for hiding this comment

Uh oh!

samuelmaddock commented Feb 7, 2024

Uh oh!

zcbenz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zcbenz commented Feb 28, 2024

Uh oh!

zcbenz commented Mar 4, 2024

Uh oh!

release-clerk bot commented Apr 8, 2024

Uh oh!

trop bot commented Apr 8, 2024

Uh oh!

dsanders11 commented Apr 8, 2024

Uh oh!

trop bot commented Apr 8, 2024

Uh oh!

trop bot commented Apr 8, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants