🇧🇪Belgium @herved

Whoops, I made a silly mistake with the logger... disregard patch #5.
Also fixing pre-existing cspell and phpcs issues in EntityView.php.

Ok, it seems metatag gets the [node:summary] token which increments the recursion counter. I guess that makes sense.
MR opened, and static composer patch attached.

herved → created an issue.

Christian.wiedemann → credited herved → .

Adding test coverage, and the static patch for composer.

herved → made their first commit to this issue’s fork.

I believe this is causing issues in some scenarios, as explained in #3402972-29: Fetching does not respect use_imagecache_root when using converted webp image styles →
Can anyone confirm? Thanks

#29 still holds true, for me.
I'll crosslink it there.

Got it, #4 is when testing with a single-valued field (see point 1 below).
I was initially testing with multi-valued fields.

To clarify, the patch prevents invalid URLs to be added as a field item. This fixes 2 issues:
1. For single-valued fields, after submitting an invalid URL, and clicking back on "Upload file" radio, the file upload field is not visible anymore. This comes from ManagedFile::preRenderManagedFile interfering, as it sets the #access to FALSE if $element['#value']['fids'] is populated
2. For multi-valued fields, after submitting an invalid URL, an empty/broken field item gets added.

I created 2 branches:
- one that simply adds the missing context in JS
- one that replaces the URL autosubmit and JS with a "Add URL" button.

#4 #5 This patch here prevents exactly that.
In the value callback, the URL now gets validated before is gets added as a field item.

