It seems like the ModuleHandler assumes that .module
files of all enabled modules are loaded at the moment of hook invokation, and doesn't try to load them "on-demand".
On the other hand, the ServerCommand doesn't load .module
files too.
That's weird.
I'll check older branches of core.
Shield module users experience a similar issue - 🐛 On View TypeError: Drupal\Core\Entity\EntityTypeManager Active .
pingwin4eg → created an issue.
Looks like a hook is trying to be invoked, but the module, which implements it, is not loaded.
I have a similar issue
TypeError: Drupal\Core\Entity\EntityFieldManager::{closure:Drupal\Core\Entity\EntityFieldManager::buildBaseFieldDefinitions():289}(): Argument #1 ($hook) must be of type callable, string given, called in web/core/lib/Drupal/Core/Extension/ModuleHandler.php on line 307
but right after the cache rebuilding. And I don't use the shield module, this issue is the only similar one appearing in google results. So I think the issue can be in Drupal core actually.
Drupal 11.1.6, PHP 8.4.5.
The problem still exists in 1.5.0.
The node doesn't have a translation yet, and the language link leads to /fr/en-path for example, which doesn't exist, so the page shows 404. And on this 404 page the language switcher links are /system/404 and /fr/system/404.
Please do the same as the default language switcher - instead of /fr/en-path links display /fr/node/NID (where nid is the same for all translations). That page still shows untranslated node with translated interface, but at least it's not 404.
The actual issue is in the advanced_language_selector module - 🐛 Error in Combination with facets module. Active
@xurdep Please reopen the issue.
Same for me. I installed the module and added a block (with a BS dropdown style) normally. When I visit the front page I get the exception.
drupal 10.3.6
advanced_language_selector 1.5.0
facets 2.0.9
TypeError: Drupal\facets\LanguageSwitcherLinksAlterer::alter(): Argument #2 ($type) must be of type string, null given, called in /app/docroot/modules/contrib/facets/facets.module on line 351 in Drupal\facets\LanguageSwitcherLinksAlterer->alter() (line 88 of modules/contrib/facets/src/LanguageSwitcherLinksAlterer.php).
facets_language_switch_links_alter(Array, NULL, Object) (Line: 552)
Drupal\Core\Extension\ModuleHandler->alter('language_switch_links', Array, NULL, Object) (Line: 414)
Drupal\language\ConfigurableLanguageManager->getLanguageSwitchLinks(NULL, Object) (Line: 231)
Drupal\advanced_language_selector\Plugin\Block\LanguageSelectorBlock->build() (Line: 171)
Drupal\block\BlockViewBuilder::preRender(Array)
call_user_func_array('Drupal\block\BlockViewBuilder::preRender', Array) (Line: 113)
Drupal\Core\Render\Renderer->doTrustedCallback('Drupal\block\BlockViewBuilder::preRender', Array, 'Render #pre_render callbacks must be methods of a class that implements \Drupal\Core\Security\TrustedCallbackInterface or be an anonymous function. The callback was %s. See https://www.drupal.org/node/2966725', 'exception', 'Drupal\Core\Render\Element\RenderCallbackInterface') (Line: 870)
Drupal\Core\Render\Renderer->doCallback('#pre_render', 'Drupal\block\BlockViewBuilder::preRender', Array) (Line: 432)
Drupal\Core\Render\Renderer->doRender(Array, 1) (Line: 248)
Drupal\Core\Render\Renderer->render(Array, 1) (Line: 165)
Drupal\Core\Render\Renderer->Drupal\Core\Render\{closure}() (Line: 638)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 166)
Drupal\Core\Render\Renderer->renderInIsolation(Array) (Line: 191)
Drupal\Core\Render\Renderer->doRenderPlaceholder(Array) (Line: 723)
Drupal\Core\Render\Renderer->Drupal\Core\Render\{closure}()
Fiber->start() (Line: 731)
Drupal\Core\Render\Renderer->replacePlaceholders(Array) (Line: 608)
Drupal\Core\Render\Renderer->doRender(Array, 1) (Line: 248)
Drupal\Core\Render\Renderer->render(Array, 1) (Line: 153)
Drupal\Core\Render\Renderer->Drupal\Core\Render\{closure}() (Line: 638)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 154)
Drupal\Core\Render\Renderer->renderRoot(Array) (Line: 288)
Drupal\Core\Render\HtmlResponseAttachmentsProcessor->renderPlaceholders(Object) (Line: 74)
Drupal\big_pipe\Render\BigPipeResponseAttachmentsProcessor->processAttachments(Object) (Line: 45)
Drupal\Core\EventSubscriber\HtmlResponseSubscriber->onRespond(Object, 'kernel.response', Object)
call_user_func(Array, Object, 'kernel.response', Object) (Line: 111)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object, 'kernel.response') (Line: 214)
Symfony\Component\HttpKernel\HttpKernel->filterResponse(Object, Object, 1) (Line: 202)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 53)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 32)
Drupal\big_pipe\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 191)
Drupal\page_cache\StackMiddleware\PageCache->fetch(Object, 1, 1) (Line: 128)
Drupal\page_cache\StackMiddleware\PageCache->lookup(Object, 1, 1) (Line: 82)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 36)
Drupal\Core\StackMiddleware\AjaxPageState->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 741)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
The block config for the reference:
uuid: 83d2a038-5384-4a4e-923e-8e8dc2f74d09
langcode: en
status: true
dependencies:
module:
- advanced_language_selector
- block_class
theme:
- custom
third_party_settings:
block_class:
classes: 'd-none d-lg-block'
id: custom_advancedlanguageselectorblock
theme: custom
region: navbar_secondary
weight: -28
provider: null
plugin: advanced_language_selector_block
settings:
id: advanced_language_selector_block
label: 'Advanced language selector block'
label_display: '0'
provider: advanced_language_selector
look_and_feel:
theme: bootstrap_dropdown
bootstrap_navigation:
general:
orientation: horizontal
style: nav-pills
css: ''
text_transformation: default
load_external_bootstrap: 0
external_bootstrap_library: advanced_language_selector/bootstrap
display:
items:
show:
icons: icons
lang_code: lang_code
lang_name: 0
icon_height: '25'
icon_alignment: left
bootstrap_offcanvas:
general:
id: ''
position: offcanvas-start
css: ''
text_transformation: default
load_external_bootstrap: 0
external_bootstrap_library: advanced_language_selector/bootstrap
display:
selected_item:
css: btn-primary
show:
icons: icons
lang_code: lang_code
lang_name: 0
icon_height: '25'
icon_alignment: right
items:
show:
icons: icons
lang_code: lang_code
lang_name: 0
icon_height: '25'
icon_alignment: left
plain_html_list:
general:
css: ''
text_transformation: default
display:
items:
show:
icons: icons
lang_code: lang_code
lang_name: 0
icon_height: '25'
icon_alignment: left
bootstrap_modal:
general:
id: ''
css: ''
text_transformation: default
load_external_bootstrap: 0
external_bootstrap_library: advanced_language_selector/bootstrap
display:
selected_item:
css: btn-primary
show:
icons: icons
lang_code: lang_code
lang_name: 0
icon_height: '25'
icon_alignment: right
items:
show:
icons: icons
lang_code: lang_code
lang_name: 0
icon_height: '25'
icon_alignment: left
plain_html:
general:
css: ''
uppercase: 1
display:
icons: 1
show:
lang_code: lang_code
lang_name: 0
icon_height: '25'
icon_alignment: right
bootstrap_dropdown:
general:
id: language_switcher
css: language-switcher
text_transformation: default
load_external_bootstrap: 0
external_bootstrap_library: advanced_language_selector/bootstrap
display:
selected_item:
css: btn-primary
show:
icons: icons
lang_code: 0
lang_name: 0
icon_height: '18'
icon_alignment: right
items:
show:
icons: icons
lang_code: 0
lang_name: 0
icon_height: '18'
icon_alignment: left
bootstrap_list_group:
general:
css: ''
text_transformation: default
load_external_bootstrap: 0
external_bootstrap_library: advanced_language_selector/bootstrap
display:
items:
show:
icons: icons
lang_code: lang_code
lang_name: 0
icon_height: '25'
icon_alignment: left
bootstrap_button_group:
general:
css: ''
text_transformation: default
load_external_bootstrap: 0
external_bootstrap_library: advanced_language_selector/bootstrap
display:
items:
css: btn-primary
show:
icons: icons
lang_code: lang_code
lang_name: 0
icon_height: '25'
icon_alignment: left
visibility: { }
@pawelgorski87 Why don't you read issue comments? Please read them, and revert your breaking changes.
Thanks @sourabhsisodia_ that's a great start.
I looked through the code and I see an alt field, but not the title. I think it should be there too.
Would be good to see some screenshots of the result before I or anyone interested will get to manual testing.
pingwin4eg → created an issue.
pingwin4eg → created an issue.
pingwin4eg → created an issue.
I've been thinking about this, and now I see 2 reasons to actually switch from guzzle to a "wrapper in wrapper" way:
- As
http_client
service can be swapped with another one, it is unclear if another service would use same option names as guzzle. - If I understand correctly, guzzle fetches the whole response at once, so using streams could be better in terms of performance (time, memory).
Fix me if I'm wrong.
$this->config['stream'] = true
This is incorrect approach since this way guzzle uses stream wrapper, leading to an indefinite recursion: remote stream wrapper -> guzzle -> remote stream wrapper...
I did some investigation regarding context options, and I think it's possible to provide the method, headers and probably some other options from the stream context (stream_context_get_options($this->context)
) to guzzle's request() $options, but I'm not sure about all of them (not even sure it's possible, but I'm not an expert in this field).
There's a separate issue for remote_stream_wrapper:7.x-1.x-dev - #2522056: Use the native PHP stream wrapper if available. → . It tries to achieve the same (or at least similar) goal using a different approach - "by wrapping the wrapper" if I understand it correctly. Maybe that is a better way, but as I see from comments - still not everything is possible.
This issue summary needs update to be more general and clear. But considering my observations I don't know whether the issue should try to work out all the context options, or not, and in which way.
PhpUnit previous major (Drupal: 10.3.6, PHP 8.1.30) and next minor (Drupal: 11.1.0-dev, PHP 8.3.13) both pass on all tests except one for HttpMimeTypeGuesserTest, but that should be fixed by 📌 Fix mime-type guesser for Drupal >= 10.3.2 Active .
I'm also going to manually test this MR in one of my projects in the near future. And I'm thinking (not sure) - whether the auto-testing for file functions (fopen, etc.) is needed.
pingwin4eg → changed the visibility of the branch project-update-bot-only to hidden.
Working on removing constructor from stream wrapper.
pingwin4eg → changed the visibility of the branch 2.x to hidden.
Phpunit passed for Drupal 10.3.6 and PHP 8.1.30 - https://git.drupalcode.org/issue/remote_stream_wrapper-3486396/-/jobs/32... . And failed for D11 (expected, because the 2.x branch doesn't ready for D11 yet).
pingwin4eg → created an issue.
It seems the 8.x-1.x branch is not supported anymore, since it's not in the table of releases on the main project page.
The 2.x branch is D10-compatible.
The 2.x branch is already D10-compatible, omitting this issue.
Re: MR !12
Stream wrapper must not have a constructor. Because this breaks fopen()
and alike functions:
[error] ArgumentCountError: Too few arguments to function Drupal\remote_stream_wrapper\StreamWrapper\HttpStreamWrapper::__construct(), 0 passed and exactly 1 expected in Drupal\remote_stream_wrapper\StreamWrapper\HttpStreamWrapper->__construct() (line 53 of .../web/modules/contrib/remote_stream_wrapper/src/StreamWrapper/HttpStreamWrapper.php) #0 [internal function]: Drupal\remote_stream_wrapper\StreamWrapper\HttpStreamWrapper->__construct()
Look at core implementations of StreamWrapperInterface
- they all use static calls to \Drupal::service(...)
or similar.
I get another error in 1.6-rc1:
ArgumentCountError: Too few arguments to function Drupal\Core\Form\ConfigFormBase::__construct(), 1 passed in .../web/modules/contrib/linked_field/src/Form/ConfigForm.php on line 36 and exactly 2 expected in Drupal\Core\Form\ConfigFormBase->__construct() (line 43 of core/lib/Drupal/Core/Form/ConfigFormBase.php).
pingwin4eg → created an issue.
Shouldn't the change record be in the draft status at this stage? I see it's published, but the issue isn't committed yet, so nothing described in it works.
And looking at the form code, it's not only with sqlite. Other DB types, drush also have versions hardcoded in strings. The correct implementation is with the PHP version check.
Ah, so the actual minimum is different. Well, then yes, the text is wrong. The version value in it should be replaced with a placeholder.
I've patched the module with project-update-bot-only changes (those only bumping versions) in one of my projects and updated Drupal core to 11. It runs OK on cron - a migration is being scheduled and items are imported without issues. Auto-tests are green. It would be great to see these changes committed.
pingwin4eg → created an issue.
I see the same warning, but on the "GTranslate Settings" page, right after installing the module and visiting the page.
GTranslate module 3.0.1
Drupal 10.2.5
PHP 8.1.28
Warning: foreach() argument must be of type array|object, string given in Drupal\Core\Render\Element\Checkboxes::valueCallback() (line 113 of core/lib/Drupal/Core/Render/Element/Checkboxes.php).
Drupal\Core\Render\Element\Checkboxes::valueCallback(Array, , Object)
call_user_func_array(Array, Array) (Line: 1285)
Drupal\Core\Form\FormBuilder->handleInputElement('gtranslate_admin', Array, Object) (Line: 1006)
Drupal\Core\Form\FormBuilder->doBuildForm('gtranslate_admin', Array, Object) (Line: 1076)
Drupal\Core\Form\FormBuilder->doBuildForm('gtranslate_admin', Array, Object) (Line: 1076)
Drupal\Core\Form\FormBuilder->doBuildForm('gtranslate_admin', Array, Object) (Line: 579)
Drupal\Core\Form\FormBuilder->processForm('gtranslate_admin', Array, Object) (Line: 325)
Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 73)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 627)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 181)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 32)
Drupal\big_pipe\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 36)
Drupal\Core\StackMiddleware\AjaxPageState->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 704)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
pingwin4eg → made their first commit to this issue’s fork.
Is it safe to use class names like that in recent comments?
What if an extension declaring the hook is not dependent on those other extensions and classes may be missing?
Is it possible to use just extension names instead?
hestenet → credited pingwin4eg → .
Needs work / doesn't pass tests:
- https://git.drupalcode.org/project/styleswitcher/-/jobs/2339518
- https://git.drupalcode.org/project/styleswitcher/-/jobs/2339511
pingwin4eg → made their first commit to this issue’s fork.
pingwin4eg → created an issue.
pingwin4eg → created an issue.
Not sure whether it's a bug or not.
pingwin4eg → created an issue.
pingwin4eg → created an issue.
Hi all. Will this issue be further worked on? I mean the CKEditor support. Should I apply the patch or one of merge requests?
pingwin4eg → created an issue.
Drush error appears even without a table prefix, removing it from IS. It was caused by xdebug.
I found out that the SQLite driver uses database attachment concept instead of actual table prefixes. Not sure whether this is OK or not, and why it was implemented this way.
If it's OK, then the settings.php documentation needs to mention that, the prefix is better be prepended to a DB file name or appended to a base name before the file extension, and reported errors need to be fixed.
In D11 the situation is a bit different, but the main problem is the same. Updated IS.
pingwin4eg → created an issue.
Trying to run php web/core/scripts/drupal server
in D11 I get another error:
The service file "core/core.services.yml" is not valid.
The patch applies and fixes the error.
The only remaining task is probably to write a test case. I'm not sure about the directory structure in the testing environment, is there a "web" (or another docroot) directory?
pingwin4eg → created an issue.
If this is still an issue, feel free to re-open, and work on a patch.
I'm trying to understand the case where this can be useful. Why a cookie is not enough for your case?
I'm trying to understand the case where this option can be useful.
I have a site with the multisite system.
So I need to have domain setting for cookies.
So currently the module sets a cookie for a domain which a visitor is viewing when switching styles. Do you want a site visitor to click a link in one of multisites, but view another one in a chosen style?
Hi @vishal.kadam . First of all, thanks for reporting this.
Unfortunately, I can't reproduce the bug in a "fresh" installation. You should provide more context.
As for your patch, it just prevents the error in console, but doesn't fix it. You click a link in a block, but a style is not changing, if I understand correctly.
I suggest you to try the latest dev version of the module in your installation, and report back. If the error still exists, please provide more details - which browser, drupal core version, other installed modules, anything that might help reproduce it.
@radheymkumar What's the purpose of the patch you uploaded? If I understand correctly, there are only changes to field_collection.api.php and tests (from the patch #11).
Auto-tests pass. Manual testing remains.
Thanks @fjgarlin. I did as you said, ran pipeline in the issue fork repo manually after commit, and it worked. The log - https://git.drupalcode.org/issue/styleswitcher-3403145/-/jobs/489497
pingwin4eg → created an issue.
Thanks @nitin_lama for working on this.
I'll continue from here.
The Nightwatch testing fail seems unrelated.
DrupalPractice code sniffing standard revealed more errors and warnings to fix.
quietone → credited pingwin4eg → .
AFAIK themes don't have a .module file, these should be changed to .theme in this doc.
pingwin4eg → created an issue.
The bug seems to appear only when $config['system.performance']['css']['preprocess'] = TRUE
. When it is so, the library's media attribute is changed from the custom value to 'all', so \Drupal\styleswitcher\StyleswitcherElementInfoAlter::preRenderHtmlTag()
doesn't set the id attribute to the link element.
Opened 🐛 Fix dynamic style switching in D10 Active for the described bug.
Thanks again everyone who worked on compatibility fixes.
pingwin4eg → created an issue.
Manual testing showed a bug - "dynamic" style switching doesn't work, you can see a style applied only after a page refresh or after opening another page. This misbehavior happens only in Drupal 10 on a real site, but not under the test (which is weird). So basically that is not an incompatibility with D10.
I'm going to commit the changes in the patch, and start a new issue for the described bug.
I think the best would be to have a template instead of styleswitcher_admin_style_overview
and a pre_render callback instead of theme_styleswitcher_admin_styles_table
.
Thank you Tim, and thank you DA! We really appreciate this.
Then it's a separate issue (and requires steps/explanation how it happens). This one is for compatibility fixes.
Does it happen only in D10?
Closed MR, because it was not full.
-
+++ b/js/styleswitcher.js @@ -56,14 +56,16 @@ - // Update the cookie first. - Drupal.styleSwitcher.cookie(style.name); + if (style) { + // Update the cookie first. + Drupal.styleSwitcher.cookie(style.name); - // Now switch the stylesheet. Path is absolute URL with scheme. - $('#styleswitcher-css').attr('href', style.path); + // Now switch the stylesheet. Path is absolute URL with scheme. + $('#styleswitcher-css').attr('href', style.path); - // Cosmetic changes. - Drupal.styleSwitcher.switchActiveLink(style.name); + // Cosmetic changes. + Drupal.styleSwitcher.switchActiveLink(style.name); + } };
Is this necessary?
-
+++ b/styleswitcher.libraries.yml @@ -6,7 +6,7 @@ styleswitcher: - - core/jquery.once + - core/once
jQuery is still used in styleswitcher.js.
Interdiffs would save reviewers' time, by the way.
There's one more use of the '#theme' => 'styleswitcher_admin_style_overview'
in StyleswitcherAdmin::buildForm()
, which is not worked out in the patch.
Reviewing.
I also encountered the "container was serialized" issue with a migration test in Style Switcher module with D 9.5.11, PHP 8.1, PHPUnits 8.5.31 (local) and 9.5.28 (DrupalCI):
1) Drupal\Tests\styleswitcher\Kernel\Migrate\d7\MigrateVariablesTest::testVariablesMigration
PHPUnit\Framework\Exception: PHP Fatal error: Uncaught AssertionError: The container was serialized. in /var/www/html/core/lib/Drupal/Core/DependencyInjection/ContainerBuilder.php:99
Stack trace:
#0 /var/www/html/core/lib/Drupal/Core/DependencyInjection/ContainerBuilder.php(99): assert(false, 'The container w...')
#1 [internal function]: Drupal\Core\DependencyInjection\ContainerBuilder->__sleep()
#2 Standard input code(89): serialize(Array)
#3 Standard input code(123): __phpunit_run_isolated_test()
#4 {main}
thrown in /var/www/html/core/lib/Drupal/Core/DependencyInjection/ContainerBuilder.php on line 99
When I switched PHP to 7.3 locally I saw the actual error:
1) Drupal\Tests\styleswitcher\Kernel\Migrate\d7\MigrateVariablesTest::testVariablesMigration
PHPUnit\Framework\Exception: PHP Fatal error: Uncaught LogicException: The database connection is not serializable. This probably means you are serializing an object that has an indirect reference to the database connection. Adjust your code so that is not necessary. Alternatively, look at DependencySerializationTrait as a temporary solution. in /app/core/lib/Drupal/Core/Database/Connection.php:2042
Stack trace:
#0 [internal function]: Drupal\Core\Database\Connection->__sleep()
#1 Standard input code(80): serialize(Array)
#2 Standard input code(112): __phpunit_run_isolated_test()
#3 {main}
thrown in /app/core/lib/Drupal/Core/Database/Connection.php on line 2042
The workaround was to call $this->startCollectingMessages();
before $this->executeMigrations(...)
.
Thank you folks for your work on this issue!
Tests are failing, so changing status to NW. Going to review and test everything myself.
I closed the issue because the module was designed to switch a style without page reloading.