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

[NEW] add a modal dialog to warn the user about sending an email without a body #868

Merged
merged 1 commit into from
Jan 30, 2024

Conversation

jacob-js
Copy link
Member

@jacob-js jacob-js commented Jan 15, 2024

Description

Add a warning prompt that's shown when the user is about to send an email with an empty body.

Issues

@PaulTGG
Copy link
Contributor

PaulTGG commented Jan 16, 2024

I found that when the setting is disabled, I was shown a warning, and prevented from sending an email with an empty body. However, when the setting is enabled, trying to send an email with an empty body still crashes php. Here's the last error message.

[Tue Jan 16 10:59:32.513723 2024] [proxy_fcgi:error] [pid 57369] [client 172.16.32.226:60912] AH01071: Got error 't()\n#3 /usr/local/share/testcypht/modules/smtp/hm-mime-message.php(92): Hm_MIME_Msg->prep_message_body()\n#4 /usr/local/share/testcypht/modules/smtp/modules.php(736): Hm_MIME_Msg->get_mime_msg()\n#5 /usr/local/share/testcypht/lib/modules_exec.php(177): Hm_Handler_process_compose_form_submit->process()\n#6 /usr/local/share/testcypht/lib/modules_exec.php(155): Hm_Module_Exec->run_handler_module()\n#7 /usr/local/share/testcypht/lib/dispatch.php(221): Hm_Module_Exec->run_handler_modules()\n#8 /usr/local/share/testcypht/lib/dispatch.php(1...'

@marclaporte
Copy link
Member

I don't want one more setting. Just make it that there is warning to the user.

@PaulTGG
Copy link
Contributor

PaulTGG commented Jan 17, 2024 via email

@marclaporte
Copy link
Member

Options are more code to maintain. It's one more thing to think about when testing or reporting issues.

I can't think of a real-world use case where someone would regularly and intentionally send emails without a subject or body.

@PaulTGG
Copy link
Contributor

PaulTGG commented Jan 17, 2024

I can't think of a real-world use case where someone would regularly and intentionally send emails without a subject or body.

You clearly don't work with some of my co-workers... lol Lots of subject-only emails flying around!

Could a workaround be to pre-populate the email composer with the signature line? Then as long as the profile has a signature, there would always be something in the email body.

@marclaporte
Copy link
Member

Idea: The warning could be:

  • Cancel sending (default)
  • Send anyway
  • Send anyway and don't warn me in the future

The third option would add a cookie to the user's browser.

@jacob-js
Copy link
Member Author

I found that when the setting is disabled, I was shown a warning, and prevented from sending an email with an empty body. However, when the setting is enabled, trying to send an email with an empty body still crashes php. Here's the last error message.

[Tue Jan 16 10:59:32.513723 2024] [proxy_fcgi:error] [pid 57369] [client 172.16.32.226:60912] AH01071: Got error 't()\n#3 /usr/local/share/testcypht/modules/smtp/hm-mime-message.php(92): Hm_MIME_Msg->prep_message_body()\n#4 /usr/local/share/testcypht/modules/smtp/modules.php(736): Hm_MIME_Msg->get_mime_msg()\n#5 /usr/local/share/testcypht/lib/modules_exec.php(177): Hm_Handler_process_compose_form_submit->process()\n#6 /usr/local/share/testcypht/lib/modules_exec.php(155): Hm_Module_Exec->run_handler_module()\n#7 /usr/local/share/testcypht/lib/dispatch.php(221): Hm_Module_Exec->run_handler_modules()\n#8 /usr/local/share/testcypht/lib/dispatch.php(1...'

@PaulTGG which PHP version is your environment using? I did not get any such error when trying this on my side. Could it be related to your PHP version?

@jacob-js
Copy link
Member Author

Idea: The warning could be:

  • Cancel sending (default)
  • Send anyway
  • Send anyway and don't warn me in the future

The third option would add a cookie to the user's browser.

Good.

@PaulTGG
Copy link
Contributor

PaulTGG commented Jan 17, 2024

@PaulTGG which PHP version is your environment using? I did not get any such error when trying this on my side. Could it be related to your PHP version?

@jacob-js I'm using PHP 8.2.7. It's the latest version that's available in the Debian repositories. (Using Debian 12.4.)

@jacob-js
Copy link
Member Author

@PaulTGG which PHP version is your environment using? I did not get any such error when trying this on my side. Could it be related to your PHP version?

@jacob-js I'm using PHP 8.2.7. It's the latest version that's available in the Debian repositories. (Using Debian 12.4.)

Can you please try switching to version 7.4?

@PaulTGG
Copy link
Contributor

PaulTGG commented Jan 18, 2024

Can you please try switching to version 7.4?

I wasn't able to complete the install using PHP 7.4. It's not in the repository of Debian 12, and I tried Debian 11, but composer gave the following error:

require.composer is invalid, it should have a vendor name, a forward slash, and a package name. The vendor and package name can be words separated by -, . or _. The complete name should match "^[a-z0-9]([_.-]?[a-z0-9]+)*/[a-z0-9](([_.]?|-{0,2})[a-z0-9]+)*$".

Hopefully the PHP version isn't the issue, since 7.4 was end-of-life a little over a year ago!

modules/smtp/site.js Outdated Show resolved Hide resolved
modules/core/setup.php Outdated Show resolved Hide resolved
modules/core/site.css Outdated Show resolved Hide resolved
@jacob-js jacob-js changed the title [NEW] add "allow empty body" setting to enable/disable sending emails with empty body [NEW] add a modal dialog to warn the user about sending an email without a body Jan 19, 2024
@jacob-js
Copy link
Member Author

Can you please try switching to version 7.4?

I wasn't able to complete the install using PHP 7.4. It's not in the repository of Debian 12, and I tried Debian 11, but composer gave the following error:

require.composer is invalid, it should have a vendor name, a forward slash, and a package name. The vendor and package name can be words separated by -, . or _. The complete name should match "^[a-z0-9]([_.-]?[a-z0-9]+)*/[a-z0-9](([_.]?|-{0,2})[a-z0-9]+)*$".

Hopefully the PHP version isn't the issue, since 7.4 was end-of-life a little over a year ago!

@PaulTGG By now, I'm certain you will not get that exception anymore using the PHP version you have.

@PaulTGG
Copy link
Contributor

PaulTGG commented Jan 22, 2024

@PaulTGG By now, I'm certain you will not get that exception anymore using the PHP version you have.

Just tried it, and everything looks good, thanks!

modules/core/site.css Outdated Show resolved Hide resolved
@kroky kroky merged commit 6c83e4a into cypht-org:master Jan 30, 2024
Shadow243 added a commit to Shadow243/cypht that referenced this pull request Jan 31, 2024
Shadow243 added a commit to Shadow243/cypht that referenced this pull request Feb 2, 2024
Shadow243 added a commit to Shadow243/cypht that referenced this pull request Feb 2, 2024
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.

None yet

5 participants