Problem/Motivation
_did_this_help_send_info() passes five values straight to Html::escape(), which accepts only strings. Two of them are routinely not strings, and each produces an uncaught TypeError that turns the AJAX submission into "Oops, something went wrong."
1. The page title can be a render array.
DidThisHelpForm::buildForm() does:
$title = \Drupal::service('title_resolver')->getTitle($request, $route_match->getRouteObject());
$form['title'] = ['#type' => 'hidden', '#value' => $title];
TitleResolver::getTitle() returns whatever the route's _title_callback returned, which is not guaranteed to be a string. Core's UserController::userTitle() returns a render array:
return $user ? ['#markup' => $user->getDisplayName(), '#allowed_tags' => Xss::getHtmlTagList()] : '';
Because the element sets #value explicitly, that array is also what $form_state->getValue('title') returns on the AJAX rebuild, so Html::escape() receives an array:
TypeError: htmlspecialchars(): Argument #1 ($string) must be of type string, array given in htmlspecialchars() (line 440 of core/lib/Drupal/Component/Utility/Html.php)
#0 core/lib/Drupal/Component/Utility/Html.php(440): htmlspecialchars()
#1 modules/contrib/did_this_help/did_this_help.module(33): Drupal\Component\Utility\Html::escape()
#2 modules/contrib/did_this_help/src/Form/DidThisHelpForm.php(199): _did_this_help_send_info()
#3 [internal function]: Drupal\did_this_help\Form\DidThisHelpForm->sendAjaxForm()
Any route whose _title_callback returns a render array is affected; /user/{user} is the reproducible core case. This one is language-independent.
2. The "Other" reason is a TranslatableMarkup object.
buildForm() appends it as an object rather than a string:
$no_choice_list = explode(PHP_EOL, $no_answers);
$no_choice_list[] = $this->t('Other');
and sendAjaxForm() reads the chosen option straight back out:
$choice_no_string = $form['no_choice_wrapper']['no_list']['#options'][$choice_no];
$data['choice_no'] = $choice_no_string;
so choosing "Other" and pressing Send reaches Html::escape() with a TranslatableMarkup, giving the same class of TypeError ("must be of type string, Drupal\Core\StringTranslation\TranslatableMarkup given").
Separately, lines 132-141 resolve the title twice, and the second assignment discards the if (empty($title)) guard above it, making that guard dead code.
Steps to reproduce
- Install and enable Did this help?, place the block.
- Visit
/user/1and press "Yes". Instead of the thank-you message you get the AJAX error, and watchdog logs theTypeErrorabove. - On any page press "No", choose "Other", press "Send" -- same class of
TypeError.
Step 3 currently also needs [#3622284], since the "No" button does not open the reason panel when the interface language is not English.
Proposed resolution
Add a _did_this_help_to_string() helper that flattens render arrays (via renderInIsolation()) and casts markup objects, then route all five values through it before escaping. Use it for the title in buildForm() as well, so the hidden field holds a real string, and drop the duplicated title resolution.
Patch against 2.0.8 attached.
Remaining tasks
Review. Note renderInIsolation() needs Drupal 10.3+, while the module's core_version_requirement is ^10.1 || ^11 || ^12 -- if 10.1/10.2 support matters, the call wants a method_exists() fallback to renderPlain().
User interface changes
None.
API changes
Adds one internal helper function, _did_this_help_to_string().
Data model changes
None. The title and choice_no columns now receive real strings instead of the request failing.
| Comment | File | Size | Author |
|---|---|---|---|
| did_this_help-escape-non-string-values.patch | 3.01 KB | drupalviking |
Comments
Comment #2
drupalvikingComment #4
levmyshkinThanks for the detailed report and the patch, @drupalviking. I applied and committed the fix to the 2.0.x branch and released it in 2.0.9.
Marking as fixed.
Comment #6
levmyshkin