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

feat: Allow WebContentsView to accept webContents object. #42086

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

khalwa
Copy link
Contributor

@khalwa khalwa commented May 8, 2024

Description of Change

Closes #42054.

This PR enables WebContentsView to accept existing webContents object in order to be able to properly handle popups. This issue is described here: #42054.

Checklist

Release Notes

Notes: Extended WebContentsView to accept pre-existing webContents object.

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label May 8, 2024
@ckerr ckerr requested a review from nornagon May 8, 2024 15:58
spec/api-web-contents-view-spec.ts Show resolved Hide resolved
shell/browser/api/electron_api_web_contents_view.cc Outdated Show resolved Hide resolved
expect(webContentsView.webContents).to.eq(wc);
expect(webContents.getAllWebContents().length).to.equal(currentWebContentsCount + 1, 'expected only single webcontents to be created');
});

Copy link
Member

Choose a reason for hiding this comment

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

Please also test what happens if you create a WebContentsView with a WebContents that's already attached somewhere else (either a BrowserWindow's WebContents or another WebContentsView's WebContents).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added the check for owner window when creating WebContentsView. Please let me know if there is a better way to determine whether WebContents is already attached to something.

@khalwa khalwa force-pushed the webContentsView-accepting-webcontents-option branch from 07cc3ac to bcf4199 Compare May 13, 2024 09:11
@khalwa khalwa requested a review from nornagon May 13, 2024 09:17
@codebytere codebytere added the semver/minor backwards-compatible functionality label May 13, 2024
@electron-cation electron-cation bot added api-review/requested 🗳 and removed new-pr 🌱 PR opened in the last 24 hours labels May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-review/requested 🗳 semver/minor backwards-compatible functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Popup handling in BrowserViews doesn't work with WebContentsView
4 participants