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(plugin): WebPWA #2994

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

feat(plugin): WebPWA #2994

wants to merge 9 commits into from

Conversation

ThaUnknown
Copy link

@ThaUnknown ThaUnknown commented Nov 3, 2024

This plugin makes discord installable as a PWA when used in browser, it's effectively a more than slightly worse version of Vesktop, but since it can re-use a browser that's already in use, does wonders for reducing RAM usage even further on devices which have little of it.

This enables vencord on web to:

  • show notification count
  • launch in separate window
  • launch in a window without the top menu bar
  • launch on startup

Most of this code is re-used from vesktop with slight alterations

Copy link
Contributor

@Sqaaakoi Sqaaakoi left a comment

Choose a reason for hiding this comment

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

Brilliant idea with a few too many assumptions about where the code is running.

Ignore the outdated stuff, you made a lot of changes did many force pushes while I was trying to review this.

src/plugins/webPWA/index.ts Outdated Show resolved Hide resolved
src/plugins/webPWA/index.ts Outdated Show resolved Hide resolved
src/plugins/webPWA/index.ts Outdated Show resolved Hide resolved
src/plugins/webPWA/index.ts Outdated Show resolved Hide resolved
src/plugins/webPWA/index.ts Outdated Show resolved Hide resolved
src/plugins/webPWA/index.ts Outdated Show resolved Hide resolved
src/plugins/webPWA/index.ts Outdated Show resolved Hide resolved
src/plugins/webPWA/index.ts Outdated Show resolved Hide resolved
@ThaUnknown
Copy link
Author

@Sqaaakoi any chance if you know if discord still hardcodes 720p 30fps for screensharing? I remember from GM days the browser code looked something like:

screenShare(videoQuality, audioQuality) {
// just ignore the settings lol
return navigator.mediaDevices.getDisplayMedia({ video: { height: 720, frameRate: 30 } })
}

but I'm struggling to navigate the new minified code, I wanted to fix that in this plugin too, but I can't verify if it's an issue

@Sqaaakoi
Copy link
Contributor

Sqaaakoi commented Nov 4, 2024

@Sqaaakoi any chance if you know if discord still hardcodes 720p 30fps for screensharing? I remember from GM days the browser code looked something like:

screenShare(videoQuality, audioQuality) {
// just ignore the settings lol
return navigator.mediaDevices.getDisplayMedia({ video: { height: 720, frameRate: 30 } })
}

but I'm struggling to navigate the new minified code, I wanted to fix that in this plugin too, but I can't verify if it's an issue

I'm pretty sure it does, but do not count on my word.

I am not sure if this plugin is the right place to be doing it, 2 reasons

  1. Large PRs often get ignored by maintainers.
  2. This would be better suited as a port of the existing Vesktop code to support web as well as Electron. If you want to do this, may I suggest asking in the Discord server (likely #vesktop-development, you may need to ask for permissions through modmail, do state your purpose as they might suggest somewhere else to be better suited) for help?

@xPaw
Copy link
Contributor

xPaw commented Nov 5, 2024

I suspect this isn't going to work when using the userscript version because of Discord's CSP. I tried to manually inject the webmanifest just by copying the code out of here:

Refused to load manifest from 'blob:https://discord.com/2720f46f-fefa-4280-ac70-2df63a550683' because it violates the following Content Security Policy directive: "default-src 'self'". Note that 'manifest-src' was not explicitly set, so 'default-src' is used as a fallback.

@ThaUnknown
Copy link
Author

I'm unsure why? This plugin works fine on my end? I'm running brave with the vencord extension

@xPaw
Copy link
Contributor

xPaw commented Nov 5, 2024

I mean the userscript version of Vencord, not the extension.

@ThaUnknown
Copy link
Author

ah yeah, it defo won't work there, nothing you can do about it, but it's a non issue, the extension is fine, and it's not heavy or problematic in any ways, the bigger issue is discord calls randomly dying because chromium suspends audiocontext in the tab for inactivity

@xPaw
Copy link
Contributor

xPaw commented Nov 5, 2024

I tried the extension build, and it's dying on document.querySelector('link[rel="icon"]') being null. For some reason Discord replaces that icon on load with a base64 version? Don't know why it's not being found by this code during load.

image

@ThaUnknown
Copy link
Author

ah right, that'd make sense, hmmmm

@ThaUnknown
Copy link
Author

@xPaw try that

@xPaw
Copy link
Contributor

xPaw commented Nov 5, 2024

That worked. However the scope needs to be scope: location.origin because it's trying to make a relative url to the base64 blob (Chrome reports this as an invalid url).

window-controls-overlay should be removed if you don't plan on handling that. Trying to hide control just draws it on top of the discord ui:
image

@ThaUnknown
Copy link
Author

because it's trying to make a relative url to the base64 blob (Chrome reports this as an invalid url).

que? this part works fine for me?

window-controls-overlay should be removed if you don't plan on handling that.

that's what this is for

@xPaw
Copy link
Contributor

xPaw commented Nov 5, 2024

que? this part works fine for me?

open devtools and look at the application > manifest, it reports an invalid scope url.

that's what this is for

I don't know what that does, if anything.

@ThaUnknown
Copy link
Author

I'm uncertain why that patch wouldn't work for you, I copied it 1:1 from vesktop

@xPaw
Copy link
Contributor

xPaw commented Nov 5, 2024

Does it for you? Can you make a screenshot.

@ThaUnknown
Copy link
Author

ThaUnknown commented Nov 5, 2024

I'm doing these updates from my lap on my phone, so not really no

@xPaw
Copy link
Contributor

xPaw commented Nov 5, 2024

@ThaUnknown
Copy link
Author

yes im aware, but i'm patching discord to use native title bar, it's not for some reason

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

Successfully merging this pull request may close these issues.

4 participants