Problem/Motivation

PHP Notice logged when switching "Configuration type" with already selected "Configuration name" in single configuration export screen at /admin/config/development/configuration/single/export.

The PHP Notice in all its "glory":

Notice: Undefined index: #value in template_preprocess_textarea() (line 389 of /web/core/includes/form.inc)

#0 /web/core/includes/bootstrap.inc(312): _drupal_error_handler_real()
#1 /web/core/includes/form.inc(389): _drupal_error_handler()
#2 /web/core/lib/Drupal/Core/Theme/ThemeManager.php(287): template_preprocess_textarea()
#3 /web/core/lib/Drupal/Core/Render/Renderer.php(436): Drupal\Core\Theme\ThemeManager->render()
#4 /web/core/lib/Drupal/Core/Render/Renderer.php(449): Drupal\Core\Render\Renderer->doRender()
#5 /web/core/lib/Drupal/Core/Render/Renderer.php(201): Drupal\Core\Render\Renderer->doRender()
#6 /web/core/lib/Drupal/Core/Render/Renderer.php(145): Drupal\Core\Render\Renderer->render()
#7 /web/core/lib/Drupal/Core/Render/Renderer.php(578): Drupal\Core\Render\Renderer->Drupal\Core\Render\{closure}()
#8 /web/core/lib/Drupal/Core/Render/Renderer.php(146): Drupal\Core\Render\Renderer->executeInRenderContext()
#9 /web/core/lib/Drupal/Core/Render/MainContent/AjaxRenderer.php(66): Drupal\Core\Render\Renderer->renderRoot()
#10 /web/core/lib/Drupal/Core/Form/FormAjaxResponseBuilder.php(89): Drupal\Core\Render\MainContent\AjaxRenderer->renderResponse()
#11 /web/core/lib/Drupal/Core/Form/EventSubscriber/FormAjaxSubscriber.php(109): Drupal\Core\Form\FormAjaxResponseBuilder->buildResponse()
#12 [internal function]: Drupal\Core\Form\EventSubscriber\FormAjaxSubscriber->onException()
#13 /web/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php(142): call_user_func()
#14 /vendor/symfony/http-kernel/HttpKernel.php(219): Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch()
#15 /vendor/symfony/http-kernel/HttpKernel.php(91): Symfony\Component\HttpKernel\HttpKernel->handleThrowable()
#16 /web/core/lib/Drupal/Core/StackMiddleware/Session.php(57): Symfony\Component\HttpKernel\HttpKernel->handle()
#17 /web/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(47): Drupal\Core\StackMiddleware\Session->handle()
#18 /web/core/modules/page_cache/src/StackMiddleware/PageCache.php(106): Drupal\Core\StackMiddleware\KernelPreHandle->handle()
#19 /web/core/modules/page_cache/src/StackMiddleware/PageCache.php(85): Drupal\page_cache\StackMiddleware\PageCache->pass()
#20 /vendor/asm89/stack-cors/src/Asm89/Stack/Cors.php(49): Drupal\page_cache\StackMiddleware\PageCache->handle()
#21 /web/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(47): Asm89\Stack\Cors->handle()
#22 /web/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(52): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle()
#23 /vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle()
#24 /web/core/lib/Drupal/Core/DrupalKernel.php(716): Stack\StackedHttpKernel->handle()
#25 /web/index.php(19): Drupal\Core\DrupalKernel->handle()
#26 {main}

Steps to reproduce

  1. Go to /admin/config/development/configuration/single/export
  2. Select some value in the select dropdown "Configuration type"
  3. Select some value in the select dropdown "Configuration name"
  4. Select some other value in the select dropdown "Configuration type"
  5. PHP Notice happens
  6. Optional start of The Apocalypse

Proposed resolution

I think this is all caused by the select dropdown "Configuration name" not being "reset" to a "neutral/none" value when switching "Configuration type"

Remaining tasks

The regular ol' 4 P's:

Patch/create MR
Poke at patch (aka Review)
Push to latest dev-branch (aka Commit)
Party

User interface changes

API changes

Data model changes

Release notes snippet

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Spokje created an issue. See original summary.

Spokje’s picture

Issue summary: View changes
Spokje’s picture

Issue summary: View changes
Spokje’s picture

Issue tags: +Bug Smash Initiative
Spokje’s picture

Issue summary: View changes
larowlan’s picture

I vaguely recall a similar issue from the early days of bugsmash

This may be a duplicate

larowlan’s picture

yivanov’s picture

cilefen’s picture

yivanov’s picture

@cilefen, I don't think it's related, because right now the UI is not usable at all, whilst the change in the other issue thread is about having - Select - as first option for the Simple configuration option. The issue is 6 years old and marked as "Postponed", which clearly doesn't seem to solve the issue we have here soon.

