Issue 87 - Javascript Injection to Browser component #137

Merged

kuhnchris merged 0 commits from master into master 2021-01-07 23:49:32 -06:00

Conversation 84 Commits - Files changed -

kuhnchris commented 2018-10-06 16:21:41 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Hey there!

I felt the urge to fix this issue, as I was having some ideas regarding what to display (for example just a certain issue in GitHub) on my stream. So I added this very rudimentary version.
It compiled fine and works just as expected, with following example:
https://github.com/SpiegelEiXXL-WGJ-Creations/WGJ64-DungeonCrawiching/issues/6
+
document.body.innerHTML = document.getElementsByClassName("unminimized-comment")[0].outerHTML;

Closes #87

=>

You DEFINITLY have to watch the console tho, since javascript errors will be reported just there, any disappear in the everlasting log of messages:

If you need anything else regarding this issue, let me know!

Hey there! I felt the urge to fix this issue, as I was having some ideas regarding what to display (for example just a certain issue in GitHub) on my stream. So I added this very rudimentary version. It compiled fine and works just as expected, with following example: https://github.com/SpiegelEiXXL-WGJ-Creations/WGJ64-DungeonCrawiching/issues/6 + document.body.innerHTML = document.getElementsByClassName("unminimized-comment")[0].outerHTML; Closes #87  =>  You DEFINITLY have to watch the console tho, since javascript errors will be reported just there, any disappear in the everlasting log of messages:  If you need anything else regarding this issue, let me know!

👍 5 👀 2

RytoEX (Migrated from github.com) reviewed 2018-10-06 16:21:41 -05:00

kuhnchris commented 2018-10-06 16:37:20 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

For quick tests, here's the tested against DMG for MacOS.
obs-studio-x64-22.0.2-168-g047e586e-modified.dmg.zip

For quick tests, here's the tested against DMG for MacOS. [obs-studio-x64-22.0.2-168-g047e586e-modified.dmg.zip](https://github.com/obsproject/obs-browser/files/2453425/obs-studio-x64-22.0.2-168-g047e586e-modified.dmg.zip)

RytoEX (Migrated from github.com) requested changes 2018-10-07 00:36:14 -05:00

RytoEX (Migrated from github.com) left a comment

• Copy link
• Copy Markdown

Hi! Thanks for submitting your first OBS Project PR. To help out, I've reviewed this PR.

In addition to my inline comments, please squash your commits into a single commit, since the second and third commits are essentially fixup commits to the first. Please also edit your final commit's message to start with a present tense verb and remove the parenthetical phrase. If you feel it's important to reference the GitHub issue in the commit message, I recommend placing that mention in the commit message body rather than its subject.

Hi! Thanks for submitting your first OBS Project PR. To help out, I've reviewed this PR. In addition to my inline comments, please squash your commits into a single commit, since the second and third commits are essentially fixup commits to the first. Please also edit your final commit's message to start with a present tense verb and remove the parenthetical phrase. If you feel it's important to reference the GitHub issue in the commit message, I recommend placing that mention in the commit message body rather than its subject.

browser-client.cpp

RytoEX (Migrated from github.com) commented 2018-10-06 23:16:28 -05:00

• Copy link
• Copy Markdown

Should probably specify "GitHub Issue" to differentiate it from our Mantis Issue Tracker. Though, I'm not really sure if this comment needs to be here.

Should probably specify "GitHub Issue" to differentiate it from our [Mantis Issue Tracker](https://obsproject.com/mantis/). Though, I'm not really sure if this comment needs to be here.

obs-browser-source.cpp

RytoEX (Migrated from github.com) commented 2018-10-06 23:13:55 -05:00

• Copy link
• Copy Markdown

Style: Here, you would use spaces for alignment, rather than using a tab to indent.

Style: Here, you would use spaces for alignment, rather than using a tab to indent.

obs-browser-source.hpp

RytoEX (Migrated from github.com) commented 2018-10-06 23:14:15 -05:00

• Copy link
• Copy Markdown

Style: Here, you would use spaces for alignment, rather than using tabs to indent.

Style: Here, you would use spaces for alignment, rather than using tabs to indent.

kuhnchris (Migrated from github.com) reviewed 2018-10-07 05:52:05 -05:00

browser-client.cpp

kuhnchris (Migrated from github.com) commented 2018-10-07 05:52:05 -05:00

• Copy link
• Copy Markdown

If you do not want a reference there I can remove it no problemo.

If you do not want a reference there I can remove it no problemo.

kuhnchris commented 2018-10-07 11:08:35 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Thanks for checking my source changes! I made your suggested correction, but I miserably fail at squashing the commit (with rebase and/or merge). Anything I need to watch out?

Thanks!

Thanks for checking my source changes! I made your suggested correction, but I miserably fail at squashing the commit (with rebase and/or merge). Anything I need to watch out? Thanks!

RytoEX commented 2018-10-07 19:34:31 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

If you're working from the git commandline, and this repo is set as "upstream" and your fork is set as "origin" you'd do something like this:

git checkout master
git fetch upstream
git rebase -i upstream/master

Your default text editor may pop up here with a temporary file. For the first commit, change "pick" to "reword" (the first word of the line). For the following commits, change "pick" to "fixup". Save the file, and close the text editor.

Your default text editor may pop up again here with another temporary file. Edit the commit message to be "Add JavaScript injection". Save the file, and close the text editor.

Git should wrap up its rebase work after this, and then you should be able to force push to your branch with the following:

git push -f origin

Let us know if you have any issues.

If you're working from the git commandline, and this repo is set as "upstream" and your fork is set as "origin" you'd do something like this: ``` git checkout master git fetch upstream git rebase -i upstream/master ``` Your default text editor may pop up here with a temporary file. For the first commit, change "pick" to "reword" (the first word of the line). For the following commits, change "pick" to "fixup". Save the file, and close the text editor. Your default text editor may pop up again here with another temporary file. Edit the commit message to be "Add JavaScript injection". Save the file, and close the text editor. Git should wrap up its rebase work after this, and then you should be able to force push to your branch with the following: ``` git push -f origin ``` Let us know if you have any issues.

RytoEX commented 2018-10-07 19:40:06 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

If you're concerned about making a mistake, you can always make a backup of your branch locally with this before starting your rebase:

git checkout master
git checkout -b javascript-injection
git checkout master

If you're concerned about making a mistake, you can always make a backup of your branch locally with this before starting your rebase: ``` git checkout master git checkout -b javascript-injection git checkout master ```

kuhnchris commented 2018-10-08 16:23:20 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Hello,

thanks so much again for your input - i squashed the commit, so it's 1 commit now.

All the best,
Chris

Hello, thanks so much again for your input - i squashed the commit, so it's 1 commit now. All the best, Chris

DDRBoxman commented 2018-10-08 18:37:27 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Can we make the default injected javascript some commented out warning about pasting random code?

Discord's console has a pretty good warning on it. I just want to make sure that users don't get cookies, etc. stolen

Can we make the default injected javascript some commented out warning about pasting random code? Discord's console has a pretty good warning on it. I just want to make sure that users don't get cookies, etc. stolen 

WizardCM commented 2018-10-08 18:50:07 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

I'd also vote to make the JS input disabled by default, and require the user tick a box and accept a warning, if possible.

I'd also vote to make the JS input disabled by default, and require the user tick a box and accept a warning, if possible.

kuhnchris commented 2018-10-09 01:17:33 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

If that's the case we need to do the same for the "css" aswell, as that one works by injecting javascript to "load" the custom css into the webpage. If that's fine by you guys I will add 2 checkboxes for that ("I understand the risks of injecting (css or javascript) into the website above and know what I am doing").

If that's the case we need to do the same for the "css" aswell, as that one works by injecting javascript to "load" the custom css into the webpage. If that's fine by you guys I will add 2 checkboxes for that ("I understand the risks of injecting (css or javascript) into the website above and know what I am doing").

kuhnchris commented 2018-10-09 02:09:00 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Added your request(s). Please add a working description to the translation(s) so you guys feel safer. ;-)

Added your request(s). Please add a working description to the translation(s) so you guys feel safer. ;-) 

