Change the mail_line_endings variable into config.

Files: 
CommentFileSizeAuthor
#16 1798920-mail_line_endings_cmi-16.patch3.71 KBjapicoder
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install. View
#12 1798920-mail_line_endings_cmi-12.patch3.96 KBjapicoder
PASSED: [[SimpleTest]]: [MySQL] 47,595 pass(es). View
#10 1798920-mail_line_endings_cmi-10.patch4 KBjapicoder
PASSED: [[SimpleTest]]: [MySQL] 47,565 pass(es). View
#7 1798920-maillineendings-drupal8-4.patch4.18 KBACF
PASSED: [[SimpleTest]]: [MySQL] 41,891 pass(es). View
#5 1798920-maillineendings-drupal8-3.patch4.18 KBACF
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install. View
#3 1798920-maillineendings-drupal8-2.patch4.18 KBACF
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install. View
#1 1798920-maillineendings-drupal8-1.patch4.18 KBACF
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install. View

Comments

ACF’s picture

Status: Active » Needs review
FileSize
4.18 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install. View

Patch to offer a fix.

Status: Needs review » Needs work

The last submitted patch, 1798920-maillineendings-drupal8-1.patch, failed testing.

ACF’s picture

Status: Needs work » Needs review
FileSize
4.18 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install. View

Fixed php mistake.

Status: Needs review » Needs work

The last submitted patch, 1798920-maillineendings-drupal8-2.patch, failed testing.

ACF’s picture

Status: Needs work » Needs review
FileSize
4.18 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install. View

Hopefully php fixed

Status: Needs review » Needs work

The last submitted patch, 1798920-maillineendings-drupal8-3.patch, failed testing.

ACF’s picture

Status: Needs work » Needs review
FileSize
4.18 KB
PASSED: [[SimpleTest]]: [MySQL] 41,891 pass(es). View

another silly php mistake.

heyrocker’s picture

Status: Needs review » Needs work
+++ b/core/includes/mail.incundefined
@@ -6,13 +6,6 @@
- * Auto-detect appropriate line endings for e-mails.
- *
- * $conf['mail_line_endings'] will override this setting.
- */
-define('MAIL_LINE_ENDINGS', isset($_SERVER['WINDIR']) || strpos($_SERVER['SERVER_SOFTWARE'], 'Win32') !== FALSE ? "\r\n" : "\n");

I'm curious why you got rid of this define. It seems like we are inserting this code into two other places now, and that doesn't make a lot of sense.

+++ b/core/modules/system/system.installundefined
@@ -1976,6 +1981,23 @@ function system_update_8021() {
+function system_update_8022() {
+  // Either check whether the variable exists or set it and save it as config.
+  // This is done this way as the variable mail_line_endings was a variable that
+  // was statically defined.
+  if (!($line_endings = update_variable_get('mail_line_endings'))) {
+    $line_endings = isset($_SERVER['WINDIR']) || strpos($_SERVER['SERVER_SOFTWARE'], 'Win32') !== FALSE ? "\r\n" : "\n";
+  }
+  config('system.mail')
+    ->set('line_endings', $line_endings)
+    ->save();

We should also variable_del('mail_line_endings') when we're done.

Additionally, $conf will still be able to override this value, it will just be done with $conf['system.mail.line_endings'] so any comment referring to $conf['mail_line_endings'] will have to be changed to reflect this.

japicoder’s picture

Assigned: Unassigned » japicoder

A month without changes and no response, I get this task to work on it.

japicoder’s picture

Status: Needs work » Needs review
FileSize
4 KB
PASSED: [[SimpleTest]]: [MySQL] 47,565 pass(es). View

Some changes since the last patch: I recovered the define, improved the update and corrected the docs referring to $conf['mail_line_endings'].

Berdir’s picture

Status: Needs review » Needs work

The code defines both a hardcoded default value and an update function, so it will always be set, for everyone. Which means that the automatic fallback is never used, which I think is wrong.

I think we should a) set the default to an empty string/NULL (not sure what exactly is possible with yaml) and b) only do the variable update if there is actually something defined and then use that value and not the constant.

japicoder’s picture

Status: Needs work » Needs review
FileSize
3.96 KB
PASSED: [[SimpleTest]]: [MySQL] 47,595 pass(es). View

a) With yaml it's possible to define both of them. I've used empty string finally.
b) Currently the variable only gets updated on a fresh install or when updating from 7.x to 8.x, checking if the variable has a value and using it or if it doesn't has a value then update with the constant.

Thank you for your suggestions :)

aspilicious’s picture

Status: Needs review » Needs work
+++ b/core/modules/system/config/system.mail.ymlundefined
@@ -0,0 +1 @@
+line_endings: ¶
\ No newline at end of file

Trailing whitespace and needs a newline

Berdir’s picture

Status: Needs work » Needs review

What happens if you start developing a site on a local windows installation and then move it to a linux server? Previously, it switched the line ending, now it will continue to use the windows one.

I'm not sure if that's really want we want. To keep the existing behavior, we have to leave it empty and check on every call to it.

japicoder’s picture

@Berdir then we have to check on the update if the variable is defined and if it has a value use it for config. If no value, don't assign one based on the define. And on the install don't assign a default value, to avoid a problem with different enviroments (development and production) as you suggest.

@aspilicious true, I thought that it wasn't a problem. Corrected on the next patch.

japicoder’s picture

FileSize
3.71 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install. View

Added changes of #15

ACF’s picture

Issue tags: -Configuration system

Status: Needs review » Needs work
Issue tags: +Configuration system

The last submitted patch, 1798920-mail_line_endings_cmi-16.patch, failed testing.

ACF’s picture

Needs re roll with increased system_update number.

ACF’s picture

Status: Needs work » Closed (duplicate)

This has ended up being a duplicate of #1821420: Convert mail variables to cmi, so closing it as a duplicate.