I am attaching the small patch I made here as well in case you struggle like me to use the UI for config exports.

jrockowitz’s picture

I think the notice that we are trying to fix is a regression triggered by #3084436: Config export field should be cleared when config type changes

I think we should address the regression and not change template_preprocess_textarea().

The attached patch fixes the regression.

Also the patch from #10 uses !empty($element['#value']) which means a '0' won't be rendered because it is considered an empty string.
@see https://stackoverflow.com/questions/4139301/0-as-a-string-with-empty-in-php

jrockowitz’s picture

Status: Active » Needs review
longwave’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me.

larowlan’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: +Needs tests

Is there a way we can add a test for this to prevent it re-occurring?

jrockowitz’s picture

I don't have the bandwidth to add the needed test coverage. Hopefully, someone can step in and help.

larowlan credited parkh.

larowlan’s picture

larowlan’s picture

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

larowlan credited gcb.

larowlan’s picture

Crediting gcb who filed a duplicate with a patch

longwave’s picture

Status: Needs review » Needs work

NW for a test.

Spokje’s picture

mikelutz’s picture

Title: PHP Notice logged when switching "Configuration type" with already selected "Configuration name" in single configuration export screen » PHP Notice logged when switching "Configuration type" in single configuration export screen
Status: Needs work » Needs review
FileSize
2.61 KB
3.28 KB

Ran into this this week. It becomes a problem if you install devel and disable the standard drupal error handler by setting the error handler to none in the devel settings. PHP's handler prints the notice at the top of the ajax response, making the form unable to switch between configuration types at all, until I re-enabled drupal's error handler.

Which does lead to a possibly effective, if not ugly test for this. It still would be better to address ajax error handling in a JS test in a more global way though.

mikelutz’s picture

well pbbft.. must need to set display_errors and error_reporting to E_ALL too. It worked locally :-(

bensti’s picture

i just get into this problem and i just uninstall devel and re enable after exporting my config view...

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

AdamPS’s picture

I confirm that if the error handler is set to None in devel settings, the single export page becomes unusable.

I don't see any way to get a failing test without #3228956 being committed.

Nice try @mikelutz, but perhaps @Spokje is correct and it's impossible to create a test? In which case, should we create a patch with the single line change, excluding the tests? The patch is just a single line and seems safe, so hopefully the committers could commit it in these circumstances.

longwave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

Agree that this is a one line trivial fix that is difficult to write a test for without adding a ton of additional code, hopefully core committers will let this one slide? Back to RTBC for #11.

mikelutz’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
2.69 KB
3.36 KB
696 bytes

I want to try one more thing, probably won't work. If it doesn't work, I'll reupload #11 and put this back to RTBC. I think the change here is obviously correct, and if we can solve #3228956: PHP notices are not detected for headless browser-triggered requests in FunctionalJavascript tests then this and a few others like it will automatically be tested for, I'm mostly just curious at this point.

alexpott’s picture

Version: 9.5.x-dev » 9.4.x-dev
Status: Needs review » Fixed

Ideally we could use WebDriverTestBase's ability to record JS errors but there is not one here - it's a PHP warning. We need to extend the ability of the js_testing_log_test to monitor the headers of AJAX responses and log any headers that start with X-Drupal-Assertion- but that's for a different issue. Let's do #11. There is an existing issue - see #3228956: PHP notices are not detected for headless browser-triggered requests in FunctionalJavascript tests.

Committed and pushed ddd29c4ca2 to 10.1.x and bc2efd7306 to 10.0.x and 03e037d86f to 9.5.x and b9532e75b0 to 9.4.x. Thanks!

Xpost with @mikelutz - I don't think doing something specific for this issue is the correct thing. Whilst #30 will work it doesn't tell you what the problem with the ajax request was so it's not a good test.

  • alexpott committed ddd29c4 on 10.1.x
    Issue #3225381 by mikelutz, jrockowitz, yivanov, Spokje, larowlan,...

  • alexpott committed bc2efd7 on 10.0.x
    Issue #3225381 by mikelutz, jrockowitz, yivanov, Spokje, larowlan,...

  • alexpott committed 03e037d on 9.5.x
    Issue #3225381 by mikelutz, jrockowitz, yivanov, Spokje, larowlan,...

  • alexpott committed b9532e7 on 9.4.x
    Issue #3225381 by mikelutz, jrockowitz, yivanov, Spokje, larowlan,...
mikelutz’s picture

No argument from me, @alexpott. I don't think the test for this specific case is worth it, and mine was not ideal. It would be much better to do #3228956: PHP notices are not detected for headless browser-triggered requests in FunctionalJavascript tests for the general case, and I see no reason to wait on this fix for that issue, which would need to fix this error and half a dozen others to pass anyway. Glad to see the fix committed. Thanks!

Status: Fixed » Closed (fixed)

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