OsirisNL commented 2018-10-09 02:26:51 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

If that's the case we need to do the same for the "css" aswell, as that one works by injecting javascript to "load" the custom css into the webpage. If that's fine by you guys I will add 2 checkboxes for that ("I understand the risks of injecting (css or javascript) into the website above and know what I am doing").

The CSS option does not inject Javascript, it executes specific javascript code to create a link tag containg the CSS as base64 data (the actual CSS is injected). So this is not remotely as dangerous as injecting arbitrary javascript code. A checkbox for the CSS field is therefore not needed.

> If that's the case we need to do the same for the "css" aswell, as that one works by injecting javascript to "load" the custom css into the webpage. If that's fine by you guys I will add 2 checkboxes for that ("I understand the risks of injecting (css or javascript) into the website above and know what I am doing"). The CSS option does not inject Javascript, it executes specific javascript code to create a link tag containg the CSS as base64 data (the actual CSS is injected). So this is not remotely as dangerous as injecting arbitrary javascript code. A checkbox for the CSS field is therefore not needed.

kuhnchris commented 2018-10-09 02:32:37 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Well, I did get some problems regarding Cross-Site-Scripting (edit: it's actually the Content Security Policy, sorry for the confusion) when I tried to interfere with the GitHub link, hence my thinking of adding a box there aswell, but, if you do not want to, I can remove that box again, no problemo.

info: obs-browser: Refused to load the stylesheet 'data:text/css;charset=utf-8;base64,Ym9keSB7IGJhY2tncm91bmQtY29sb3I6IHJnYmEoMCwgMCwgMCwgMCk7IG1hcmdpbjogMHB4IGF1dG87IG92ZXJmbG93OiBoaWRkZW47IH0=' because it violates the following Content Security Policy directive: "style-src 'unsafe-inline' assets-cdn.github.com".
 (source: data:text/css;charset=utf-8;base64,Ym9keSB7IGJhY2tncm91bmQtY29sb3I6IHJnYmEoMCwgMCwgMCwgMCk7IG1hcmdpbjogMHB4IGF1dG87IG92ZXJmbG93OiBoaWRkZW47IH0=:1)
[1009/093347.965961:INFO:CONSOLE(1)] "Refused to load the stylesheet 'data:text/css;charset=utf-8;base64,Ym9keSB7IGJhY2tncm91bmQtY29sb3I6IHJnYmEoMCwgMCwgMCwgMCk7IG1hcmdpbjogMHB4IGF1dG87IG92ZXJmbG93OiBoaWRkZW47IH0=' because it violates the following Content Security Policy directive: "style-src 'unsafe-inline' assets-cdn.github.com".
", source: data:text/css;charset=utf-8;base64,Ym9keSB7IGJhY2tncm91bmQtY29sb3I6IHJnYmEoMCwgMCwgMCwgMCk7IG1hcmdpbjogMHB4IGF1dG87IG92ZXJmbG93OiBoaWRkZW47IH0= (1)

Well, I did get some problems regarding Cross-Site-Scripting (edit: it's actually the Content Security Policy, sorry for the confusion) when I tried to interfere with the GitHub link, hence my thinking of adding a box there aswell, but, if you do not want to, I can remove that box again, no problemo. ``` info: obs-browser: Refused to load the stylesheet 'data:text/css;charset=utf-8;base64,Ym9keSB7IGJhY2tncm91bmQtY29sb3I6IHJnYmEoMCwgMCwgMCwgMCk7IG1hcmdpbjogMHB4IGF1dG87IG92ZXJmbG93OiBoaWRkZW47IH0=' because it violates the following Content Security Policy directive: "style-src 'unsafe-inline' assets-cdn.github.com". (source: data:text/css;charset=utf-8;base64,Ym9keSB7IGJhY2tncm91bmQtY29sb3I6IHJnYmEoMCwgMCwgMCwgMCk7IG1hcmdpbjogMHB4IGF1dG87IG92ZXJmbG93OiBoaWRkZW47IH0=:1) [1009/093347.965961:INFO:CONSOLE(1)] "Refused to load the stylesheet 'data:text/css;charset=utf-8;base64,Ym9keSB7IGJhY2tncm91bmQtY29sb3I6IHJnYmEoMCwgMCwgMCwgMCk7IG1hcmdpbjogMHB4IGF1dG87IG92ZXJmbG93OiBoaWRkZW47IH0=' because it violates the following Content Security Policy directive: "style-src 'unsafe-inline' assets-cdn.github.com". ", source: data:text/css;charset=utf-8;base64,Ym9keSB7IGJhY2tncm91bmQtY29sb3I6IHJnYmEoMCwgMCwgMCwgMCk7IG1hcmdpbjogMHB4IGF1dG87IG92ZXJmbG93OiBoaWRkZW47IH0= (1) ```

OsirisNL commented 2018-10-09 02:46:25 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

That's just Chromium not allowing the css because the site's Content Security Policy doesn't accept the data: scheme as a source for css.
Though that might be circumvented by adding it as a

That's just Chromium not allowing the css because the site's Content Security Policy doesn't accept the data: scheme as a source for css. Though that might be circumvented by adding it as a <style> tag, but that's another issue.

RytoEX (Migrated from github.com) requested changes 2018-10-09 07:50:56 -05:00

RytoEX (Migrated from github.com) left a comment

• Copy link
• Copy Markdown

I've gone over this again. Please see my inline comments. It looks like you're continually changing tabs into spaces. Your IDE may be doing this automatically for you. Please keep an eye out for these whitespace conversions, as they do not follow the project's style preferences.

The commit message still uses past tense verbs. It should be using present tense verbs (see the commit history for examples). It also still refers to "OBS-Browser", but it shouldn't need to since that is this project. It also still refers to GitHub Issue #83 rather than GitHub Issue #87.

This is how the commit message should look:

Add custom JavaScript injection

Add custom JavaScript injection, solving GitHub Issue #87. Requires a 
user to confirm they want to inject JavaScript via a confirmation 
checkbox.

I've gone over this again. Please see my inline comments. It looks like you're continually changing tabs into spaces. Your IDE may be doing this automatically for you. Please keep an eye out for these whitespace conversions, as they do not follow the project's style preferences. The commit message still uses past tense verbs. It should be using present tense verbs (see the [commit history](https://github.com/obsproject/obs-browser/commits/master) for examples). It also still refers to "OBS-Browser", but it shouldn't need to since that is this project. It also still refers to GitHub Issue #83 rather than GitHub Issue #87. This is how the commit message should look: ``` Add custom JavaScript injection Add custom JavaScript injection, solving GitHub Issue #87. Requires a user to confirm they want to inject JavaScript via a confirmation checkbox. ```

obs-browser-plugin.cpp

RytoEX (Migrated from github.com) commented 2018-10-09 07:46:37 -05:00

• Copy link
• Copy Markdown

Style: It looks like the indentation here was changed from a tab to spaces. Please revert this change, and use a tab to indent in the following lines.

Style: It looks like the indentation here was changed from a tab to spaces. Please revert this change, and use a tab to indent in the following lines.

obs-browser-plugin.cpp

RytoEX (Migrated from github.com) commented 2018-10-09 07:47:57 -05:00

• Copy link
• Copy Markdown

Style: Spaces are used throughout this block for indentation instead of tabs. Please fix these lines to use tabs for indentation like the surrounding lines.

Style: Spaces are used throughout this block for indentation instead of tabs. Please fix these lines to use tabs for indentation like the surrounding lines.

obs-browser-source.cpp

RytoEX (Migrated from github.com) commented 2018-10-09 07:48:18 -05:00

• Copy link
• Copy Markdown

Style: Spaces have been used here for indentation. Please use tabs instead.

Style: Spaces have been used here for indentation. Please use tabs instead.

RytoEX (Migrated from github.com) commented 2018-10-09 07:48:28 -05:00

• Copy link
• Copy Markdown

Style: Spaces have been used here for indentation. Please use tabs instead.

Style: Spaces have been used here for indentation. Please use tabs instead.

obs-browser-source.cpp

RytoEX (Migrated from github.com) commented 2018-10-09 07:49:01 -05:00

• Copy link
• Copy Markdown

Style: Use tabs to indent, and then spaces to align this and the following line. Use the lines above as examples.

Style: Use tabs to indent, and then spaces to align this and the following line. Use the lines above as examples.

obs-browser-source.cpp

RytoEX (Migrated from github.com) commented 2018-10-09 07:49:20 -05:00

• Copy link
• Copy Markdown

Style: Spaces have been used here for indentation. Please use tabs instead.

Style: Spaces have been used here for indentation. Please use tabs instead.

RytoEX (Migrated from github.com) commented 2018-10-09 07:49:25 -05:00

• Copy link
• Copy Markdown

Style: Spaces have been used here for indentation. Please use tabs instead.

Style: Spaces have been used here for indentation. Please use tabs instead.

obs-browser-source.hpp

RytoEX (Migrated from github.com) commented 2018-10-09 07:49:33 -05:00

• Copy link
• Copy Markdown

Style: Spaces have been used here for indentation. Please use a tab instead.

Style: Spaces have been used here for indentation. Please use a tab instead.

RytoEX (Migrated from github.com) commented 2018-10-09 07:49:37 -05:00

• Copy link
• Copy Markdown

Style: Spaces have been used here for indentation. Please use a tab instead.

Style: Spaces have been used here for indentation. Please use a tab instead.

Enoxs commented 2018-10-09 11:06:37 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

What about windows platform?

What about windows platform?

OsirisNL commented 2018-10-09 12:22:18 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

@Enoxs what do you mean?

@Enoxs what do you mean?

kuhnchris commented 2018-10-09 12:33:43 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

You mean a pre-built binary for windows @Enoxs ? If yes, you will have to wait for the next official release and for this commit to be merged into master.
I sadly do not have a Windows developer machine on hand so I cannot compile a version for you to test right now.

You mean a pre-built binary for windows @Enoxs ? If yes, you will have to wait for the next official release and for this commit to be merged into master. I sadly do not have a Windows developer machine on hand so I cannot compile a version for you to test right now.

kuhnchris (Migrated from github.com) reviewed 2018-10-09 12:36:02 -05:00

obs-browser-source.hpp

kuhnchris (Migrated from github.com) commented 2018-10-09 12:36:01 -05:00

• Copy link
• Copy Markdown

Ah sorry, I was under the impression that you want ONLY spaces. It's a while since I saw mixed indentations required. Will fix while removing the CSS flag as requested above.

Ah sorry, I was under the impression that you want ONLY spaces. It's a while since I saw mixed indentations required. Will fix while removing the CSS flag as requested above.

kuhnchris commented 2018-10-09 12:55:55 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

@RytoEX thanks again for checking, I checked in the changes you mentioned, checked against vim this time, so the tabbing should be better...
Also changed the commit text. If that's better i'll resquash and use a present tense commit text.
Also, do you need the string(s) added to the translation files?

@RytoEX thanks again for checking, I checked in the changes you mentioned, checked against vim this time, so the tabbing should be better... Also changed the commit text. If that's better i'll resquash and use a present tense commit text. Also, do you need the string(s) added to the translation files?

RytoEX (Migrated from github.com) requested changes 2018-10-09 14:24:40 -05:00

RytoEX (Migrated from github.com) left a comment

• Copy link
• Copy Markdown

Another review. Please note, I did not mark every single line where an issue occurs. Please apply my requested changes throughout your PR.

Yes, you will need to add appropriate lang strings for the two strings that you've added. You only have to add them for English (en-US). Translations are done via Crowdin.

Another review. Please note, I did not mark every single line where an issue occurs. Please apply my requested changes throughout your PR. Yes, you will need to add appropriate lang strings for the two strings that you've added. You only have to add them for English (en-US). Translations are done via Crowdin.

browser-client.cpp

RytoEX (Migrated from github.com) commented 2018-10-09 14:16:05 -05:00

• Copy link
• Copy Markdown

Just remove this code instead of commenting it out.

Just remove this code instead of commenting it out.

RytoEX (Migrated from github.com) commented 2018-10-09 14:19:44 -05:00

• Copy link
• Copy Markdown

Avoid unnecessary/unrelated whitespace changes.

Avoid unnecessary/unrelated whitespace changes.

browser-client.cpp

RytoEX (Migrated from github.com) commented 2018-10-09 14:16:46 -05:00

• Copy link
• Copy Markdown

Style: Place a single space between the closing parenthesis and the opening curly brace.

Style: Place a single space between the closing parenthesis and the opening curly brace.

obs-browser-plugin.cpp

RytoEX (Migrated from github.com) commented 2018-10-09 14:21:42 -05:00

• Copy link
• Copy Markdown

Nitpick: JavaScript is the correct capitalization/stylization.

Nitpick: JavaScript is the correct capitalization/stylization.

RytoEX (Migrated from github.com) commented 2018-10-09 14:21:50 -05:00

• Copy link
• Copy Markdown

Nitpick: JavaScript is the correct capitalization/stylization.

Nitpick: JavaScript is the correct capitalization/stylization.

obs-browser-source.cpp

RytoEX (Migrated from github.com) commented 2018-10-09 14:18:48 -05:00

• Copy link
• Copy Markdown

Style: Place this at the end of the preceding line. Curly braces should not have their own line like this.

Style: Place this at the end of the preceding line. Curly braces should not have their own line like this.

kuhnchris commented 2018-10-09 16:56:28 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Once again, thanks! One question: is there any implicit limit to the number of characters for the texts in the plugin configuration dialog? I started out with the following text, but I think the length seems unpracticable.

Once again, thanks! One question: is there any implicit limit to the number of characters for the texts in the plugin configuration dialog? I started out with the following text, but I think the length seems unpracticable. 

Voldi87 commented 2019-04-17 09:21:59 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Lame question, but how can I open the Javascript box?

Lame question, but how can I open the Javascript box?

kuhnchris commented 2019-04-17 11:26:24 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Hey there @Voldi87 - the commit isnt productive yet, so sadly you'd have to compile it yourself. The "open" the settings box you simply configure the browser component

Hey there @Voldi87 - the commit isnt productive yet, so sadly you'd have to compile it yourself. The "open" the settings box you simply configure the browser component

Voldi87 commented 2019-04-17 12:21:57 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Hi,

I don't exactly understand this. I've downloaded it and installed it for
windows 64bit from releases menu (v1.31).

And then what should I do?

KuhnChris notifications@github.com ezt írta (időpont: 2019. ápr. 17., Sze
18:26):

Hey there @Voldi87 https://github.com/Voldi87 - the commit isnt
productive yet, so sadly you'd have to compile it yourself. The "open" the
settings box you simply configure the browser component

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/obsproject/obs-browser/pull/137#issuecomment-484162939,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AvZK3SGEUk-NLqZiTX2o5eG-neQFKo6Lks5vh0szgaJpZM4XLjuC
.

Hi, I don't exactly understand this. I've downloaded it and installed it for windows 64bit from releases menu (v1.31). And then what should I do? KuhnChris <notifications@github.com> ezt írta (időpont: 2019. ápr. 17., Sze 18:26): > Hey there @Voldi87 <https://github.com/Voldi87> - the commit isnt > productive yet, so sadly you'd have to compile it yourself. The "open" the > settings box you simply configure the browser component > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <https://github.com/obsproject/obs-browser/pull/137#issuecomment-484162939>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AvZK3SGEUk-NLqZiTX2o5eG-neQFKo6Lks5vh0szgaJpZM4XLjuC> > . >

kuhnchris commented 2019-04-18 01:54:48 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Hi, I don't exactly understand this. I've downloaded it and installed it for windows 64bit from releases menu (v1.31). And then what should I do? KuhnChris notifications@github.com ezt írta (időpont: 2019. ápr. 17., Sze 18:26):
…
Hey there @Voldi87 https://github.com/Voldi87 - the commit isnt productive yet, so sadly you'd have to compile it yourself. The "open" the settings box you simply configure the browser component — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#137 (comment)>, or mute the thread https://github.com/notifications/unsubscribe-auth/AvZK3SGEUk-NLqZiTX2o5eG-neQFKo6Lks5vh0szgaJpZM4XLjuC .

As I stated: This is not merged yet.
If you downloaded the official release of OBS, then you cannot use this change yet.

> Hi, I don't exactly understand this. I've downloaded it and installed it for windows 64bit from releases menu (v1.31). And then what should I do? KuhnChris <notifications@github.com> ezt írta (időpont: 2019. ápr. 17., Sze 18:26): > […](#) > Hey there @Voldi87 <https://github.com/Voldi87> - the commit isnt productive yet, so sadly you'd have to compile it yourself. The "open" the settings box you simply configure the browser component — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <[#137 (comment)](https://github.com/obsproject/obs-browser/pull/137#issuecomment-484162939)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AvZK3SGEUk-NLqZiTX2o5eG-neQFKo6Lks5vh0szgaJpZM4XLjuC> . As I stated: This is not merged yet. If you downloaded the official release of OBS, then you cannot use this change yet.

chrisjameschamp commented 2019-05-20 13:17:11 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

When is this going to be merged? This would solve a gang load of my problems

When is this going to be merged? This would solve a gang load of my problems

kuhnchris commented 2019-05-20 14:47:02 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

@semaj4712 i think I left a question for @RytoEX but I haven't got a reply (regarding the length of the text). Anyways, I am not a member of obs-browser so I sadly cannot tell you anything about a time-line when/how/if this is going to get merged. :-(

@RytoEX if I can help in any way, let me know

@semaj4712 i think I left a question for @RytoEX but I haven't got a reply (regarding the length of the text). Anyways, I am not a member of obs-browser so I sadly cannot tell you anything about a time-line when/how/if this is going to get merged. :-( @RytoEX if I can help in any way, let me know

chrisjameschamp commented 2019-05-20 15:21:18 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Would it not be possible implement this as a third party plugin?

Would it not be possible implement this as a third party plugin?

kuhnchris commented 2019-05-20 16:13:59 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Well, you can always fork this and compile it yourself, and replace obs-browser with it, if you need it "quickly", but in my opinion it wouldn't make sense to split a component off just because of an additional functionality.

You can, however, go ahead and do that if you want. The code is up there if you want to try. :-)

Well, you can always fork this and compile it yourself, and replace obs-browser with it, if you need it "quickly", but in my opinion it wouldn't make sense to split a component off just because of an additional functionality. You can, however, go ahead and do that if you want. The code is up there if you want to try. :-)

kuhnchris (Migrated from github.com) reviewed 2019-05-21 01:58:50 -05:00

kuhnchris (Migrated from github.com) left a comment

• Copy link
• Copy Markdown

addressed

addressed

chrisjameschamp commented 2019-05-21 11:55:09 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

@kuhnchris I think compiling it myself is a little out of the spectrum of what I can do, literally no idea how to even begin, hopefully these changes get implemented in the pre-compiled version of OBS soon

@kuhnchris I think compiling it myself is a little out of the spectrum of what I can do, literally no idea how to even begin, hopefully these changes get implemented in the pre-compiled version of OBS soon

kuhnchris commented 2019-05-21 16:01:24 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

For what OS do you actually need it compiled? I added a DMG up there (proceed with caution please) - so since there seems to be interest I could wipe up a DLL for windows, if you guys want to help out with the testing?

Thanks

For what OS do you actually need it compiled? I added a DMG up there (proceed with caution please) - so since there seems to be interest I could wipe up a DLL for windows, if you guys want to help out with the testing? Thanks

👍 1

chrisjameschamp commented 2019-05-21 16:58:44 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

I am on PC

I am on PC

Wiethoofd commented 2019-06-26 19:34:21 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Wouldn't it make more sense to have the 'Enable Custom Javascript Field in Browser Sources' setting in the Sources section in the Advanced tab of the options; where you can change the Browser Source Hardware Acceleration. Or would you want to keep this as a separate option per browser source that is active and 'not shut down' (for performance reasons?).

Wouldn't it make more sense to have the 'Enable Custom Javascript Field in Browser Sources' setting in the Sources section in the Advanced tab of the options; where you can change the _Browser Source Hardware Acceleration_. Or would you want to keep this as a separate option per browser source that is active and 'not shut down' (for performance reasons?).

kuhnchris commented 2019-06-27 06:02:32 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

I think that is something that came in after my changes (I think). I'm fine with any changes or suggestions, as long as it gets this forward ;-)

I think that is something that came in after my changes (I think). I'm fine with any changes or suggestions, as long as it gets this forward ;-)

drjameshouse commented 2020-01-31 23:00:24 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Any updates on this? I would love to try it out and test on windows. I was hoping to build myself, but I only have access to VS at work and would not be able to install QT (strict rules) so I am unable to. If someone could create DLLs for windows let me know and I can test them.

Any updates on this? I would love to try it out and test on windows. I was hoping to build myself, but I only have access to VS at work and would not be able to install QT (strict rules) so I am unable to. If someone could create DLLs for windows let me know and I can test them.

kristoff-it commented 2020-04-12 07:31:40 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

This is a 25 line PR that has been left open since 2018 and by reading the reviews it seems the code itself doesn't have any issue.

Can a maintainer please press the merge button or explain if there's a reason why the OBS project doesn't want this PR?

This is a 25 line PR that has been left open since 2018 and by reading the reviews it seems the code itself doesn't have any issue. Can a maintainer please press the merge button or explain if there's a reason why the OBS project doesn't want this PR?

WizardCM commented 2020-04-13 02:56:25 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

@kuhnchris When you have time, please squash your commits and rebase on master.

@kuhnchris When you have time, please squash your commits and rebase on master.

👍 1

kuhnchris commented 2020-04-13 06:44:03 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Sure, i'll try to rebase it (and test-build it) until the end of the week and let a comment in here as soon as I'm ready!

Sure, i'll try to rebase it (and test-build it) until the end of the week and let a comment in here as soon as I'm ready!

👍 1

kuhnchris commented 2020-04-13 11:17:53 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

I just finished compiling the win32 build of this. The text in the checkbox is a bit too long, do we want to shorten it, or rather do you have any official text regarding the warning we want to put?

Else, if no other changes are requested, I'll squish the commit(s).
Thanks!

I just finished compiling the win32 build of this. The text in the checkbox is a bit too long, do we want to shorten it, or rather do you have any official text regarding the warning we want to put?  Else, if no other changes are requested, I'll squish the commit(s). Thanks!

👍 4

WizardCM commented 2020-05-18 05:23:48 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

I believe Properties support newlines, so it's just a matter of adding an \n

I'd go with this:
JavaScriptActive="I understand the risks of injecting JavaScript into the website above.\nI did not copy random JavaScript code from strangers."

It's shorter and more to the point. I debated using the term "security risks", could see how it fits visually. What we should do is use a long_description (aka tooltip) that outlines the dangers in more detail, and that way we're not limited visually (it'll also need newlines).

I believe Properties support newlines, so it's just a matter of adding an `\n` I'd go with this: `JavaScriptActive="I understand the risks of injecting JavaScript into the website above.\nI did not copy random JavaScript code from strangers."` It's shorter and more to the point. I debated using the term "security risks", could see how it fits visually. What we should do is use a `long_description` (aka tooltip) that outlines the dangers in more detail, and that way we're not limited visually (it'll also need newlines).

kuhnchris commented 2020-05-18 05:40:52 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

OK then I'll try to add that one in and give it another build

OK then I'll try to add that one in and give it another build

kuhnchris commented 2020-06-12 12:17:27 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Sadly, negative. No "\n" support in the obs_module_text. (at least for add_bool)
I now added the following:

JavaScript="JavaScript Code"
JavaScriptActive="I understand the risks of injecting JavaScript into the website above."
JavaScript.ToolTip="Do not copy random JavaScript code from strangers."

Resulting in this:

Would this be sufficient?

Thanks,
Chris

Sadly, negative. No "\n" support in the obs_module_text. (at least for add_bool) I now added the following: ``` JavaScript="JavaScript Code" JavaScriptActive="I understand the risks of injecting JavaScript into the website above." JavaScript.ToolTip="Do not copy random JavaScript code from strangers." ``` Resulting in this:  Would this be sufficient? Thanks, Chris

rogueyoshi commented 2020-07-03 01:12:13 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

If this gets merged in, I think a toggle to enable Cross-Site Scripting (even if for only the JS code being executed here) would be appropriate too (available at least through browser flags already). It would be absurdly useful to allow XSS for stream overlays and wouldn't be much more dangerous than just allowing JS there anyways.

If this gets merged in, I think a toggle to enable Cross-Site Scripting (even if for only the JS code being executed here) would be appropriate too (available at least through browser flags already). It would be absurdly useful to allow XSS for stream overlays and wouldn't be much more dangerous than just allowing JS there anyways.

PatTheMav commented 2020-07-03 04:40:26 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Not to diminish the work that went into this, but what's the intended use case/audience for this?

Pasting arbitrary JS in any application is something that we should've collectively learned is bad™ years ago. If you need your custom JS to run, paste it into a local HTML and use that one in the plugin. Need to handle CSP and CORS? Run a local webserver and configure headers accordingly.

There is a reason why browsers don't even allow JS pasted into the location bar anymore and I'm not too keen seeing the same mistakes repeated in OBS.

Not to diminish the work that went into this, but what's the intended use case/audience for this? Pasting arbitrary JS in any application is something that we should've collectively learned is bad™ years ago. If you need your custom JS to run, paste it into a local HTML and use that one in the plugin. Need to handle CSP and CORS? Run a local webserver and configure headers accordingly. There is a reason why browsers don't even allow JS pasted into the location bar anymore and I'm not too keen seeing the same mistakes repeated in OBS.

kuhnchris commented 2020-07-03 05:00:16 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

As you described - if you just need a small change or focus on a part during stream you'd have to go through all the hoops (local webserver due to CORS, etc.), basically turning it into a project. For us devs, yes, this may be a simple project, but OBS is mainly used by people who need easy accessibility to various functionalities instead of learning all the bits and bytes.

My particular use-case was to select parts of external sites to display on stream, you could certainly do it now with CSS overwrites.

This feature is also not for "random people copy random js code into random boxes on your PC" - by this reasoning you aren't ever allowed to open cmd.exe on windows and paste commands there, or ever listen to people on the internet, for that matter. This feature is for people who know what they do, hence all the information there.

If you really think this is "dangerous" we may aswell put it behind some "Expert Mode" checkbox, but I do not know how that would integrate in the OBS design.

Also, since this feature is nearing 2 years of run time, I doubt it will ever make it in.

As you described - if you just need a small change or focus on a part during stream you'd have to go through all the hoops (local webserver due to CORS, etc.), basically turning it into a project. For us devs, yes, this may be a simple project, but OBS is mainly used by people who need easy accessibility to various functionalities instead of learning all the bits and bytes. My particular use-case was to select parts of external sites to display on stream, you could certainly do it now with CSS overwrites. This feature is also not for "random people copy random js code into random boxes on your PC" - by this reasoning you aren't ever allowed to open cmd.exe on windows and paste commands there, or ever listen to people on the internet, for that matter. This feature is for people who know what they do, hence all the information there. If you really think this is "dangerous" we may aswell put it behind some "Expert Mode" checkbox, but I do not know how that would integrate in the OBS design. Also, since this feature is nearing 2 years of run time, I doubt it will ever make it in.

PatTheMav commented 2020-07-03 05:07:52 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Well, I for one would definitely block cmd.exe for the common user, but that's just me.. 😉

I think it's really important to understand what use cases this will solve and if there is maybe another way we can make it happen for users.

Given your use case, am I right to understand that you felt it was easier to modify a browser-based overlay than using OBS' built-in tools to create a new source/layer to achieve the same? What does OBS lack to achieve the same?

Well, I for one would definitely block `cmd.exe` for the common user, but that's just me.. 😉 I think it's really important to understand what use cases this will solve and if there is maybe another way we can make it happen for users. Given your use case, am I right to understand that you felt it was easier to modify a browser-based overlay than using OBS' built-in tools to create a new source/layer to achieve the same? What does OBS lack to achieve the same?

👎 1

data-enabler commented 2020-07-03 14:25:15 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

what's the intended use case/audience for this?

I'll give you an example. The Discord voice overlay loads really tiny JPEG avatar images that look terrible when scaled up, but this can easily be fixed with a tiny bit of JavaScript. The overlay uses a non-public API to communicate with your local Discord app, so writing a replacement isn't an option, and I'm pretty sure the app checks the origin for any page that tries to use the API. So I would have to do everything that you already mentioned, plus probably edit the hosts file on the machine to send streamkit.discord.com requests to said local server. The solution I actually ended up going with was to just manually inject the JS myself by connecting to the page using Puppeteer, which is a lot more portable, but still annoying. I'd love to be able to just paste my script in a box and call it a day.

And for the record, this is a feature that XSplit has already had for a long time.

The concern for security seems a bit misplaced here. Technically speaking, I'm fairly certain the CSS injection that obs-broswer already supports can be used for nefarious ends if people are willing to just copy-paste random stuff from the internet.

> what's the intended use case/audience for this? I'll give you an example. The [Discord voice overlay](https://streamkit.discord.com/overlay) loads really tiny JPEG avatar images that look terrible when scaled up, but this can easily be fixed with a tiny bit of JavaScript. The overlay uses a non-public API to communicate with your local Discord app, so writing a replacement isn't an option, and I'm pretty sure the app checks the origin for any page that tries to use the API. So I would have to do everything that you already mentioned, plus probably edit the hosts file on the machine to send `streamkit.discord.com` requests to said local server. The solution I actually ended up going with was to just manually inject the JS myself by connecting to the page using Puppeteer, which is a lot more portable, but still annoying. I'd love to be able to just paste my script in a box and call it a day. And for the record, this is a feature that XSplit has already had for a long time. The concern for security seems a bit misplaced here. Technically speaking, I'm fairly certain the CSS injection that obs-broswer already supports can be used for nefarious ends if people are willing to just copy-paste random stuff from the internet.

👍 3

kristoff-it commented 2020-07-03 14:36:21 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

I too think the security concerns in this thread have been somewhat exaggerated.

My usecase, which I believe to be fairly common, is that I have a custom overlay that polls twitch for notifications and I don't want to hardcode the API key in the source file. Another example is a countdown overlay: I'd like to be able to change the start time without having to change the source.

Right now it's possible to make do with url fragments, but a JS box would be much more appropriate.

I too think the security concerns in this thread have been somewhat exaggerated. My usecase, which I believe to be fairly common, is that I have a custom overlay that polls twitch for notifications and I don't want to hardcode the API key in the source file. Another example is a countdown overlay: I'd like to be able to change the start time without having to change the source. Right now it's possible to make do with url fragments, but a JS box would be much more appropriate.

addmitt commented 2020-07-25 02:48:54 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

I just went looking for something like this. I want to embed Restream's chat but want to add a bit of Javascript for more advanced filters, and throttling to prevent spam from a single user.

Perhaps hide the feature in its entirety unless it is enabled from the Advanced Settings? Where more lengthy warnings could be displayed that don't inhibit the UX of the BrowserSource properties?

I just went looking for something like this. I want to embed Restream's chat but want to add a bit of Javascript for more advanced filters, and throttling to prevent spam from a single user. Perhaps hide the feature in its entirety unless it is enabled from the Advanced Settings? Where more lengthy warnings could be displayed that don't inhibit the UX of the BrowserSource properties?

PeterAkakpo commented 2020-10-25 09:22:15 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

assuming there is a javascript function called update() inside the loaded source file (.html). how do i execute such a function from OBS

assuming there is a javascript function called **update()** inside the loaded source file (.html). how do i execute such a function from OBS

ryantheleach commented 2020-12-14 20:28:00 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

As for the comment wondering whether the checkbox had enough room to display a full warning, truncate it to "Inject custom JS (Dangerous)"

Upon click, show a popup with details, asking to confirm, explain the full issue, instead of assuming the user already knows the risks.

As for the comment wondering whether the checkbox had enough room to display a full warning, truncate it to "Inject custom JS (Dangerous)" Upon click, show a popup with details, asking to confirm, explain the full issue, instead of assuming the user already knows the risks.

rogueyoshi commented 2020-12-17 01:35:47 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

@kuhnchris would it take much effort to bundle this as an external plugin for OBS? Then we could have the feature in the interim of the merge process.

@kuhnchris would it take much effort to bundle this as an external plugin for OBS? Then we could have the feature in the interim of the merge process.

kuhnchris commented 2020-12-17 11:21:00 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

@kuhnchris would it take much effort to bundle this as an external plugin for OBS? Then we could have the feature in the interim of the merge process.

Depends, we could fork it, but that would essentially create 2 plugins with the same source and someone to maintain it, plus merge all the upstream changes. I mean, technically possible, sure, but not sure if someone can be found to keep track of it.

> @kuhnchris would it take much effort to bundle this as an external plugin for OBS? Then we could have the feature in the interim of the merge process. Depends, we could fork it, but that would essentially create 2 plugins with the same source and someone to maintain it, plus merge all the upstream changes. I mean, technically possible, sure, but not sure if someone can be found to keep track of it.

rogueyoshi commented 2020-12-17 11:24:58 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

@kuhnchris I'm not sure if CI would help with that but if it's too much effort, I would at least request for a plug-in if for some reason a merge doesn't occur.

@kuhnchris I'm not sure if CI would help with that but if it's too much effort, I would at least request for a plug-in if for some reason a merge doesn't occur.

kuhnchris commented 2020-12-17 13:02:36 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Hmm, maybe there's a way to do that via GitHub Actions or Webhooks. I'll take a look at it over the weekend and let you guys know, OK?

Hmm, maybe there's a way to do that via GitHub Actions or Webhooks. I'll take a look at it over the weekend and let you guys know, OK?

👍 1

dodgepong commented 2021-01-07 09:03:48 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

my streamkit could be improved without me using Fiddler and modifying the code directly

Why would you want to type Javascript into the browser source properties when you have the ability to modify code directly? Why do you need to use Fiddler?

> my streamkit could be improved without me using Fiddler and modifying the code directly Why would you want to type Javascript into the browser source properties when you have the ability to modify code directly? Why do you need to use Fiddler?

WizardCM commented 2021-01-07 23:59:38 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Hi there, while DDRBoxman was pushing new commits, it seems the bot that you run on your repo bugged out when rebasing, so it deleted your changes. Unfortunately, this means GitHub thought this PR was merged (because the commit hash matched) and automatically closed the PR.

You'll have to revert your branch, rebase manually, then submit a new PR.

Hi there, while DDRBoxman was pushing new commits, it seems the bot that you run on your repo bugged out when rebasing, so it deleted your changes. Unfortunately, this means GitHub thought this PR was merged (because the commit hash matched) and automatically closed the PR. You'll have to revert your branch, rebase manually, then submit a new PR.

kuhnchris commented 2021-01-08 03:11:25 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Hi there, yeah I noticed. At this point I however question if I should reopen the PR at all as the original one was dated back in 2018. If we still want to have a discussion about this let me know, else I'll just be done with it for now. (we can also move this to Discord if you want)

And yes, the bot accidently force-pushed so I'd have to rebase my local copy again, I just uninstalled it so this shouldn't happen again.

Thanks,
Chris

Hi there, yeah I noticed. At this point I however question if I should reopen the PR at all as the original one was dated back in 2018. If we still want to have a discussion about this let me know, else I'll just be done with it for now. (we can also move this to Discord if you want) And yes, the bot accidently force-pushed so I'd have to rebase my local copy again, I just uninstalled it so this shouldn't happen again. Thanks, Chris

ryantheleach commented 2021-01-22 02:42:57 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Why would you want to type Javascript into the browser source properties when you have the ability to modify code directly? > Why do you need to use Fiddler?

Dodgepong, you are making the assumption that the browser source code is a file that the user controls directly. Injecting javascript is basically what the browser extensions greasemonkey/tampermonkey were built for, so it shows there is a usecase.

> Why would you want to type Javascript into the browser source properties when you have the ability to modify code directly? > Why do you need to use Fiddler? Dodgepong, you are making the assumption that the browser source code is a file that the user controls directly. Injecting javascript is basically what the browser extensions greasemonkey/tampermonkey were built for, so it shows there is a usecase.

dodgepong commented 2021-01-22 08:40:19 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Dodgepong, you are making the assumption that the browser source code is a file that the user controls directly

The comment I was replying to seems to have been deleted, but if I recall correctly it implied that their code was local, or could have been read that way.

> Dodgepong, you are making the assumption that the browser source code is a file that the user controls directly The comment I was replying to seems to have been deleted, but if I recall correctly it implied that their code was local, or could have been read that way.

ryantheleach commented 2021-01-22 11:02:11 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Sorry Dodgepong, I assumed that it was an earlier edit of: https://github.com/obsproject/obs-browser/pull/137#issuecomment-653656733

Sorry Dodgepong, I assumed that it was an earlier edit of: https://github.com/obsproject/obs-browser/pull/137#issuecomment-653656733

Codingale commented 2021-01-22 11:37:49 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

The comment I was replying to seems to have been deleted, but if I recall correctly it implied that their code was local, or could have been read that way.

Ah, I see you replied to my comment, I had deleted it since it was basically the exact same as #137 but phrased worse, sorry if I implied I could modify it without using Fiddler or similar MITM proxy.

Didn't mean to delete a comment that someone had replied to, I had woke up after a long day and re-read the comments and saw that someone already had my use case in mind. But didn't reload to see if someone replied.

Basically the code for StreamKit can be modified by users to make it more efficient. I recall a video tutorial that uses multiple browser sources for something that could be done with one JS tweak but no option exists for it. Here's the video

I just want to reaffirm I'd really like this feature as it'd enable more creativity in otherwise limited overlays without having to trust a MITM certificate or some other 3rd party means that could expose you in other ways.

> The comment I was replying to seems to have been deleted, but if I recall correctly it implied that their code was local, or could have been read that way. Ah, I see you replied to my comment, I had deleted it since it was basically the exact same as #137 but phrased worse, sorry if I implied I could modify it without using Fiddler or similar MITM proxy. Didn't mean to delete a comment that someone had replied to, I had woke up after a long day and re-read the comments and saw that someone already had my use case in mind. But didn't reload to see if someone replied. Basically the code for StreamKit can be modified by users to make it more efficient. I recall a video tutorial that uses multiple browser sources for something that could be done with one JS tweak but no option exists for it. [Here's the video](https://www.youtube.com/watch?v=I_GHGxZoahQ) I just want to reaffirm I'd really like this feature as it'd enable more creativity in otherwise limited overlays without having to trust a MITM certificate or some other 3rd party means that could expose you in other ways.

👍 3

TeamSteeve commented 2021-03-08 05:19:41 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Went looking for this exact feature. It would be so handy if I could insert some javascript that would fill the log-in page details of my nest cams (which I very much do want to be password protected).

Any chance of this making it into an OBS release?

Went looking for this exact feature. It would be so handy if I could insert some javascript that would fill the log-in page details of my nest cams (which I very much do want to be password protected). Any chance of this making it into an OBS release?

👍 3

schneidermanuel commented 2021-04-05 13:23:19 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Looking for this. Is there a simple way to get this working on Windows?

Looking for this. Is there a simple way to get this working on Windows?

willlllllio commented 2021-05-13 14:44:17 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Would be great if this ever got merged.

There are often times where streamers I know use some page as browser source and everything is mostly working except one or two tiny little things being wrong that could easily be patched with 5 lines of JS but is impossible to do within OBS. Some people here recommend local files or servers but that's usually not applicable for modifying 3rd party sites.

Examples:

• an alerts page that had one alert way way louder than others without a way to configure it
• a really great countdown timer page that required manual user interaction to start the timer every time
• a script to block loading avatar images on a leaderboard/stats overlay page for a game that allows users to set their own avatar image, leading to some setting lewd/obscene/racist pictures to catch out streamers and forcing them to not use said overlay at all
• a page that wouldn't load without a chrome only API being there on startup because of a bug in a feature detection function throwing an error on page load when run in CEF

All fixed with a couple lines of JS that you currently have to run in a full browser via a Userscript and capture in OBS instead of just throwing a couple lines of JS in the browser source properties patching those things.

Regarding security concerns, I fully get the possible dangers of it but they seem a bit overstated here. Even all major browsers let everyone open a JS console and just run any code the users pastes with maybe a warning in some of them, and have tons of extensions allowing the customization of loaded pages in various ways. The idea that any amount of hacking around on pages like this is bad™ and should be discouraged and everything being locked down and walled gardened for "user safety" seems kinda saddening to me especially in an open source project.

Regarding implementation, it would be great if it used OnLoadStart for JS injection instead of OnLoadEnd used for CSS as OnLoadStart would more easily allow overwriting browser provided APIs like for example Audio. It doesn't make much difference for CSS but for JS if you only run the code on OnLoadEnd it will often be too late already as the page's scripts are already loaded and ran and many scripts seem to get references for things like browser APIs just once on load and then keep using those forever, seems pretty common especially in bundled SPAs now. SoOnLoadStart seems nicer for injecting JS just to ensure you're running before anything else. Anyone wanting to run after load can still easily do that with the event this way too. I tried it locally and seemed to work without issues, only a couple lines change needed.

I would love seeing this merged eventually if possible. I would also be down to make any changes needed and make a new PR if that's needed to get it merged just and only in case @kuhnchris has no interest in doing that anymore. (I have no intention of stealing credit here, sorry if that comes of like it, would just really like to see this merged eventually and just offering only in case the original author isn't interested anymore).

Would be great if this ever got merged. There are often times where streamers I know use some page as browser source and everything is mostly working except one or two tiny little things being wrong that could easily be patched with 5 lines of JS but is impossible to do within OBS. Some people here recommend local files or servers but that's usually not applicable for modifying 3rd party sites. Examples: * an alerts page that had one alert way way louder than others without a way to configure it * a really great countdown timer page that required manual user interaction to start the timer every time * a script to block loading avatar images on a leaderboard/stats overlay page for a game that allows users to set their own avatar image, leading to some setting lewd/obscene/racist pictures to catch out streamers and forcing them to not use said overlay at all * a page that wouldn't load without a chrome only API being there on startup because of a bug in a feature detection function throwing an error on page load when run in CEF All fixed with a couple lines of JS that you currently have to run in a full browser via a Userscript and capture in OBS instead of just throwing a couple lines of JS in the browser source properties patching those things. Regarding security concerns, I fully get the possible dangers of it but they seem a bit overstated here. Even all major browsers let everyone open a JS console and just run any code the users pastes with maybe a warning in some of them, and have tons of extensions allowing the customization of loaded pages in various ways. The idea that any amount of hacking around on pages like this is bad™ and should be discouraged and everything being locked down and walled gardened for "user safety" seems kinda saddening to me especially in an open source project. Regarding implementation, it would be great if it used `OnLoadStart` for JS injection instead of `OnLoadEnd` used for CSS as `OnLoadStart` would more easily allow overwriting browser provided APIs like for example Audio. It doesn't make much difference for CSS but for JS if you only run the code on `OnLoadEnd` it will often be too late already as the page's scripts are already loaded and ran and many scripts seem to get references for things like browser APIs just once on load and then keep using those forever, seems pretty common especially in bundled SPAs now. So`OnLoadStart` seems nicer for injecting JS just to ensure you're running before anything else. Anyone wanting to run after load can still easily do that with the event this way too. I tried it locally and seemed to work without issues, only a couple lines change needed. I would love seeing this merged eventually if possible. I would also be down to make any changes needed and make a new PR if that's needed to get it merged just and only in case @kuhnchris has no interest in doing that anymore. (I have no intention of stealing credit here, sorry if that comes of like it, would just really like to see this merged eventually and just offering only in case the original author isn't interested anymore).

👍 4

Codingale commented 2021-05-14 06:10:36 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Would be great if this ever got merged.

https://github.com/obsproject/obs-browser/pull/137#event-4182938897

Just waiting on a new build of OBS I thought for public release?

> Would be great if this ever got merged.  https://github.com/obsproject/obs-browser/pull/137#event-4182938897 Just waiting on a new build of OBS I thought for public release?

dodgepong commented 2021-05-14 06:25:58 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Nope, that was a Github bug: https://github.com/obsproject/obs-browser/pull/137#issuecomment-756565772

Nope, that was a Github bug: https://github.com/obsproject/obs-browser/pull/137#issuecomment-756565772

Codingale commented 2021-05-14 06:38:18 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Ah, didn't get that email and didn't look closer thanks.

Ah, didn't get that email and didn't look closer thanks.

kuhnchris commented 2021-05-14 06:41:02 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Yeah I screwed up merging the recent master into my branch, hence the error.
I talked to the guys over at Discord and there is a very good reason why this is still halted, I do get their basic intention behind not merging it in, tho it takes a very long time and there still isn't a timeline announced regarding the change they are holding this back. Just to address @willlllllio - this would have been just a regular javascript injection into the CEF (Chrome Embedded Framework) - basically executing javascript after the page has been loaded. The implementation still works with the recent implementation of obs-browser, so if you check out the fork and compile the module yourself it works just as intended, if you need any changes on that you can let me know. I just cannot provide binaries (dlls or so) for this.

Yeah I screwed up merging the recent master into my branch, hence the error. I talked to the guys over at Discord and there is a very good reason why this is still halted, I do get their basic intention behind not merging it in, tho it takes a *very* long time and there still isn't a timeline announced regarding the change they are holding this back. Just to address @willlllllio - this would have been just a regular javascript injection into the CEF (Chrome Embedded Framework) - basically executing javascript after the page has been loaded. The implementation still works with the recent implementation of obs-browser, so if you check out the fork and compile the module yourself it works just as intended, if you need any changes on that you can let me know. I just cannot provide binaries (dlls or so) for this.

willlllllio commented 2021-05-14 11:42:25 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

@kuhnchris yeah I did compile it locally and tested it, the problem I had was that the JS gets executed too late to do some things which is what I was referring to.

loading order seems to go:

1. CefLoadHandler::OnLoadStart
2. page loading and main page JS executing already
3. CefLoadHandler::OnLoadEnd

So when injecting the JS in OnLoadEnd the page's own JS will already have ran before the injected code at that point, which makes doing some things with the injected code impossible. For example replacing the browser Audio API with a modified version won't work if the page's code already got a reference to the real Audio object in its initial loading phase.

Injecting the JS earlier in CefLoadHandler::OnLoadStart solved that for me, as it guarantees the injected code runs before any actual page code. Seems overall cleaner and anyone that wants to run their injected JS only after load finished can still easily do that via the documented loaded event just via normal JS.

https://magpcss.org/ceforum/apidocs3/projects/(default)/CefLoadHandler.html

@kuhnchris yeah I did compile it locally and tested it, the problem I had was that the JS gets executed too late to do some things which is what I was referring to. loading order seems to go: 1. `CefLoadHandler::OnLoadStart` 2. page loading and main page JS executing already 3. `CefLoadHandler::OnLoadEnd` So when injecting the JS in `OnLoadEnd` the page's own JS will already have ran before the injected code at that point, which makes doing some things with the injected code impossible. For example replacing the browser Audio API with a modified version won't work if the page's code already got a reference to the real Audio object in its initial loading phase. Injecting the JS earlier in `CefLoadHandler::OnLoadStart` solved that for me, as it guarantees the injected code runs before any actual page code. Seems overall cleaner and anyone that wants to run their injected JS only after load finished can still easily do that via the documented loaded event just via normal JS. https://magpcss.org/ceforum/apidocs3/projects/(default)/CefLoadHandler.html

kuhnchris commented 2021-05-14 12:11:44 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

technically the current implementation is ran "OnLoadEnd".
github.com/kuhnchris/obs-browser@53cfefe74a/browser-client.cpp (L442)
We could add the other 2 CEF Load Handlers too, shouldn't be that big of an issue - question rather is: should those be separate javascript "boxes" or should you just have 1 box and a radio button group with 2 buttons selecting between Injecting at "Start" or "End"?

*technically* the current implementation is ran "OnLoadEnd". https://github.com/kuhnchris/obs-browser/blob/53cfefe74a2347e9054212bb4c014766e53ee5f4/browser-client.cpp#L442 We could add the other 2 CEF Load Handlers too, shouldn't be that big of an issue - question rather is: should those be separate javascript "boxes" or should you just have 1 box and a radio button group with 2 buttons selecting between Injecting at "Start" or "End"?

willlllllio commented 2021-05-14 12:23:39 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

I don't think there's any need for any special UI for that, always injecting the JS in OnLoadStart should be enough. Anyone that wants to only run their code once loaded can just wrap it in window.addEventListener('load', func) to get the same behavior as OnLoadEnd just via normal JS .

I don't think there's any need for any special UI for that, always injecting the JS in `OnLoadStart` should be enough. Anyone that wants to only run their code once loaded can just wrap it in `window.addEventListener('load', func)` to get the same behavior as `OnLoadEnd` just via normal JS .

👍 4

reversefold commented 2021-09-07 12:39:08 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Was this merged or not? I'm not seeing the Javascript injection option in browser source properties when building from master.

Was this merged or not? I'm not seeing the Javascript injection option in browser source properties when building from master.

RytoEX commented 2021-09-07 12:52:03 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Was this merged or not? I'm not seeing the Javascript injection option in browser source properties when building from master.

No. GitHub's UI showing that the PR was merged is a bug. See:
https://github.com/obsproject/obs-browser/pull/137#issuecomment-841185024

> > > Was this merged or not? I'm not seeing the Javascript injection option in browser source properties when building from master. No. GitHub's UI showing that the PR was merged is a bug. See: https://github.com/obsproject/obs-browser/pull/137#issuecomment-841185024

kristoff-it commented 2021-11-17 09:34:28 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Could we now please merge this PR for real, if only just to spite Streamlabs?

I also wouldn't mind it for running my own overlays, but eroding the feature disparity with Streamlabs (which implements this already) would be the cherry on top.

Could we now please merge this PR for real, if only just to spite Streamlabs? I also wouldn't mind it for running my own overlays, but eroding the feature disparity with Streamlabs (which implements this already) would be the cherry on top.

kuhnchris commented 2021-11-17 09:43:59 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

This depends on the timeline of obs-browser. I talked to the guys in Discord - basically I am waiting for an "OK" or "go" from their side to re-implement this change. No idea how far the timeline fell apart with all the virus thing going on. 🤷

This depends on the timeline of obs-browser. I talked to the guys in Discord - basically I am waiting for an "OK" or "go" from their side to re-implement this change. No idea how far the timeline fell apart with all the virus thing going on. :shrug:

🚀 4

Fenrirthviti commented 2021-11-17 11:35:18 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Most of the discussion regarding our upgrade plan and progress can be found here: https://github.com/obsproject/obs-studio/discussions/3853

This is the most pressing task for Browser Source stuff, after which we'll be revisiting proposed changes and other updates.

Most of the discussion regarding our upgrade plan and progress can be found here: https://github.com/obsproject/obs-studio/discussions/3853 This is the most pressing task for Browser Source stuff, after which we'll be revisiting proposed changes and other updates.

lacksfish commented 2021-11-22 04:48:35 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

@kuhnchris I'd also be very interested in this feature. I need it to edit names in the Streamkit Discord overlay.

@kuhnchris I'd also be very interested in this feature. I need it to edit names in the Streamkit Discord overlay.

rogueyoshi commented 2021-11-22 09:49:55 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

@lacksfish ghetto css example to replace names. use the person's avatar image as a selector since user-id doesn't seem to be exposed in the frontend. you can get more accurate styles if you copy them from streamkit, i was just lazy.

.avatar ~ .user .name {
    position: absolute;
    visibility: collapse;
}

.avatar ~ .user .name:before {
    position: relative;
    top: -4px;
    left: -6px;
    visibility: visible;
    display: inline;
    background: rgba(30, 33, 36, 0.95);
    padding: 0.2rem 0.4rem;
    border-radius: 0.20rem;
}

.avatar[src="https://cdn.discordapp.com/avatars/157643090700664833/5d733b3f84ae07796fd2c356bbef6e70.jpg"] ~ .user .name:before {
    content: "changeme";
}

@lacksfish ghetto css example to replace names. use the person's avatar image as a selector since user-id doesn't seem to be exposed in the frontend. you can get more accurate styles if you copy them from streamkit, i was just lazy. ``` .avatar ~ .user .name { position: absolute; visibility: collapse; } .avatar ~ .user .name:before { position: relative; top: -4px; left: -6px; visibility: visible; display: inline; background: rgba(30, 33, 36, 0.95); padding: 0.2rem 0.4rem; border-radius: 0.20rem; } .avatar[src="https://cdn.discordapp.com/avatars/157643090700664833/5d733b3f84ae07796fd2c356bbef6e70.jpg"] ~ .user .name:before { content: "changeme"; } ``` 

👀 1

GStefanowich commented 2021-11-22 14:52:42 -06:00 (Migrated from github.com)

• Copy link
• Copy Markdown

since user-id doesn't seem to be exposed in the frontend

User IDs are exposed in the front-end, just sort of convoluted. They're visible in the data-reactid tag, mine is "180106401924775936"

data-reactid=".0.0.0.$180106401924775936/=1$180106401924775936"

Which can be targeted in CSS using the *= selector

.voice-state[data-reactid*="180106401924775936"] {
	some-css-rules: here;
}

> since user-id doesn't seem to be exposed in the frontend User IDs are exposed in the front-end, just sort of convoluted. They're visible in the `data-reactid` tag, mine is "180106401924775936" > data-reactid=".0.0.0.$**180106401924775936**/=1$**180106401924775936**" Which can be targeted in CSS using the `*=` selector ```css .voice-state[data-reactid*="180106401924775936"] { some-css-rules: here; } ```

👍 1

platinumazure commented 2023-10-29 20:53:08 -05:00 (Migrated from github.com)

• Copy link
• Copy Markdown

Is there a successor to this PR? I don't see an obvious link from the comment history but I wanted to double check. Thanks!

(I understand the "merged" status is a glitch in this case.)

Edit: I missed #354, apologies.

Is there a successor to this PR? I don't see an obvious link from the comment history but I wanted to double check. Thanks! (I understand the "merged" status is a glitch in this case.) Edit: I missed #354, apologies.

archiver-bot referenced this pull request from a commit 2026-09-23 09:56:20 -05:00

obs-browser: merge upstream changes (#137)

Sign in to join this conversation.

Reviewers

No reviewers

RytoEX

Labels

Clear labels   

area/accessibility

Improves accessibility of the program

  

area/dependency

Issue is in external library or dependency, not OBS itself

  

Confirmed

  

Crash Report

  

Delayed

Merging has been delayed due to an issue

  

Design Issue

There is an issue with incorporating this pull request's design. It may take some time to finalize.

  

Duplicate

  

Feedback required

Issue does not contain enough information or is missing the template

  

Good first issue

New to contributing to OBS? Start here!

  

kind/bug

Categorizes issue or PR as related to a bug.

  

kind/cleanup

Non-breaking change which makes code smaller or more readable

  

kind/enhancement

Enhancements are not bugs or new features but can improve usability or performance.

  

platform/linux

  

platform/macos

  

platform/windows

  

priority/high

This is an important PR

  

Question

  

Request for Comments

More feedback & discussion is requested

  

Seeking Other Contributors

This PR needs help from any other developers who are willing to give a crack at this code

  

Seeking Testers

  

Won't Fix / By Design

No labels

area/accessibility

area/dependency

Confirmed

Crash Report

Delayed

Design Issue

Duplicate

Feedback required

Good first issue

kind/bug

kind/cleanup

kind/enhancement

platform/linux

platform/macos

platform/windows

priority/high

Question

Request for Comments

Seeking Other Contributors

Seeking Testers

Won't Fix / By Design

Milestone

Clear milestone

No items

No milestone

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

gh-obsproject/obs-browser!137

No description provided.