Contrib.social

Legal document titles show double-escaped HTML

Open on Drupal.org →
Created on 29 June 2020, almost 5 years ago
Updated 23 January 2024, about 1 year ago

Problem/Motivation

The title of an entity_legal document is double-escaped when viewing the document. This is most noticeable when it contains an apostrophe (' which is escaped as the HTML entity ')

Steps to reproduce

  1. Download Drupal core version 8.9.0, Token version 8.x-1.7, and clone entity_legal version 3.0.x (at time-of-writing, I was at commit fa0a50d)
  2. Install Drupal core with the Minimal (or Standard) install profile
  3. Go to /admin/modules and enable entity_legal
  4. Go to /admin/structure/legal/add and enter the following information:
    • Administrative label = Guide de l'utilisateur
    • (leave all other fields at their default values)
    • Click Save
    • Title = Guide de l'utilisateur
    • Document text = Lorem ipsum
    • Acceptance label= I agree to the <a href="[entity_legal_document:url]">[entity_legal_document:label]</a> document (note the initial value for this field is double-escaped, but that's not the primary reason for this ticket - I can work-around this)
    • (leave all other fields at their default values)
    • Click Save
    • (leave all other fields at their default values)
    • Click Save
  5. Go to /admin/structure/legal
  6. Click the title Guide de l'utilisateur (i.e.: to view the new legal document)
  7. Observe the title of the page:
    • Expected behavior: the title is Guide de l'utilisateur
    • Actual behavior: the title is Guide de l&#039;utilisateur

As far as I can tell, the title is double-escaped due to how the "Title pattern" setting on an entity_legal document is resolved. By default, it is set to [entity_legal_document:label] - in this case, the token subsystem renders early → when it escapes the HTML when \Drupal\Core\Utility\Token::replace() is called in \Drupal\entity_legal\Controller\EntityLegalController::documentPageTitle().

This (escaped) string gets passed through \Drupal\Core\Controller\TitleResolver::getTitle(), \Drupal\Core\Render\MainContent\HtmlRenderer::prepare(), HtmlRenderer::prepare(), and \Drupal\Core\Render\Renderer::doRender() to Twig; and since its still a string at that point, Twig considers it to be user-entered text and auto-escapes → it a second time.

Proposed resolution

As far as I can tell, there are several ways we could resolve this, but the resolution recommended in the docs for the Token::replace() method we are using is to have EntityLegalController::documentPageTitle() return a #markup render array instead of a plain string.

Remaining tasks

  2. Review and feedback
  3. RTBC and feedback
  4. Commit
  5. Release

User interface changes

(none)

API changes

(none)

Data model changes

(none)

Release notes snippet

To be determined

🐛 Bug report
Status

Needs work

Version

3.0

Component

Code

Created by

🇨🇦Canada mparker17 UTC-4

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.

  • First commit to issue fork.
  • Merge request !22Reroll patch for entity_legal 4.0.x → (Open) created by ultrabob
  • Pipeline finished with Failed
    about 1 year ago
    #81090
  • Comment about 1 year ago →
    🇯🇵Japan ultrabob Japan

    I confirmed that this issue still exists in version 4.0.x of the module, confirmed that the fix submitted previously in a patch still works, and created a merge request with the changes applied to the new version. I'm attaching the patch equivalent of that MR.

  • Pipeline finished with Failed
    about 1 year ago
    #81091
  • Pipeline finished with Failed
    about 1 year ago
    #81098
  • Pipeline finished with Failed
    about 1 year ago
    #81100
  • Pipeline finished with Failed
    about 1 year ago
    #81101
  • Pipeline finished with Running
    about 1 year ago
    #81103
  • Pipeline finished with Running
    about 1 year ago
    #81122
  • Pipeline finished with Failed
    about 1 year ago
    #81126
  • Pipeline finished with Failed
    about 1 year ago
    #81127
  • Pipeline finished with Failed
    about 1 year ago
    #81129
  • Pipeline finished with Failed
    about 1 year ago
    #81135
  • Pipeline finished with Failed
    about 1 year ago
    #81138
  • Pipeline finished with Failed
    about 1 year ago
    #81140
  • Pipeline finished with Failed
    about 1 year ago
    #81144
  • Pipeline finished with Failed
    about 1 year ago
    #81145
  • Pipeline finished with Failed
    about 1 year ago
    #81150
  • Pipeline finished with Failed
    about 1 year ago
    #81155
  • Pipeline finished with Running
    about 1 year ago
    #81159
  • Pipeline finished with Running
    about 1 year ago
    #81508
  • Pipeline finished with Failed
    about 1 year ago
    #81517
  • Pipeline finished with Failed
    about 1 year ago
    #81526
  • Pipeline finished with Failed
    about 1 year ago
    #81544
  • Comment about 1 year ago →
    🇯🇵Japan ultrabob Japan

    I'm sorry for the spam while trying to fix the test. All the coding standards issues are fixed and I confirm that the patch fixes the issue we faced, but I don't have any more time to try and fix the test. Tests are not my strong suit. If someone else can make that work it will be great. I reverted to the original test code with coding standards fixes applied. I'm also attaching the new patch.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024