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

Issue #33 - Apprise notification line break #167

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Emmo213
Copy link

@Emmo213 Emmo213 commented Aug 7, 2024

Adding a line break for email notifications never worked for me. After some troubleshooting what I discovered was that when the settings form is submitted PHP will urlencode the parameters and when we access those parameters using _GET it does a urldecode, but for some reason certain characters aren't decoded properly. I don't know if it's a PHP bug or something else, but by base64 encoding the values when the form is submitted the setting values remain intact.

For email I've been using <br/> for a line break, but suspect others who are using /r/n or /n are having the same issue.

While personally I only needed this for the notification body I did it for the title too in case that's helpful for somebody else.

This change works with the actual notification but not with the current test notification. Not included in this pull request, it's always bothered me that sending a test notification and sending an actual notification used two different sets of code. That's why, as mentioned in issue #33, the test notification with a line break would succeed but the actual notification would fail, and why this fix works with the actual notification but not with the current test notification. I'm looking at combining those two code bases in a separate pull request.

Edit: the change to use the same code base for both regular and test notifications is PR #174 .

@Emmo213
Copy link
Author

Emmo213 commented Sep 8, 2024

It's been a month with no discussion. Should I close this PR as unwanted?

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.

1 participant