Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MM-59823] Migrate BrowserView to WebContentsView #3177

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

devinbinnie
Copy link
Member

Summary

A while back, Electron implemented BrowserView as an experimental way of nesting sandboxed Chromium views inside of a main windows, replacing webview. However, for the longest time that functionality remained experimental and not fully supported.

This PR migrates us over to the new WebContentsView, introduced in Electron 30 which should be more stable and supported. It should also simplify resizing logic.

Ticket Link

https://mattermost.atlassian.net/browse/MM-59823

NONE

@devinbinnie devinbinnie added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Oct 23, 2024
@devinbinnie devinbinnie requested review from pvev, yasserfaraazkhan, a team and enahum and removed request for a team October 23, 2024 19:43
@yasserfaraazkhan yasserfaraazkhan added the Run Desktop E2E Tests This label will trigger the workflow that runs e2e automation tests label Oct 24, 2024
Copy link

Here are the test results below:

Test Summary for Linux on commit a0c98b6

The following known failed tests have been fixed on Linux:
- menu/view MM-T820 should open Developer Tools For Application Wrapper for main window

Test Summary for macOS on commit a0c98b6

New failed tests found on macOS:

  • Add Server Modal MM-T1312 should focus the first text input

The following known failed tests have been fixed on macOS:
- popup MM-T2827_1 should be able to select all in popup windows

Test Summary for Windows on commit a0c98b6

All stable tests passed on Windows.

@github-actions github-actions bot removed the Run Desktop E2E Tests This label will trigger the workflow that runs e2e automation tests label Oct 24, 2024
@devinbinnie devinbinnie added this to the v5.11.0 milestone Oct 24, 2024
@devinbinnie devinbinnie removed the 3: QA Review Requires review by a QA tester label Oct 24, 2024
Copy link
Contributor

@pvev pvev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! nice change @devinbinnie 👍

@devinbinnie
Copy link
Member Author

/update-branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2: Dev Review Requires review by a core committer release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants