diff --git a/core/includes/bootstrap.inc b/core/includes/bootstrap.inc index 1d05b4a..ecb4c1d 100644 --- a/core/includes/bootstrap.inc +++ b/core/includes/bootstrap.inc @@ -514,10 +514,10 @@ function watchdog_exception($type, Exception $exception, $message = NULL, $varia * drupal_set_message(t('An error occurred and processing did not complete.'), 'error'); * @endcode * - * @param string $message + * @param string|array|\Drupal\Component\Utility\SafeStringInterface $message * (optional) The translated message to be displayed to the user. For * consistency with other messages, it should begin with a capital letter and - * end with a period. + * end with a period. A render array may also be provided for markup. * @param string $type * (optional) The message's type. Defaults to 'status'. These values are * supported: @@ -526,7 +526,10 @@ function watchdog_exception($type, Exception $exception, $message = NULL, $varia * - 'error' * @param bool $repeat * (optional) If this is FALSE and the message is already set, then the - * message won't be repeated. Defaults to FALSE. + * message won't be repeated. Defaults to FALSE. SafeStrings and strings which + * are equivalent will not be repeated as the in_array() comparison is not + * strict. However, if a string is passed once as a render array and once as + * string or SafeString this will repeated regardless of this setting. * * @return array|null * A multidimensional array with keys corresponding to the set message types. @@ -562,9 +565,13 @@ function drupal_set_message($message = NULL, $type = 'status', $repeat = FALSE) } $new = array( - 'safe' => SafeMarkup::isSafe($message), - 'message' => (string) $message, + // Only preserve safeness for strings. Objects and render arrays do not + // need to be marked as safe. + 'safe' => (is_string($message) && SafeMarkup::isSafe($message)), + 'message' => $message, ); + // Do not use strict type checking so that equivalent string and + // SafeStringInterface objects are detected. if ($repeat || !in_array($new, $_SESSION['messages'][$type])) { $_SESSION['messages'][$type][] = $new; } @@ -610,6 +617,8 @@ function drupal_get_messages($type = NULL, $clear_queue = TRUE) { // the messages also needs to be stored in the session. We retrieve the // safe status here and determine whether to mark the string as safe or // let autoescape do its thing. See drupal_set_message(). + // It's only nessasary to set the message as 'safe' when the message is + // not a renderable array. if ($message['safe']) { $message['message'] = SafeMarkup::set($message['message']); } diff --git a/core/modules/system/src/Tests/Bootstrap/DrupalSetMessageTest.php b/core/modules/system/src/Tests/Bootstrap/DrupalSetMessageTest.php index 37a1663..394c0b2 100644 --- a/core/modules/system/src/Tests/Bootstrap/DrupalSetMessageTest.php +++ b/core/modules/system/src/Tests/Bootstrap/DrupalSetMessageTest.php @@ -24,23 +24,41 @@ class DrupalSetMessageTest extends WebTestBase { public static $modules = array('system_test'); /** - * Tests setting messages and removing one before it is displayed. + * Tests drupal_set_message(). */ - function testSetRemoveMessages() { + function testDrupalSetMessage() { // The page at system-test/drupal-set-message sets two messages and then // removes the first before it is displayed. $this->drupalGet('system-test/drupal-set-message'); $this->assertNoText('First message (removed).'); - $this->assertText('Second message (not removed).'); - } + $this->assertRaw(t('Second message with markup! (not removed).')); - /** - * Tests setting duplicated messages. - */ - function testDuplicatedMessages() { - $this->drupalGet('system-test/drupal-set-message'); + // Ensure duplicate messages are handled as expected. $this->assertUniqueText('Non Duplicated message'); $this->assertNoUniqueText('Duplicated message'); + + // Ensure render arrays are rendered. + $this->assertRaw('Render array with markup!'); + $this->assertUniqueText('Render array with markup!'); + $this->assertRaw('Render array 2 with markup!'); + + // Ensure render arrays with objects are rendered. + $this->assertRaw('Render array with SafeString markup!'); + $this->assertUniqueText('Render array with SafeString markup!'); + + // Ensure SafeString objects are rendered as expected. + $this->assertRaw('SafeString with markup!'); + $this->assertUniqueText('SafeString with markup!'); + $this->assertRaw('SafeString2 with markup!'); + + // Ensure when the same message is of different types it is not duplicated. + $this->assertUniqueText('Non duplicate SafeString / string.'); + $this->assertNoUniqueText('Duplicate SafeString / string.'); + + // Render arrays and strings which are equivalent cannot be de-duplicated. + // Test this known shortcoming. + // @todo Consider resolving this in https://www.drupal.org/node/2545414. + $this->assertNoUniqueText('Duplicate render array / string'); } } diff --git a/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php b/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php index 5ac7ca0..ecb25cc 100644 --- a/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php +++ b/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php @@ -10,6 +10,7 @@ use Drupal\Core\Access\AccessResult; use Drupal\Core\Controller\ControllerBase; use Drupal\Core\Render\RendererInterface; +use Drupal\Core\Render\SafeString; use Drupal\Core\Session\AccountInterface; use Drupal\Core\Url; use Symfony\Component\HttpFoundation\RedirectResponse; @@ -101,7 +102,7 @@ public function mainContentFallback() { public function drupalSetMessageTest() { // Set two messages. drupal_set_message('First message (removed).'); - drupal_set_message('Second message (not removed).'); + drupal_set_message(t('Second message with markup! (not removed).')); // Remove the first. unset($_SESSION['messages']['status'][0]); @@ -112,6 +113,39 @@ public function drupalSetMessageTest() { drupal_set_message('Duplicated message', 'status', TRUE); drupal_set_message('Duplicated message', 'status', TRUE); + + // Add a render array message. + drupal_set_message(['#markup' => 'Render array with markup!']); + // Test duplicate render arrays. + drupal_set_message(['#markup' => 'Render array with markup!']); + // Ensure that multiple render array messages work. + drupal_set_message(['#markup' => 'Render array 2 with markup!']); + + // Add a render array message containing objects. + drupal_set_message(['#markup' => SafeString::create('Render array with SafeString markup!')]); + // Test duplicate render array messages. + drupal_set_message(['#markup' => SafeString::create('Render array with SafeString markup!')]); + + // Add a SafeString message. + drupal_set_message(SafeString::create('SafeString with markup!')); + // Test duplicate SafeString messages. + drupal_set_message(SafeString::create('SafeString with markup!')); + // Ensure that multiple SafeString messages work. + drupal_set_message(SafeString::create('SafeString2 with markup!')); + + // Test mixing of types. + drupal_set_message(SafeString::create('Non duplicate SafeString / string.')); + drupal_set_message('Non duplicate SafeString / string.'); + + drupal_set_message(SafeString::create('Duplicate SafeString / string.'), 'status', TRUE); + drupal_set_message('Duplicate SafeString / string.', 'status', TRUE); + + // This mixing of types will cause a duplicate even though it is not + // intended. + // @todo Consider resolving this in https://www.drupal.org/node/2545414. + drupal_set_message(['#markup' => 'Duplicate render array / string']); + drupal_set_message('Duplicate render array / string'); + return []; }