Skip to content
New issue

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

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

Already on GitHub? # to your account

Improve window.open support #140

Closed
Smona opened this issue May 10, 2016 · 18 comments
Closed

Improve window.open support #140

Smona opened this issue May 10, 2016 · 18 comments

Comments

@Smona
Copy link

Smona commented May 10, 2016

When trying to download a free song from toneden.io, the page which redirects from a soundcloud verification gets stuck, making it impossible to download.

Example of where this happens

@PalmerAL
Copy link
Collaborator

I'm guessing this is probably because we don't support window.open very well.

@miere
Copy link

miere commented May 11, 2016

The same happens on many sites when I try to use Facebook or Twitter login integrations...

@PalmerAL
Copy link
Collaborator

PalmerAL commented Nov 5, 2017

from #446:

Site netlify.com (and many others) do authentication after logging in in the new window and refreshing the main window.

Problem:
window.opener is nuil.

See

electron/electron#9533
electron/electron#9961
nativefier/nativefier#164

@PalmerAL
Copy link
Collaborator

PalmerAL commented Nov 6, 2017

@igk1972 see #440:

Muon does seem to solve a lot of the issues with this (and support for popup windows would be nice as well). The biggest problem with Muon (and what's stopped me from considering a switch in the past) is that there seems to be almost no documentation on it. Brave's added a bunch of API's specific to their browser, and changed a bunch of things about how Electron's existing features work, and the only way (unless I'm missing something?) to figure out how to use it is to look at Brave's source code and guess what the correct API is. Meanwhile, glancing through their issue tracker, they don't seem to answer questions about it (which I completely understand, they're not obligated to, but it's a problem for us). If we migrate to Muon, I think we're going to have a lot of problems with this; I'm not really sure what the best option is here.

@PalmerAL
Copy link
Collaborator

PalmerAL commented Nov 6, 2017

But I do agree that Muon has a lot of advantages, maybe it's worth trying to figure out how to use it based on the Brave source code.

@PalmerAL PalmerAL changed the title Toneden free download - redirect failure Toneden free download - redirect failure [window.open not supported] Dec 30, 2017
@PalmerAL PalmerAL mentioned this issue Apr 5, 2018
@PalmerAL
Copy link
Collaborator

PalmerAL commented Apr 5, 2018

Brave has announced that they're stopping development on Muon (https://brave.com/development-plans-for-upcoming-release/), so I guess we need to figure out an Electron-based solution for this.

@dbfreem
Copy link

dbfreem commented Jul 16, 2019

Is this still something being considered? I ran into a situation today where popups not being supported was a blocker for me using min.

@PalmerAL
Copy link
Collaborator

I'd like to have this too, but I'm not sure how feasible it is. We might be able to hack something together using preload scripts, and send synchronous IPC messages between the parent and the child window, but I'm not sure how well it would actually work. If it doesn't, then this would need to be implemented in Electron, which I think would require some pretty significant changes to Electron (although I'm not sure exactly what changes would be required). If someone wanted to work on it, I'm guessing they'd be willing to support it, but it doesn't seem to be a priority for anyone contributing to Electron right now.

@lawrencenull
Copy link

It seems electron does natively support window.open now through several methods.

https://electronjs.org/docs/api/window-open

@PalmerAL
Copy link
Collaborator

Oh, wow, you're right; everything does seem to work as expected when using the nativeWindowOpen option. (I'm wondering if that changed at some point, because I'm almost certain I tried that before, but maybe I'm just remembering wrong?)

@lawrencenull
Copy link

They changed it at some time or another. I was having the same problem a few months back and ran into this.

@larskouwenhoven
Copy link

Is there any outlook on a solution to this issue? It's the one reason keeping me from switching to Min, since I e.g. use Feedly on a daily basis, and cannot log in now.

@PalmerAL
Copy link
Collaborator

It works if you add nativeWindowOpen: true here and remove this. However, I don't think we actually want websites to be able to open arbitrary popup windows, since that's easy to abuse. Ideally, we could make the new window open in a tab, but Electron doesn't support that right now; I'm not sure how much work it would be to implement. Alternatively, we could make a popup blocker, and require you to give permission to websites before they can open popups. I think that would be simple-ish to do, although it would be rather annoying to use.

@benstigsen
Copy link

I think the permission popup would be the best approach or some setting that allows popups globally. Because having to mess with the code to be able to access sites that require Google authentication popup doesn't seem very user-friendly.

@sgreenden
Copy link

I can't log into Rateyourmusic.com

@PalmerAL
Copy link
Collaborator

1.5-ish year update on this: A few months ago, Electron got an (unofficial, unsupported) option to open popup windows as a BrowserView, which would allow them to act like a normal tab. This would address the issues with supporting popup windows that I previously mentioned.

I tried implementing it, and it mostly works, and solves the compatibility issues. However, it's fairly buggy - preload scripts don't always run, and closing a popup window will sometimes cause the whole app to crash (which someone else also reported).

(If anyone wants to try it out, it's here: https://github.com/minbrowser/min/tree/popup-window-exp)

If that doesn't work out (which it looks like is the case; I don't think it's acceptable to ship that with the current level of bugginess), then the options are:

  • Allow every site to open "normal" popup windows (which create a new window instead of a new tab). These are well-supported by Electron, but a) we lose the ability to display a browser UI around them, and b) this breaks some other sites, which use window.open as a way to create new tabs, and so creating an actual window doesn't make sense.
  • (What I'm planning to do now) create an exception for auth providers (google accounts, github, etc.) which allows links to those auth pages to open as a popup window. Everything else gets opened in a new tab as it currently does.
    • Benefits: Solves the majority of the issues reported here, avoids the risk of crashes and using unsupported features of Electron.
    • Downside: some sites will still be broken.

@PalmerAL PalmerAL changed the title Toneden free download - redirect failure [window.open not supported] Improve window.open support May 10, 2021
@basilioss
Copy link

basilioss commented May 31, 2021

This browser looks amazing and it's very comfortable. But this bug stops me from using this program because I can't sigh in to some sites. I hope this will be fixed in the near future.

@PalmerAL
Copy link
Collaborator

This was fixed in the latest release 🎉

The approach I mentioned above turned out to not be as buggy as I thought, and so I finished off the implementation and released it. Most of the issues mentioned here should be fixed now. If you see any more, please let me know or open a new issue!

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

No branches or pull requests

9 participants