Add Textarea option to normalize newlines to \n

Created on 9 March 2021, over 3 years ago
Updated 10 September 2024, about 1 month ago

Problem/Motivation

In ✨ Export configuration YAML strings as multiline Fixed , we earned neat readable exports for multiline config items IFF they do not contain "\r".
Many config contain \r though, simply for lazynesss, as the HTML standard mandates textarea elements to return "\r\n" line breaks.

Proposed resolution

Add a "#normalize_newlines" option to \Drupal\Core\Render\Element\Textarea.
So beautifying config is a oneliner for module developers.

Remaining tasks

Do it.

User interface changes

None.

API changes

Add a "#normalize_newlines" option to \Drupal\Core\Render\Element\Textarea.

Data model changes

None.

Release notes snippet

The Textarea element has a now "#normalize_newlines" option.
Config before... config after...
Example form element...

πŸ“Œ Task
Status

RTBC

Version

11.0 πŸ”₯

Component
RenderΒ  β†’

Last updated 6 days ago

Created by

πŸ‡©πŸ‡ͺGermany geek-merlin Freiburg, Germany

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡ΊπŸ‡¦Ukraine abramm Lutsk

    The patch in #34 doesn't apply to 10.3, so re-rolled it against both 10.3.x (for those of us who need this patch to be applied to their sites) and 11.x branches.
    Also, removed the unrelated change and split patches to test and test+fix.

  • πŸ‡ΊπŸ‡¦Ukraine abramm Lutsk

    abramm β†’ changed the visibility of the branch 11.x to hidden.

  • πŸ‡ΊπŸ‡¦Ukraine abramm Lutsk

    abramm β†’ changed the visibility of the branch 9.2.x to hidden.

  • πŸ‡ΊπŸ‡¦Ukraine abramm Lutsk

    Added two MRs (test only and test + code change), let's see if tests still passes.

  • πŸ‡ΊπŸ‡¦Ukraine abramm Lutsk

    abramm β†’ changed the visibility of the branch 3202631-add-textarea-option to hidden.

  • Pipeline finished with Success
    4 months ago
    Total: 613s
    #211601
  • Pipeline finished with Failed
    4 months ago
    Total: 689s
    #211599
  • Status changed to RTBC 4 months ago
  • πŸ‡«πŸ‡·France andypost

    Thanks, looks good to me

  • Status changed to Needs work 3 months ago
  • πŸ‡¬πŸ‡§United Kingdom catch

    This seems good but still needs a change record.

  • First commit to issue fork.
  • Pipeline finished with Canceled
    about 1 month ago
    Total: 162s
    #273769
  • πŸ‡ΊπŸ‡¦Ukraine Taran2L Lviv

    taran2l β†’ changed the visibility of the branch 3202631-11.x-test-only to hidden.

  • Status changed to Needs review about 1 month ago
  • πŸ‡ΊπŸ‡¦Ukraine Taran2L Lviv

    Small code update + added a change record. Please review

  • Pipeline finished with Failed
    about 1 month ago
    Total: 1577s
    #273776
  • Pipeline finished with Canceled
    about 1 month ago
    Total: 160s
    #273816
  • Pipeline finished with Failed
    about 1 month ago
    Total: 585s
    #273817
  • Status changed to Needs work about 1 month ago
  • It looks like the test is failing.

  • Pipeline finished with Canceled
    about 1 month ago
    Total: 66s
    #273876
  • Pipeline finished with Failed
    about 1 month ago
    Total: 158s
    #273878
  • Pipeline finished with Failed
    about 1 month ago
    Total: 136s
    #273880
  • Pipeline finished with Running
    about 1 month ago
    #273888
  • Pipeline finished with Failed
    about 1 month ago
    Total: 2841s
    #273907
  • πŸ‡«πŸ‡·France andypost

    One test still fails

    ---- Drupal\Tests\Core\Render\Element\TextareaTest ----
    
    
    Status    Group      Filename          Line Function                            
    --------------------------------------------------------------------------------
    [31mFail      Other      phpunit-367.xml      0 Drupal\Tests\Core\Render\Element\Te
    [0m    PHPUnit Test failed to complete; Error: PHPUnit 10.5.30 by Sebastian
        Bergmann and contributors.
        
        Runtime:       PHP 8.3.11
        Configuration: /builds/issue/drupal-3202631/core/phpunit.xml.dist
        
        ..WWW..                                                             7 / 7
        (100%)
        
        Time: 00:00.038, Memory: 8.00 MB
        
        3 tests triggered 1 PHP warning:
        
        1)
        /builds/issue/drupal-3202631/core/lib/Drupal/Core/Render/Element/Textarea.php:66
        Undefined array key "#normalize_newlines"
        
        Triggered by:
        
        * Drupal\Tests\Core\Render\Element\TextareaTest::testValueCallback#2
         
        /builds/issue/drupal-3202631/core/tests/Drupal/Tests/Core/Render/Element/TextareaTest.php:22
        
        * Drupal\Tests\Core\Render\Element\TextareaTest::testValueCallback#3
         
        /builds/issue/drupal-3202631/core/tests/Drupal/Tests/Core/Render/Element/TextareaTest.php:22
        
        * Drupal\Tests\Core\Render\Element\TextareaTest::testValueCallback#4
         
        /builds/issue/drupal-3202631/core/tests/Drupal/Tests/Core/Render/Element/TextareaTest.php:22
        
        OK, but there were issues!
        Tests: 7, Assertions: 7, Warnings: 1.
  • The test was working earlier. It should be

      public function testNormalizeNewlines() {
        $form_state = $this->prophesize(FormStateInterface::class)->reveal();
        $element = ['#normalize_newlines' => TRUE];
        $input = "some\r\ndifferent\rline\nendings";
        $expected = "some\ndifferent\nline\nendings";
        $this->assertSame($expected, Textarea::valueCallback($element, $input, $form_state));
      }
    
  • Pipeline finished with Canceled
    about 1 month ago
    Total: 66s
    #274570
  • Pipeline finished with Success
    about 1 month ago
    Total: 5594s
    #274572
  • Status changed to Needs review about 1 month ago
  • πŸ‡ΊπŸ‡¦Ukraine Taran2L Lviv

    It's green now.

    I've refactored test a bit to be more robust and support more cases.

  • Status changed to RTBC about 1 month ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave
    1) Drupal\Tests\Core\Render\Element\TextareaTest::testValueCallback with data set #5 ('some\ndifferent\nline\nendings', 'some\r\ndifferent\rline\nendings')
    Failed asserting that two strings are identical.
    --- Expected
    +++ Actual
    @@ @@
     #Warning: Strings contain different line endings!
    -'some
    -different
    -line
    +'some
    line
     endings'
    

    Ran test-only feature to see coverage
    Summary appears complete
    Code change itself seems pretty straight forward

    LGTM!

  • First commit to issue fork.
  • πŸ‡³πŸ‡ΏNew Zealand quietone

    There are no unanswered questions here and is does look straight forward as said in the previous comment.

    Unfortunately, one of the comments was not wrapped correctly. I updated that one and made a change to another.

    I then reviewed the test and found that most of the defaults for the element did not need to be set. That lead me to rework the test and the data provider. The data provider now sets the element configuration as needed and the structure is changed as well.

    This now needs another review.

  • Pipeline finished with Success
    15 days ago
    Total: 594s
    #299455
  • πŸ‡³πŸ‡ΏNew Zealand quietone

    Coming back to check the change record. I see no evidence here that anyone has reviewed the change record. I took a brief look and was overwhelmed by the second paragraph with a long sentence that is hard to parse. Remember, the readers of change records will have varying levels of skills with the English language.

    Setting to needs work for a review of the change record.

  • The comment changes look good to me.

Production build 0.71.5 2024