webtorrent / webtorrent-desktop

❤️ Streaming torrent app for Mac, Windows, and Linux
https://webtorrent.io/desktop
MIT License
9.62k stars 997 forks source link

Electron 27 update + fix #2388

Closed bendlas closed 3 months ago

bendlas commented 8 months ago

[ ] Documentation update [x] Bug fix [ ] New feature [ ] Other, please explain:

What changes did you make? (Give an overview)

Event format seems to have changed, so don't use event.sender, but main.win in main/windows/main.js

Which issue (if any) does this pull request address?

Supersedes https://github.com/webtorrent/webtorrent-desktop/pull/2385

Is there anything you'd like reviewers to focus on?

welcome[bot] commented 8 months ago

🙌 Thanks for opening this pull request! You're awesome.

socket-security[bot] commented 8 months ago

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/electron-packager@17.1.2 filesystem Transitive: environment, eval, network, shell, unsafe +80 6.69 MB vertedinde
npm/electron@27.0.2 environment, filesystem, shell Transitive: network +21 4.88 MB electron-nightly

🚮 Removed packages: npm/electron-packager@15.5.2, npm/electron@15.5.7, npm/react-dom@17.0.2, npm/react@17.0.2

View full report↗︎

github-actions[bot] commented 6 months ago

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

bendlas commented 6 months ago

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

Very much so, I would even say it's security-relevant. I'm happy to resolve the merge, if it has any chance of being reviewed.

I'm also happy to become a member and help with maintenance, if that's an option for you @feross.

Also cc @ThaUnknown, @DiegoRBaquero, since you still seem to be active in this org.

bendlas commented 6 months ago

btw, in NixOS, we're delivering this right now, in order to keep pace as we deprecate old electron versions: https://github.com/NixOS/nixpkgs/blob/master/pkgs/applications/video/webtorrent_desktop/default.nix#L12-L23

farribeiro commented 6 months ago

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

Very much so, I would even say it's security-relevant. I'm happy to resolve the merge, if it has any chance of being reviewed.

I'm also happy to become a member and help with maintenance, if that's an option for you @feross.

Also cc @ThaUnknown, @DiegoRBaquero, since you still seem to be active in this org.

ofc... It would be interesting for you to contact the @feross asking to join the organization

ninjamuffin99 commented 5 months ago

just tested and works good 🤝

github-actions[bot] commented 3 months ago

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

bendlas commented 3 months ago

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

The only thing blocking this is lack of response from maintainers!

cc @feross @alxhotel @bnjmnt4n @Borewit @DiegoRBaquero @Flet @gillesdemey @imsnif @iovis @ivantodorovich @josephfrazier @mappum @mvayngrib @rom1504 @SilentBot1 @ThaUnknown @transitive-bullshit @watson

PS as stated previously, I'm willing to step up and take maintainership

ThaUnknown commented 3 months ago

pinging 18 people? do you have a death wish?

gillesdemey commented 3 months ago

Took this for a quick spin on MacOS and the app doesn't appear to be interactive – seems the main culprit is applying -webkit-app-region: drag; on the entire application instead of just the header.

.app {
  -webkit-app-region: drag;
}

should be

.header {
  -webkit-app-region: drag;
}

Can you also rebase with master?

farribeiro commented 3 months ago

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

The only thing blocking this is lack of response from maintainers!

cc @feross @alxhotel @bnjmnt4n @Borewit @DiegoRBaquero @Flet @gillesdemey @imsnif @iovis @ivantodorovich @josephfrazier @mappum @mvayngrib @rom1504 @SilentBot1 @ThaUnknown @transitive-bullshit @watson

PS as stated previously, I'm willing to step up and take maintainership

try on Discord

ThaUnknown commented 3 months ago

yet another 18 pings, how wonderful

bendlas commented 3 months ago

try on Discord

In fact I have done that a few weeks back https://discord.com/channels/612575111718895616/841760353439973376/1189956775680671854

pinging 18 people? do you have a death wish?

What's that supposed to mean? I know you didn't just issue a credible threat to my life in a public forum, so I can only assume that this is some sort of hyperbole?

rom1504 commented 3 months ago

@bendlas does it work? Comments above seem to indicate it doesn't

bendlas commented 3 months ago

@rom1504 It has been working on linux and it has been what we're shipping on NixOS, due to older versions of electron being disabled for security reasons.

bendlas commented 3 months ago

@gillesdemey thanks for the constructive feedback! Would you mind re-testing? I don't have access to a MacOS machine ..

farribeiro commented 3 months ago

Don't forget to update WebTorrent to version 2.x

bendlas commented 3 months ago

Don't forget to update WebTorrent to version 2.x

Are you sure that should be done as part of this PR?

gillesdemey commented 3 months ago

@gillesdemey thanks for the constructive feedback! Would you mind re-testing? I don't have access to a MacOS machine

Issue is resolved; I think we're good to merge this :)

Don't forget to update WebTorrent to version 2.x

I'd rather see a separate PR to upgrade the webtorrent dependency since that's a major version bump (and it could block this PR).

It could include some patches for dependencies – @bendlas if you're concerned about those I encourage you to send a PR for that too ;)

welcome[bot] commented 3 months ago

🎉 Congrats on getting your first pull request landed!

farribeiro commented 3 months ago

yeah... finally