AJAX form submission is not encoding new lines in textarea values as CRLF pairs, but only as LF - resulting into a different values than browser submission

ESLint complains about this, see https://dispatcher.drupalci.org/job/drupal_patches/3937/checkstyleResult/ :

The body of a for-in should be wrapped in an if statement to filter unwanted properties from the prototype. (guard-for-in)

Can't say anything about the patch itself, so leaving at needs review for now, but a re-roll should fix the above.

should we have an request subscriber or something like this processing the form values early in the boot process and ensuring new lines are presented as CRLF pairs?

So we already have the abstraction layer between $form_state->getUserInput() and $form_state->getValues() so since problem seems to be limited to form submissions whether we could solve it in the Form API itself. If this is in fact limited to textareas, maybe we could even implement this logic in \Drupal\Core\Render\Element\Textarea::valueCallback(). Then in $form_state->getUserInput() we would still have the potentially inconsistent value but anything going through $form_state->getValues() would have a properly normalized value. And it would still be pretty self-contained.

Nice! To me this looks like a pretty nice fix, but this should be looked at by a Form API maintainer. Tagging accordingly. Although since this is in the ajax system component, I guess they might not see it?

One minor note on the patch, looks great otherwise:

+++ b/core/lib/Drupal/Core/Render/Element/Textarea.php
@@ -57,7 +57,10 @@ public static function valueCallback(&$element, $input, FormStateInterface $form
+      // Ensure that new lines are always formatted as CRLF pairs.
+      $processed_input = preg_replace('@\r?\n@', "\r\n", $input) ?: $input;

Maybe we could document why we do this?

Wow, that comment is awesome, thank you!!!

Looked at this again after a while. The fix is really nice and self-contained and comes with test coverage, so I think it's good to go from my point of view. Subsystem maintainer review wouldn't hurt, but since this doesn't actually touch FormBuilder or anything of the sort I think the risk here is very limited.

Unrelated failure

+++ b/core/lib/Drupal/Core/Render/Element/Textarea.php
@@ -57,7 +57,18 @@ public static function valueCallback(&$element, $input, FormStateInterface $form
+      $processed_input = preg_replace('@\r?\n@', "\r\n", $input) ?: $input;

I asked @hchonov about why we do the ?: $input fallback.

tl;dr: It makes sense, thus, leaving at RTBC.

Details:
He pointed me to the PHP documentation which says that NULL will be returned if an error occurred.

So I thought that in case of an error, it would probably be good to fail explicitly (which in this case would be returning NULL as the form value) instead of just using the unprocessed string so that one becomes aware of the error and can fix it.

@hchonov then pointed me to https://bugs.php.net/bug.php?id=70110 (see #2792877: Symfony YAML parser fails on some strings when running PHP 7 for more information), which could result in such an error leading preg_replace() to return NULL. In this edge case, where PCRE JIT compilation is on, and someone posts a string which reveals this bug into a textarea, it is presumably better to simply submit the text without normalized newlines into the database instead of failing.This is exactly what the fallback achieves. If someone does want the newlines to be normalized they can turn PCRE JIT compilation off.

This got fixed upstream https://github.com/jquery-form/form/pull/516 - perhaps we should just pull that it and not have to do this on all text submitted in textareas.

Plus the text in #29 that explains:

+++ b/core/lib/Drupal/Core/Render/Element/Textarea.php
@@ -57,7 +57,18 @@ public static function valueCallback(&$element, $input, FormStateInterface $form
+      $processed_input = preg_replace('@\r?\n@', "\r\n", $input) ?: $input;

the ternary should also be in the code - if we are going to do this.

Yeah, I agree with #30 that that is the better fix. Talked to @hchonov (a while back) and he agreed, as well.
I do think it's worth to keep this open for reference, though, until we have upgraded jquery form. So marking postponed on #2899141: jQuery Form Plugin update to latest stable release. Once that is in, this can go straight to "closed (duplicate)".

I know its postponed, but we still need and use it, therefore rerolled #23

Just in case some one runs in here ;)

Per #32