Problem/Motivation
Splitting this from #3392196: Exceptions in batch no longer are shown on the page when Javascript is disabled because there are actually 2 reasons why this happens. Once if you have Javascript enabled and one if you do not.
Obviously most users will have JS enabled but the non-JS version is still important because it means you can't test this without being a JS tests.
@bnjmnm comment on #3392196-21: Exceptions in batch no longer are shown on the page when Javascript is disabled
Info on the FunctionalJavaScript test failure:
- The test failure is in fact due to a change introduced [##2987444]
- Prior to that change, AJAX errors were only reported in the JS console. Nothing reported in the Drupal UI. This was addressed by adding
core/drupal.messageas a dependency tocore/drupal.ajax- Apparently
core/drupal.messageassumes theres a'#type' => 'status_messages'render element on the page.\Drupal\Tests\system\FunctionalJavascript\Batch\ProcessingTestis failing because it expects the markup that would be provided by thestatus_messagesrender element.Clearly something needs to be addressed in
core/drupal.message since JSsince JS libraries can't declare dependencies on render elements. It's safe to say that isn't something that needs to be addressed in this issue's scope. Adding a'#type' => 'status_messages'element to the test page should get the FJS test working, and the underlying issue has its own issue now #3396099: The core/drupal.message library requires a status_messages render element
Steps to reproduce
Proposed resolution
Use the approach in 3406612-use-messages-theme merge request
In \Drupal\system\Controller\BatchController::batchPage simply add
// Directly render a status message placeholder without any messages.
// Messages are not intended to be show on the batch page, but in the event
// an error in a AJAX callback the messages will be displayed.
$output['batch_messages'] = [
'#theme' => 'status_messages',
'#message_list' => [],
'#status_headings' => [
'status' => t('Status message'),
'error' => t('Error message'),
'warning' => t('Warning message'),
],
];
This intentionally does not use '#type' => 'status_messages', which automatically render the server side messages which is not expected behavior on the batch page.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | 3406612-show-messages-5.patch | 500 bytes | tedbow |
Issue fork drupal-3406612
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
tedbowComment #3
tedbowcopying my response to @bnjmnm from the other issue
Comment #5
tedbowLooking at
\Drupal\system\Controller\BatchController::batchPageMy guess is that
#show_messageshere is useless and a holdover from Drupal 7 or 8. https://api.drupal.org/api/drupal/includes%21common.inc/function/drupal_...And the only reason messages aren't shown on the batch page and then display after the batch job is finished is that there is no
element on the page.
Update loading a patch and my guess is that
\Drupal\Tests\locale\Functional\LocaleConfigTranslationImportTest::testConfigTranslationModuleInstallwon't fail like it does when I add thestatus_messageselement.Comment #6
tedbowMarking this as major because it would probably stop #3253158: Add Alpha level Experimental Update Manager module. Anyone who used the module would not see any errors when using the form. We did put a temporary fix in the contrib module to address this #3406812: Ensure that errors are shown on form updates in core 10.1.x which just does
in
hook_page_top(). If this fix is good enough to alpha commit of Automatic Updates in core then we could bump this issue down.Comment #8
tedbowI pushed another branch with a different approach
3406612-use-messages-themeInstead of using the StatusMessages render element and adding changes that would allow it to not actually render server side messages this branch just adds and element with
'#theme' => 'status_messages',, therefore just putting the mark up on the page.This may be simpler than changing
StatusMessagesfor the very unique case of the batch page were you don't want the messages to form the server to showComment #10
tedbowCreated follow-up #3407067: message.js doesn't work status messages element with no child element but whitespace, incompatible default template this keeps us from using stark theme in the test. That issue could also use a review. Fix I think is very simple.
Comment #11
phenaproximaI think this is a reasonable, well-scoped fix for this problem until the more general #3396099: The core/drupal.message library requires a status_messages render element lands. It's good to finally have test coverage around this, too.
Comment #15
lauriiiCommitted d562eda and pushed to 11.x. Also cherry-picked to 10.2.x. Thanks!