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

[Bug]: 'close' event is not triggered for sandboxed windows #28215

Open
3 tasks done
ChALkeR opened this issue Mar 16, 2021 · 19 comments
Open
3 tasks done

[Bug]: 'close' event is not triggered for sandboxed windows #28215

ChALkeR opened this issue Mar 16, 2021 · 19 comments
Labels

Comments

@ChALkeR
Copy link
Contributor

ChALkeR commented Mar 16, 2021

Preflight Checklist

Electron Version

12.0.1, 11.3.0, 10.4.1

What operating system are you using?

Other Linux

Operating System Version

Linux workstation 5.11.6-arch1-1 #1 SMP PREEMPT Thu, 11 Mar 2021 13:48:23 +0000 x86_64 GNU/Linux

What arch are you using?

x64

Last Known Working Electron version

No response

Expected Behavior

close event should be triggered when the webpage closes itself with window.close(), as it is for non-sandboxed contexts.

Actual Behavior

close event is not triggered on window.close() from inside the webpage for sandboxed contexts.

Testcase Gist URL

https://gist.github.com/cecc82a9d8ee56d01aa97b4c716eea9e

Testcase:

const { app, BrowserWindow } = require('electron')
app.whenReady().then(() => {
  const win = new BrowserWindow({
    width: 800, height: 600,
    webPreferences: {
      // sandbox: false, // works
      sandbox: true, // broken
    }
  })
  win.loadFile('about:blank')
  win.on('close', () => { console.log('close') }) // not triggered if broken
  win.on('closed', () => { console.log('closed') })
  setTimeout(() => {
    win.webContents.executeJavaScript('window.close()') // broken
    // win.close() // works
  }, 0)
})

Here, executeJavaScript is not necessary -- the page can do the same itself, or that could be alternatively done from devtools.

  • If the window is closed externally (via .close() method of BrowserWindow instance, or via Alt-F4 or other OS/DE method), close event is always triggered.

  • If sandbox is not enabled, close event is always triggered.

  • closed (as opposed to close) event is also triggered if the window was closed.

  • If the window closes itself with window.close() and it's sandboxed -- close event doesn't get triggered.

@ChALkeR ChALkeR changed the title 'close' event is not triggered for sandboxed windows [Bug]: 'close' event is not triggered for sandboxed windows Mar 16, 2021
@nornagon nornagon added has-repro-gist Issue can be reproduced with code at https://gist.github.com/ 10-x-y 11-x-y 12-x-y platform/all labels Mar 16, 2021
@nornagon
Copy link
Member

nornagon commented Mar 16, 2021

NB. I copied your repro to a Fiddle-compatible gist for easy Fiddling. I can indeed reproduce this.

@nornagon
Copy link
Member

Interestingly, win.webContents.on("close") does get emitted.

@ChALkeR
Copy link
Contributor Author

ChALkeR commented Mar 18, 2021

Is win.webContents.on("close") documented somewhere?
Also, seems to be no way to prevent the window close action from there.

@nornagon
Copy link
Member

Is win.webContents.on("close") documented somewhere?

Huh, apparently not.

Also, seems to be no way to prevent the window close action from there.

gotcha, yeah it makes sense that it should be possible to prevent the window close.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2022

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. If you have any new additional information—in particular, if this is still reproducible in the latest version of Electron or in the beta—please include it with your comment!

@github-actions github-actions bot added the stale label Oct 6, 2022
@schontz
Copy link

schontz commented Oct 6, 2022

Bump

@github-actions github-actions bot removed the stale label Oct 7, 2022
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. If you have any new additional information—in particular, if this is still reproducible in the latest version of Electron or in the beta—please include it with your comment!

@github-actions github-actions bot added the stale label Jan 10, 2023
@ChALkeR
Copy link
Contributor Author

ChALkeR commented Jan 10, 2023

bump, this looks still valid

@github-actions github-actions bot removed the stale label Jan 11, 2023
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. If you have any new additional information—in particular, if this is still reproducible in the latest version of Electron or in the beta—please include it with your comment!

@github-actions github-actions bot added the stale label Apr 12, 2023
@pushkin-
Copy link

repros in v24

@github-actions github-actions bot removed the stale label Apr 13, 2023
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. If you have any new additional information—in particular, if this is still reproducible in the latest version of Electron or in the beta—please include it with your comment!

@github-actions github-actions bot added the stale label Jul 12, 2023
@pushkin-
Copy link

bump

@schontz
Copy link

schontz commented Jul 12, 2023

Tested the original fiddle on v25.2.0 and verified that this is still broken.

@github-actions github-actions bot removed the stale label Jul 13, 2023
@electron-issue-triage
Copy link

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. If you have any new additional information—in particular, if this is still reproducible in the latest version of Electron or in the beta—please include it with your comment!

@pushkin-
Copy link

repros in v27

@electron-issue-triage
Copy link

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. If you have any new additional information—in particular, if this is still reproducible in the latest version of Electron or in the beta—please include it with your comment!

@pushkin-
Copy link

repros in v29 alpha 8

@pushkin-
Copy link

pushkin- commented Jan 19, 2024

Note that in my case, I have a window with multiple BrowserViews in it (like tabs). This case needs to be considered when fixing this since fixing this in the naive way would mean that when one webContents closes, the entire window closes, which isn't really appropriate. It seems like it only makes sense for the "close" event to fire when there are no other webContents in the BrowserWindow.

@rafaberaldo
Copy link

It's a weird little inconsistency, although pretty easily to workaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants