Tokens are not replaced on the preview page. Consider this example form:

_site_name_:
  '#type': fieldset
  '#title': '[site:name]'
  _site_name_2:
    '#type': checkbox
    '#title': '[site:name]'
markup:
  '#type': markup
  '#display_on': both
  '#markup': '[site:name]'

While tokens are replaced on the form, on the preview page, '[site:name]' is displayed.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Ice-D created an issue. See original summary.

jrockowitz’s picture

Priority: Normal » Major
FileSize
3.13 KB

@Ice-D Great catch. This is a major bug.

Attached is webform that replicates the issue.

jrockowitz’s picture

This issue impact emails and confirmations as well. The attached webform includes a preview, email, and confirmation.

jrockowitz’s picture

Status: Active » Needs review
FileSize
3.95 KB

Let's see if this patch breaks any tests.

Status: Needs review » Needs work

The last submitted patch, 4: 2939412-5.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jrockowitz’s picture

Status: Needs work » Needs review
FileSize
4.73 KB

Status: Needs review » Needs work

The last submitted patch, 6: 2939412-7.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jrockowitz’s picture

Status: Needs work » Needs review
FileSize
4.81 KB

  • jrockowitz committed 3a5fe96 on 8.x-5.x
    Issue #2939412 by jrockowitz, Ice-D: Tokens not replaced on preview page
    
jrockowitz’s picture

Status: Needs review » Fixed

I committed the patch. Please download the latest dev release to review.

  • jrockowitz committed fd1a52b on 8.x-5.x
    Revert "Issue #2939412 by jrockowitz, Ice-D: Tokens not replaced on...
jrockowitz’s picture

Status: Fixed » Needs work

This change is causing a regression where the admin theme can't be applied when viewing a webform submission where the webform has a token value in an element.

The problem is the token replacement in \Drupal\webform\Plugin\WebformElementBase::initialize is setting the default theme before the admin theme can be applied

I am going to revert this patch.

Steps to reproduce the issue...

  • Apply patch
  • View any Contact webform submission
bucefal91’s picture

I tried to investigate on this issue but I couldn't find the side effects you're speaking about. I have done:

  • on top of latest 8.x-5.x
  • applied the patch #8
  • cleared the drupal caches
  • confirmed the webform from main description of this ticket successfully replaces the tokens in preview
  • tried to catch the side effect, but couldn't - I submitted a contact webform, and was able to see it OK both ways at /admin/structure/webform/manage/contact/submission/5 in admin theme and at /webform/contact/submissions/5 in front end theme.

Did I miss some extra step?

jrockowitz’s picture

I can't reproduce the issue in my dev environment. The issue happened on client's website when you went to edit a submission, I am starting to suspect the client's website is the issue. The good news is I can test the patch on their development website.

jrockowitz’s picture

Assigned: Unassigned » jrockowitz

Assigning this to myself so that I test this patch on my client's site.

Ice-D’s picture

Just wanted to say that I'm using this patch too and have had no problems so far.

jrockowitz’s picture

Status: Needs work » Needs review
FileSize
5.4 KB
880 bytes

The issue is the token.module's field_tokens() function calls \Drupal::service('renderer')->renderPlain($field_output); for all field related tokens and this initializes the default site theme and sets is as the active theme. This is causing the webform submission view and edit pages to not use the admin theme and instead these pages are using the default site theme.

My specific issue is related to a token called [webform-authenticated-user:field_user_last_name] which is used as a #default_value for a textfield element.

The patch is triggering the problem because element property tokens are now being replaced when a webform is first initialized which happens very early on in a request/page's bootstrapping.

I know this is a lot to explain and I am not even sure how to write test coverage. For now, I am including an updated patch that the resets the active theme when it is being set when tokens are being replaced. The code in the patch (and interdiff) is only triggered by my above-described edge case.

jrockowitz’s picture

The attached patch use dependency injection. I hate that this is a 'trust me' patch but trust this minor tweak solve the edge case problem.

The steps to reproduce are...

  • Add a 'Name' text field (called field_name) to users (/admin/config/people/accounts/fields/add-field)
  • Populate the current user's 'Name' field (/user/1/edit)
  • Update the 'Contact' form and set [webform-authenticated-user:field_name] as the #default_value to the 'Name' element (/admin/structure/webform/manage/contact/element/name/edit)
  • Create a test submission and view a single result (/admin/structure/webform/manage/contact/results/submissions)
  • Confirm that the displayed theme is the default site theme and not the admin (seven) theme. (see below)

@bucefal91 Do you understand the general problem? If you can confirm the problem and the solution maybe we can commit the patch without any more tests. Personally, I am just looking for a reasonable amount of test coverage that prevents regressions, so I am okay not having a test for this edge case.

bucefal91’s picture

Yup. I tested it too and it looks OK. I executed your steps and confirm it appears in the front end theme. I also reviewed the patch and didn't find anything.

The only thing is that I replaced one more invocation of \Drupal::service() with the injected service. Interdiff will tell it better than my English.

jrockowitz’s picture

Status: Needs review » Fixed

@bucefal91 Thanks for fixing that mistake. I committed the patch.

  • jrockowitz committed 12774d8 on 8.x-5.x authored by bucefal91
    Issue #2939412 by jrockowitz, bucefal91, Ice-D: Tokens not replaced on...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

jrockowitz’s picture

Status: Closed (fixed) » Needs review
FileSize
9.97 KB

The regression in #2968554: Clear cache via the UI is throwing an exception is making want to try a safer approach.

andileco’s picture

I applied this to 5.x-dev on my site, and I am no longer getting the "error occurred" page like I was getting before. The token replacement is working for me too.

andileco’s picture

Status: Needs review » Reviewed & tested by the community

  • jrockowitz committed 029a483 on 8.x-5.x
    Issue #2939412 by jrockowitz, bucefal91, Ice-D: Tokens not replaced on...
jrockowitz’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.