PS: I re-tested on a clean 10.3.x core and file_url 2.x and found another issue, see 🐛 URL not sumbitted (missing context in JS behavior detach Active

herved → created an issue.

herved → created an issue.

This is still causing issues for me.
With both hotlink and use_imagecache_root set to TRUE, and using a file with a dot in its name e.g.: foo.bar.jpg it redirects to foo.bar and wrongly strips the extension (expected is foo.bar.jpg).
You can replicate this with an image field directly with the widget preview (using thumbnail image style which doesn't use any image_convert effect).
Can anyone else confirm this? I'm moving this back to needs work for now.

If I understand correctly, there are multiple scenarios depending on hotlink=TRUE/FALSE and use_imagecache_root=TRUE/FALSE.
Also I wonder, why would this be limited to webp? I suspect it is applicable to any image style using the image_convert effect (which can have many more supported extensions like webp, png, jpeg, jpe, gif,..).

Hello, I stumbled on this for the issue depicted in #108.
We have a sticky header that relies on Drupal displace - top: var(--drupal-displace-offset-top, 0); - which is not positioned correctly on mobile with the toolbar.

Reading #131 is not really reassuring.
Can a compromize be found here maybe?

We are using solr of course, and a custom facet, querying a date_range solr field ().

Patch #12 is not working for me.

+++ b/src/Plugin/search_api/backend/SearchApiSolrBackend.php
@@ -3472,7 +3474,16 @@ class SearchApiSolrBackend extends BackendPluginBase implements SolrBackendInter
+      if ('*' === $value) {

Unless.. was is meant to be if ('*' === $v) { ?

Thanks for your support!

If I understand correctly, solr.DateRangeField uses the spatial functionality.
So in your example it would return it, since they intersect. unless I'm mistaken?

Thanks for your input,

The description is about querying a date range field with something like [2024-03-04T00:00:00Z TO *], which I think should not be supported.
A query like [2024-03-04T00:00:00Z TO *] should only be supported for a date field, which stores a single date, not a range.

I'm confused. From Solr's doc:

Solr’s DateRangeField supports the same point in time date syntax described above (with date math described below) and more to express date ranges.

So I understand that [2024-03-04T00:00:00Z TO *] is perfectly valid, which would filter on >= 2024-03-04T00:00:00Z
Or am I missing something?

solr.DateRangeField was implemented in https://issues.apache.org/jira/browse/SOLR-6103 which does mention such ranges.

Patch #4 works well for me.
We could also add a test, possibly in \Drupal\Tests\search_api_solr\Kernel\Processor\DateRangeTest

#6: I can also confirm 🐛 Filter value formatting might fail on custom Search API field types Fixed introduced a regression, as we are not able to pass * for date range fields anymore.
I suppose not a lot of sites use date range fields but passing * when querying such fields is very common https://solr.apache.org/guide/8_7/working-with-dates.html#date-range-for...

herved → created an issue.

#13: this is what I attempted to explain in #12.
To me the patch from #5 now included is not really what most people would need because it is applied as a multiplier of the overall keyword score.
Therefore it is not independent from keywords, unlike in solr which gets added to the overall score, see https://git.drupalcode.org/project/search_api_solr/-/blob/4.x/src/Plugin...
Also, it has no effect if no keywords are given.

And if we don't search for any keywords, this boosting has no effect at all and will be skipped in \Drupal\search_api_db\Plugin\search_api\backend\Database::createDbQuery

Coming from 📌 Replace upload validators with new ones introduced in Drupal 10.2 Needs work as we are having duplicate extensions descriptions as well on our project.
This issue makes more sense to me and looks like a much cleaner way to add SVG support.
Tested on core 10.2, works perfectly, thank you.

Only 2 minor nits:
1. There is a typo in comments: Druapal > Drupal
2. I think the plugin annotation shouldn't be there as svg_image_field_formatter_info_alter() already registers the class. But this is probably not directly related to this issue.

Hello, and sorry to comment on a closed issue, but is this whole xdebug issue considered work in progress? I see some follow-ups.
I stumbled on a really nasty issue where after submitting a form, status messages would randomly not be displayed.
Session data would not get committed in time (after \Drupal\Core\Session\SessionHandler::write) before the next GET request retrieves the session data.
Or even users would not be created in time/fast enough for the next event to succeed.
All kind of inexplicable events, absolute horror to debug.

Even after updating to core 10.2.4 which includes this issue, the same happens.
Only the workarounds from the IS work (ensure that develop is not in your xdebug.mode value OR pin xdebug in CI image to xdebug 3.2.2).

Hi @Anybody, sure, done.

#12 we had a similar requirement for one of our projects (to sync the published state across translations).
We investigated https://www.drupal.org/project/moderation_state_sync → but this creates tons of revisions so it is not ideal.

After some investigation if looks like moderation state is forced to be translatable (by design), and there is no easy way to get around it.
I came up with this, which instead of syncing makes the moderation state untranslatable, which prevents a revisions galore.
It seems to be working but EntityUntranslatableFieldsConstraintValidator gets triggered and I didn't have time to properly test it.

Eventually we abandoned the idea, for now. But it would be nice to find a solution for this.
Perhaps moderation state could work according to the translatability status of the published field? but perhaps there are implications.

#22 doesn't apply on 10.2.x after 📌 Allow controller service wiring via constructor parameter attributes Fixed
#23 is missing the BuildAlter class, adding it back.

I'm seeing this on Drupal 10.2.3 as well, issue 🐛 big_pipe sometimes fails to load blocks Active got included in 10.2.1 so I guess it makes sense.
The error is almost on all pages for me. It seems one of my browser extensions (Scraper 1.7, on brave, which I now disabled as I'm not using it) is causing DOM mutations on the body element, which triggers this.

big_pipe needs to be more resilient.
The MR fixes it. I wonder though if the NULL check should happen above, in checkMutationAndProcess?
I'll add a static diff patch of it (latest commit 6580c866), for composer, in case anyone else needs it.

Static patch from MR 143, if anyone else needs it.

+++ b/src/Element/Select2.php
@@ -209,7 +210,11 @@ class Select2 extends Select {
+      $element['#prefix'] = '<div id="' . $wrapper_id . '">';
+      $element['#suffix'] = '</div>';

Update: I haven't really checked in depth but why do we need a wrapper div here?
This would need a change record or similar as it can break existing styling in some cases.

Hiding patch: prevent_twig_debug_only-3380299-6.patch (covered by ✨ Make Twig debug comments optional for empty markup. Active )
The other patch (allow_preprocess_abort_rendering-3380299-4.patch) is still relevant though.

For me #20 doesn't work.
I have a theme that is already installed, and some configs using it (in config_sync).
When I change a layout machine name and attempt to clear cache and config import, it fails.
#16 and #3 are working though.

@GiorgosK Interesting, but what if we don't want to make custom menu links translatable?
This PR goes beyond just fixing the PHP error AFAICS, but conceptually it seems to make more sense.
I haven't really reviewed it, I only needed a reroll/static patch.
Perhaps a maintainer could weight in?

This solves the issue for me as well for a similar case, with views, thanks.

Here's a patch of the current MR (latest commit dd8e8c18) in case anyone else needs it.
PS: our setup enforces static patches and MR diffs are not allowed (as they can change unexpectedly).

And here is a patch against 8.x-1.5, if anyone else needs it.
PS: our setup enforces static patches and MR diffs are not allowed (as they can change unexpectedly).

I attempted to rebase the MR but apparently I don't have the rights.
Here's a patch against 8.x-1.x.

Here's a patch from MR 6, latest commit (db2dffba), in case anyone else needs it.
Our setup enforces static patches and MR diffs are not allowed (as they can change unexpectedly).

See #3402972-11: Fetching does not respect use_imagecache_root when using converted webp image styles →

Hi @recrit, @smustgrave,

The suggestion in #8 doesn't seem to fix 🐛 Regression - Local files with a dot in their filenames are not correctly served Active .
Files with a dot are still "hotlinked" without the extension for me.
E.g: some.image.png?itok=**** > some.image?itok=****

Same here on D10.1 with #17, thanks! +1

herved → created an issue.

Here is a patch version of PR 87 on github (latest commit f1cc9a7), if anyone else needs it.
PS: our setup enforces static patches and MR diffs are not allowed (as they can change unexpectedly).

Here's a patch version of MR 4399 (latest commit 21efdb1b), if anyone else needs it.
PS: our setup enforces static patches and MR diffs are not allowed (as they can change unexpectedly).

Here's a patch version of MR 1943 (latest commit bbcb4ac7), if anyone else needs it.
PS: our setup enforces static patches and MR diffs are not allowed (as they can change unexpectedly).

Hi @newswatch, that's correct. This applies cleanly on 3.15, and 3.x-dev, I just tested.
https://git.drupalcode.org/project/ds/-/blob/8.x-3.x/ds.module?ref_type=...
https://git.drupalcode.org/project/ds/-/blob/8.x-3.15/ds.module?ref_type...

Then it might be something else.
Are you sure you enabled a DS layout first? With DS, we first need to enable a DS layout on the view mode before we can use DS field templates. This often comes up.

Are you possibly referring to 🐛 Field formatter settings form and AJAX issue (regression on 10.1.x after #857312) Active ?

A possible fix, based on EntityDisplayFormBase::multistepAjax.

herved → created an issue.

Another side effect: if we have a custom field settings form element with an ajax callback, and we click any input after that callback run, the entire form refreshes and we loose our input values.

This breaks for example Display Suite:
- Go to manage display
- for any field, edit settings and change "Field Template" from "Master" to "Minimal"
- click for example the "Show label colon" or any other field
-> the entire form refreshes and we loose all values we entered.

Either ds_field_ui_display_overview_multistep_js() needs to be updated to add the ajax-new-content class (similar to how EntityDisplayFormBase::multistepAjax does) or is this be considered as a core bug/regression?

Hello @Baysaa,

sr-only is a fontawesome-specific class, added since version 4.6.0 (commit)
I'm not a maintainer but I'm not against changing it to "visually-hidden" which would make it independent from any FA version.
Feel free to post a patch.

This looks nice.
Here is a reroll against 11.x. I only added a small test case if that's ok.

PS: As per ✨ Allow theme preprocess hooks to abort rendering Active , making it non disruptive by using a switch is a good idea, but could also make it harder to solve the other issue I was trying to solve there. Outputting debug markup when the theme output is completely empty prevents a proper emptiness check.
Any opinions on this? I'm kinda stuck there. Perhaps the issue is not scoped very well there.

Thanks @euk

I thought about creating a specific issue for it but I see you did it already, I'll have a look at it, thanks.
It looks like a better approach than #6 here (BTW don't use that one it's messing the order of debug markup).
The approach in #3400912 makes it non disruptive by using a switch, which is interesting but could also make it harder to solve the other issue I'm trying to solve here.
I'll leave this open for now.

I tested patch #9 on 2 projects locally and I can also see a nice performance gain.
Note that I don't have the actual files (pdf, docs, etc) locally so the impact on prod must be much more significant.
Command: drush cr && drush sapi-c && time drush sapi-i

project 1:
- without patch: 5m7,417s
- with patch: 3m31,457s

project 2:
- without patch: 5m24,580s
- with patch: 1m46,645s

Also, switching to DOM parsing instead of regex makes total sense.
I'm not too worried about #14 but it's a good point.
So, +1 thanks!

I noticed 🐛 Make BaseFieldOverride inherit internal property from the base field Fixed fixed isInternal() for BaseFieldOverride in core 10.1.6
I believe it should address point 1 but not point 2 (from #9).
Is my assumption correct in the first place? that we want to skip internal and read-only fields but not computed

Ah indeed. Here's a reroll with latest 1.x (8fc509ec).

Hi @adriancid, the patch in #3 does change things a bit but is it breaking things?
Unless I'm missing something, it only simplifies. I can't find any side effects.

Hi @robpowell, @tcrawford,

It would be great if you could check out the patch in 📌 Stop using $error->arrayPropertyPath Needs review which should fix both issues (this one + the deprecation of arrayPropertyPath), so we could close this as duplicate.

Thanks

Thanks @mkalkbrenner,
I'll test again soon with D10.1, facets 3.x-dev and the BEF patch and close this hopefully.
I believe I had found a bug with isRenderedInCurrentRequest, if so I'll report it in a separate issue.

Note that arrayPropertyPath excludes the delta (first 'key' from getPropertyPath): see \Drupal\Core\Field\WidgetBase::flagErrors

So if you need to retrieve the whole path and you have $violation->arrayPropertyPath
Then you can get the same this way:

$property_path_array = explode('.', $error->getPropertyPath());
array_shift($property_path_array);

Argh, I just stumbled on this, more specifically the issue described in 🐛 Constraints on paragraph field highlight all fields in paragraph Needs review .
I think the approach here makes more sense since arrayPropertyPath is deprecated.

But patch #2 is not working for me: it doesn't remove the delta and doesn't pass the full path.
I have the following violation path: $delta.subform.my_date_field.widget
The delta should be removed just like \Drupal\Core\Field\WidgetBase::flagErrors does:

// Separate violations by delta.
$property_path = explode('.', $violation->getPropertyPath());
$delta = array_shift($property_path);
...
$violation->arrayPropertyPath = $property_path;

Because it later passes the $delta_element to WidgetBase::errorElement
This way NestedArray::getValue() would get ['subform', 'my_date_field', 'widget'] which is correct and returns the sub-element properly.

Hello, thanks for the update.

For me it doesn't work, I tried both the latest 3.0.x (b71823b), and 3.0-beta1 + patch #8 from #3225764.
I'm on core 10.1.4, without views_ajax_get.

It looks like views attempts to get the current page from \Drupal\Core\Pager\PagerParameters::getPagerParameter which retrieves it from $request->query->get('page', ''); while \Drupal\facets\Controller\FacetsViewsAjaxController::facetsRemoveQueryParams removes the page param from $request->query.

This can be replicated with nodes (e.g.: basic pages) by simply adding an untranslated node reference field, adjusting the display to show the rendered entity, and using the following hierarchy and translations of basic pages:
- EN level 0 (translated to FR)
-- EN level 1 (untranslated)
--- EN level 2 (translated to FR)

The only difference with paragraphs is that translations of level 1 are created automatically upon saving the level 0 translations, which we don't get with nodes. But the end result is basically the same.
When viewing FR level 0 with the above setup:
- without the patch: we see EN Level 1, EN Level 2
- with the patch: we see EN Level 1, FR Level 2

I'll try to create a phpunit test when I have some time so it makes more sense.

I created a core issue there 💬 EntityViewDisplay::buildMultiple enforces the langcode even when viewing the default translation Active .
Looks more like a core issue to me from EntityViewDisplay::buildMultiple but any input is welcome.

Test to see what the CI says...

herved → created an issue.

herved → created an issue.

@7 Indeed I also noticed this. This commit changed $request->getMethod() === Request::METHOD_POST to $request->isXmlHttpRequest(). Perhaps it wasn't intentional ?
Again, I have no clue how AJAX facets are supposed to work since I can't make them work myself despite trying so I can't even debug or understand what this code is all about.

In the mean time, here is a fix for #6.

I found an issue with patch #3 when accessing /views/ajax:

Warning: array_flip(): Can only flip string and integer values, entry skipped in Drupal\Core\Entity\EntityStorageBase->loadMultiple() (line 278 of core/lib/Drupal/Core/Entity/EntityStorageBase.php).

Drupal\Core\Entity\EntityStorageBase->loadMultiple(Array) (Line: 262)
Drupal\Core\Entity\EntityStorageBase->load(NULL) (Line: 26)
Drupal\facets\Controller\FacetsViewsAjaxController->ajaxView(Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 583)
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: 166)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 74)
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: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 704)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

It needs to check if the view_name param is present.

Indeed it does, and I see no side effects. I'll use that instead then. Thanks for pointing it out.
I guess we could close this one then?
Cheers

Ok the assert is wrong there it seems.
Not exactly sure what to assert tbh, we mainly need to ensure that there are no exceptions thrown even though the current user context is NULL (because the anonymous user is used).

We could also do this but it's not really what we are trying to test:

$context_repository = $this->container->get('context.repository');
$contexts = $context_repository->getAvailableContexts();
$this->assertArrayHasKey('@user.current_user_context:current_user', $contexts);
$this->assertFalse($contexts['@user.current_user_context:current_user']->hasContextValue());

PS: Very interesting to see that such a simple URL causes the issue as anonymous. My case in #42 was hitting the same issue but from \Drupal\Core\Routing\AccessAwareRouter::match

Cross-posting #3357354-16: The 'entity:user' context is required and not present → here.
The patch solves my issue on 2 projects caused by image styles as explained there. Few, what an absolute horror to debug. Thank you so much!

@Berdir from #12: Could the introduction of image lazy loading in core be directly related to the emergence of this issue?

Edit: I will reroll the patch for 10.1.x

I have now stumbled on this for 2 projects in behat tests.
FWIW in my case it's not the missing anonymous user issue (it's there) 🐛 The 'entity:user' context is required and not present Closed: duplicate
Here is my stacktrace:

Context.php:72, Drupal\Core\Plugin\Context\Context->getContextValue()
EntityConverter.php:149, Drupal\Core\ParamConverter\EntityConverter->convert()
ParamConverterManager.php:100, Drupal\Core\ParamConverter\ParamConverterManager->convert()
ParamConversionEnhancer.php:45, Drupal\Core\Routing\Enhancer\ParamConversionEnhancer->enhance()
Router.php:270, Drupal\Core\Routing\Router->applyRouteEnhancers()
Router.php:150, Drupal\Core\Routing\Router->matchRequest()
AccessAwareRouter.php:90, Drupal\Core\Routing\AccessAwareRouter->matchRequest()
AccessAwareRouter.php:144, Drupal\Core\Routing\AccessAwareRouter->match()
LanguageNegotiationUserAdmin.php:138, Drupal\user\Plugin\LanguageNegotiation\LanguageNegotiationUserAdmin->isAdminPath()
LanguageNegotiationUserAdmin.php:104, Drupal\user\Plugin\LanguageNegotiation\LanguageNegotiationUserAdmin->getLangcode()
LanguageNegotiator.php:197, Drupal\language\LanguageNegotiator->negotiateLanguage()
LanguageNegotiator.php:137, Drupal\language\LanguageNegotiator->initializeType()
ConfigurableLanguageManager.php:218, Drupal\language\ConfigurableLanguageManager->getCurrentLanguage()
LanguageRequestSubscriber.php:92, Drupal\language\EventSubscriber\LanguageRequestSubscriber->setLanguageOverrides()
LanguageRequestSubscriber.php:74, Drupal\language\EventSubscriber\LanguageRequestSubscriber->onKernelRequestLanguage()
ContainerAwareEventDispatcher.php:111, call_user_func:{/home/herve/Documents/commission/ecdc/ecdc-monorepo/web/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php:111}()
ContainerAwareEventDispatcher.php:111, Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch()
HttpKernel.php:142, Symfony\Component\HttpKernel\HttpKernel->handleRaw()
HttpKernel.php:74, Symfony\Component\HttpKernel\HttpKernel->handle()
Session.php:58, Drupal\Core\StackMiddleware\Session->handle()
KernelPreHandle.php:48, Drupal\Core\StackMiddleware\KernelPreHandle->handle()
ReverseProxyMiddleware.php:48, Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle()
NegotiationMiddleware.php:51, Drupal\Core\StackMiddleware\NegotiationMiddleware->handle()
StackedHttpKernel.php:51, Drupal\Core\StackMiddleware\StackedHttpKernel->handle()
DrupalKernel.php:704, Drupal\Core\DrupalKernel->handle()
index.php:19, {main}()

This issue seems to happen somewhat randomly for me.
After some investigation, this is happening exclusively on image styles routes.
I believe the random factor is partially due to the fact that I use selenium, and the browser caches images (so disabling selenium's browser cache helps), but also a race condition as follows: behat ends the scenario, deletes the user, and at the same time, the homepage gets refreshed and lazy loads the images, creating new requests.
Most probably the user still exists at the beginning of these image styles requests, but behat (still running in the background) deletes the user and when the image style requests arrives in \Drupal\user\ContextProvider\CurrentUserContext::getRuntimeContexts and attempts to load the user, it returns NULL (this I confirmed), causing in turn The 'entity:user' context is required and not present exception to be thrown from \Drupal\Core\Plugin\Context\Context::getContextValue

I will see if 🐛 User context missing when using toUrl in some circumstances Needs work helps solving the issue.
PS: I know this issue is closed but perhaps this info could be useful to someone else, who knows.

#32 indeed a new untranslatable boolean field with previously existing translations is enough to cause this.
Upon saving one of the translations we get the error message.
The EntityUntranslatableFieldsConstraintValidator can trigger for other reasons but this here is one of them and can happen with just nodes.

Here are my steps:
- Install standard drupal profile
- At /admin/modules: Enable the content translation module
- At /admin/config/regional/language: add a language
- At /admin/config/regional/content-language: enable translation for Content > Article + check "Hide non translatable fields on translation forms"
- At /node/add/article: add an article node (just the title is enough) + create a translation
- At /admin/structure/types/manage/article/fields/add-field: create a new boolean field (leave it as untranslatable)
- Go back to the node translation edit form, update the title and attempt to save > we get "Non-translatable fields can only be changed when updating the original language."

EntityUntranslatableFieldsConstraintValidator will return TRUE for hasUntranslatableFieldsChanges because the boolean field items went from nothing to something (a BooleanItem with [value => FALSE]. It will then trigger the error if hasTranslationChanges returns TRUE, which it will if changes are detected on any translatable fields (e.g: title).
Breakpoints here and there help.

Patch #79 works for me, thanks.

I'm facing the same issue.
I believe this especially shows on Drupal 10.1 as it now passes the page parameter directly (possibly from 📌 Allow AJAX to use GET requests Fixed ?)

I would like to understand why that code in facets is there and find a proper fix but I cannot even get ajax facets to work at all...
The only ajax-related test in \Drupal\Tests\facets\FunctionalJavascript\AjaxBehaviorTest doesn't seem to actually use ajax...
Things do not make any sense to me TBH...
PS: I created 🐛 How to make AJAX work with facets 3? (issue with isRenderedInCurrentRequest) Active

Fixing tests...

Oh snap, I just found about this issue, after creating 🐛 Fix Font Awesome markup and CSS Needs review
I proposed the same there but using FA's sr-only class and with an aria-hidden on the icon.
There is also a CSS issue with FontAwesome (no spacing).

Here is the patch with both CSS and markup fixes.
AXE has no issues with that: https://jsfiddle.net/s23w8q4j/

Fixing PHPStan error from #92.

Reroll of #86 for current 10.1.x (interdiff empty).
I'm re-adding the patch for optional_end_date module as well, for more visibility.

Fixing PHPStan issues.

Reroll for 10.1.x and adapted the unit tests.

Functional tests are still needed but it would be great to get some feedback on the whole issue/approach before moving forward with that.
As-is, the access check happens in a similar way to EntityReferenceFormatterBase but I wonder if this access check should even happen at this level, and in this way (or perhaps using a #pre_render or #access_callback)...
This could extend way beyond link formatters.

Reroll of #17 for current 10.1.x

The process layer would have been a good candidate to achieve this but it was removed in D8 #1843650: Remove the process layer (hook_process and hook_process_HOOK) →
So I have no idea what we could do here. Pretty sure we don't want to reinstate it...

In terms of scenarios: region (content var), field (items var), layout (content var) and paragraph (content var) are some examples of theme hooks where those variables can contain a complex render array which turns out to be completely empty after rendering.
By complex render array I mean not just #cache and #access but nested render arrays.

Hi donquixote, thanks for jumping in :)

Indeed, we are currently adding our preprocess last using hook_theme_registry_alter() and I remember we also had to use hook_module_implements_alter() to place our theme_registry_alter in last position because of the field_group module also implementing it. What a mess ^^
So yes, that would make total sense. Perhaps a postprocess hook ?

Also, perhaps we need a convincing test case to get a better grip on what we are trying to accomplish.

True, I will try to write some scenarios tomorrow.

+++ b/core/themes/engines/twig/twig.engine
@@ -48,14 +42,27 @@ function twig_render_template($template_file, array $variables) {
+  if (empty(trim($rendered_markup))) {

Duh, empty() is the wrong condition... let's try trim($rendered_markup) === ''

I found out about 🌱 [meta] Themes improperly check renderable arrays when determining visibility Needs work and I'm trying to wrap my head around it.
FWIW here are 2 patches as mentioned in the description.
With this approach preprocess hooks are responsible to bubble up cache metadata.
Are there any downsides to doing this?