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.

Sign up for GitHub

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

Jump to bottom

fix: set nativeWindowOpen when sandboxed #18273

Merged
merged 1 commit into from May 27, 2019
Merged

fix: set nativeWindowOpen when sandboxed #18273

merged 1 commit into from May 27, 2019

Conversation

miniak
Copy link
Contributor

@miniak miniak commented May 12, 2019 •
edited by MarshallOfSound

Description of Change

Fixes #17132.

Checklist

Release Notes

Notes:

  • Fixed issues with popups created from sandboxed <webview>:
  • Clicking link with target="_blank" not emitting 'new-window' event.
  • window.open() not returning null when allowpopups is not set.

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label May 12, 2019
@miniak
Copy link
Contributor Author

miniak commented May 12, 2019

cc @chrismohr

@miniak miniak self-assigned this May 12, 2019
@miniak miniak force-pushed the miniak/fix-new-window-in-sandbox branch 7 times, most recently from 0f4150d to c52b22e Compare May 13, 2019 00:20
@miniak miniak changed the title fix: clicking link with target="_blank" does emit 'new-window' event with sandbox enabled fix: set nativeWindowOpen when sandboxed May 13, 2019
@miniak miniak marked this pull request as ready for review May 13, 2019 01:01
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label May 14, 2019
MarshallOfSound
MarshallOfSound reviewed May 17, 2019
View reviewed changes
Copy link
Member

@MarshallOfSound MarshallOfSound left a comment

Choose a reason for hiding this comment

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

Tentative approve, but concerned with the backports (although this is a fix, it is a change in user-facing JS behavior). I'd fell more comfortable not backporting this beyond 6

@miniak
Copy link
Contributor Author

miniak commented May 19, 2019 •
edited

@MarshallOfSound removed backport labels for 4 and 5. It can be discussed separately whether backporting this fix into older version is safe

@miniak miniak force-pushed the miniak/fix-new-window-in-sandbox branch from c52b22e to 870ebb0 Compare May 20, 2019 15:15
@miniak miniak mentioned this pull request May 21, 2019
Closed
3 tasks
@miniak
Copy link
Contributor Author

miniak commented Jun 1, 2019

@nornagon, @zcbenz do you agree with @MarshallOfSound that backporting to 4/5 is risky, even though it's a fix?

@zcbenz
Copy link
Member

zcbenz commented Jun 1, 2019

I'm neutral on this, backporting it doesn't seem very risky to me though.

codebytere pushed a commit that referenced this pull request Jun 8, 2019
@miniak @codebytere
fix: set nativeWindowOpen when sandboxed (#18273) (#18556)
7902c3c
@sofianguy sofianguy added this to Fixed in 6.0.0-beta.7 in 6.1.x Jun 11, 2019
miniak added a commit that referenced this pull request Jun 14, 2019
@miniak
fix: set nativeWindowOpen when sandboxed (#18273)
fa9b637
miniak added a commit that referenced this pull request Jun 14, 2019
@miniak
fix: set nativeWindowOpen when sandboxed (#18273)
8b303cb
@miniak miniak mentioned this pull request Jun 14, 2019
Merged
5 tasks
@trop
Copy link
Contributor

trop bot commented Jun 14, 2019

A maintainer has manually backported this PR to "4-2-x", please check out #18797

@trop trop bot added the in-flight/4-2-x label Jun 14, 2019
@miniak miniak mentioned this pull request Jun 14, 2019
Merged
5 tasks
@trop
Copy link
Contributor

trop bot commented Jun 14, 2019

A maintainer has manually backported this PR to "5-0-x", please check out #18798

@trop trop bot added the in-flight/5-0-x label Jun 14, 2019
miniak added a commit that referenced this pull request Jun 14, 2019
@miniak
fix: set nativeWindowOpen when sandboxed (#18273)
7fba74f
@miniak miniak mentioned this pull request Jun 14, 2019
Closed
5 tasks
@trop
Copy link
Contributor

trop bot commented Jun 14, 2019

A maintainer has manually backported this PR to "3-1-x", please check out #18799

@trop trop bot added the in-flight/3-1-x label Jun 14, 2019
miniak added a commit that referenced this pull request Jun 14, 2019
@miniak
fix: set nativeWindowOpen when sandboxed (#18273)
bf81aca
codebytere pushed a commit that referenced this pull request Jun 20, 2019
@miniak @codebytere
fix: set nativeWindowOpen when sandboxed (#18273) (#18798)
4318ad8
codebytere pushed a commit that referenced this pull request Jun 20, 2019
@miniak @codebytere
fix: set nativeWindowOpen when sandboxed (#18273) (#18797)
5f30c61
@sofianguy sofianguy added this to Fixed in 5.0.6 in 5.0.x Jun 26, 2019
@nornagon nornagon mentioned this pull request Apr 21, 2020
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nornagon nornagon nornagon approved these changes

@alexeykuzmin alexeykuzmin alexeykuzmin approved these changes

@zcbenz zcbenz zcbenz approved these changes

@MarshallOfSound MarshallOfSound MarshallOfSound approved these changes

@ppontes ppontes ppontes approved these changes

@deepak1556 deepak1556 Awaiting requested review from deepak1556

Assignees

@miniak miniak

Labels
None yet
Projects
No open projects
5.0.x
Fixed in 5.0.6
6.1.x
Fixed in 6.0.0-beta.7
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Clicking link with target=_blank does not fire 'new-window' event with Sandbox Enabled
6 participants
@miniak @nornagon @zcbenz @alexeykuzmin @MarshallOfSound @ppontes