NodeTypeForm::validateForm doesn't validate HTML for description

Created on 11 June 2018, almost 7 years ago
Updated 16 February 2023, about 2 years ago

Problem/Motivation

When someone enters invalid HTML (e.g. opening a "<em>" without a closing </em> tag) as the content type's description, Javascript errors will occur on the node/add page.

Either we need to filter the output on "/node/add" or we need to validate the input on "admin/structure/types/manage/nodetype".

Proposed resolution

Since the labels can be translated and those can also contain wrong HTML it may be better to do both.

Remaining tasks

validate input and filter output.

User interface changes

none

API changes

none

Data model changes

none

🐛 Bug report
Status

RTBC

Version

10.1

Component
Node system 

Last updated about 18 hours ago

No maintainer
Created by

🇧🇪Belgium mpp

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.

  • 🇨🇳China jungle Chongqing, China

    The patch in #16 is still valid against 10.1.x

    2978818-16-tests-only.patch reproduced it, and 2978818-16.patch fixed it.

    Tagging "Needs Review Queue Initiative" because I saw the NR request in #needs-review-queue-initiative on slack.

  • Status changed to Needs work about 2 years ago
  • 🇫🇮Finland lauriii Finland

    It looks like the proposed resolution doesn't actually match what is in the issue summary. Proposed solution seems to be to do it on runtime to not run into the same problem with translations. The downside of that solution is that it could interfere with existing markup but overall doing it runtime seems more in line with how we do markup normalization in other places because this way we can solve it for all different use cases.

  • 🇦🇺Australia acbramley

    Still valid, I tested #20 i.e doing Html::normalize at runtime in template_preprocess_node_add_list but it didn't work, I have no idea why though, debugging into the preprocess showed valid markup in the description variables. (EDIT: claro_preprocess_node_add_list also needed the fix)

    Also, these descriptions are shown elsewhere such as on admin/structure/types so I don't think it makes sense to do it at runtime.

  • Pipeline finished with Success
    12 days ago
    Total: 1000s
    #455638
  • 🇺🇸United States smustgrave

    Seems like a good update

    1) Drupal\Tests\node\Functional\NodeTypeTest::testNodeTypeEditing
    Behat\Mink\Exception\ExpectationException: The string "<em>Lorem ipsum.</em>" was not found anywhere in the HTML response of the current page.
    /builds/issue/drupal-2978818/vendor/behat/mink/src/WebAssert.php:888
    /builds/issue/drupal-2978818/vendor/behat/mink/src/WebAssert.php:363
    /builds/issue/drupal-2978818/core/tests/Drupal/Tests/WebAssert.php:559
    /builds/issue/drupal-2978818/core/modules/node/tests/src/Functional/NodeTypeTest.php:160
    FAILURES!
    Tests: 6, Assertions: 95, Failures: 1.
    Exiting with EXIT_CODE=1
    

    Shows the test coverage

    Great extension of an existing test.

    LGTM.

  • 🇫🇷France nod_ Lille

    I would prefer not changing the user input since that wouldn't solve the use case of a bad translation. Good news is that the tests shouldn't need to be changed, we only need to change how it's solved :)

    A problem in the translation can create issues like 🐛 CKEditor 5 toolbar configuration not show buttons in spanish installation Active .

    Also the issue summary and the solution are not the same.

Production build 0.71.5 2024