Library information alter should not be context-aware

Closing as no progress has been made and no further reports have been made.

If this is still a problem feel free to reopen it.

I made a mini module to demonstrate the issue. Find modal_test.zip attached.

I've tested this in a clean Drupal 10.1.3 install, using 'drush site:install' + enable gin and gin_toolbar for admin theme. No other changes than adding the 'modal_test' module. Of course caching is enabled.

After install of 'modal_test', as anonymous user, browse to page '/modal-test/page-for-modal' and click 'Open modal'. This should show:

Next, as admin the same page and modal should show as

Both cases are as expected, according to #3308200-7: Why do I see dialog.css in my theme? → .

Now, clear the caches and view the page and modal again as admin. This should show the same as the previous images.
Now view again as anonymous, this should show the broken theming:

In the browser inspector you can see gin theming is applied and missing CSS variables:

Clear the cache, refresh the page as anonymous and see the modal again as shown in the first image.

About solving this, removing

if ($extension == 'core' && isset($libraries['drupal.dialog'])) {
    $libraries['drupal.dialog']['dependencies'][] = 'claro/claro.drupal.dialog';
    $libraries['drupal.dialog']['dependencies'][] = 'gin/dialog';
 }

from gin_toolbar_library_info_alter() solves the problem for anonymous, proving that adding the dependencies in that hook is the problem.
Is extending those libraries in that still required? Removing it entirely, refreshing the page as admin, the modal still shows in Gin theming.

Since having the Gin dialog style bleed into the frontend theme is a design choice, as far as I can tell right now, making sure the Gin CSS variables are available in the frontend fixes the problem of the broken dialogs when the cache is 'wrong'.

For the real problem, that the additions in gin_toolbar_library_info_alter are not context aware, I have no suggestion at this time.
Depending on the choices made for Gin 4.x, the problem might become moot.

So, for now, see the MR for work suggested workaround + additions in the README to point out the design choice and how to 'opt-out';

Patch attached, corresponding with the MR at moment of writing.

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

A combination of

libraries-extend:
  core/drupal.dialog:
    - claro/claro.drupal.dialog
    - gin/dialog

to force the dialog CSS to be loaded independent of how the cache was filled and the patch from #10 to force the CSS variables to be loaded provides an acceptable workaround for this issue.

For the long term solution I'd prefer a proper opt-in or opt-out for using the Gin dialog styling for both users with and without permission to see the Gin toolbar. In my opinion the styling should always be the same in both cases for consistency. I think I have a slight preference for an opt-in as it feels bit weird that enabling a toolbar module has an effect on dialog styling. On the other hand I think the Gin toolbar styling is much prettier without any doubt. :-)

Hi

Using the gin-toolbar module (8.x-1.x), I have produced this (popup) issue with Drupal versions 10 and 11. However, the issue has been resolved after applying MR #11. I have included both the before and after the video.
Before:- https://www.awesomescreenshot.com/video/32055676?key=9463acd25968bdd3506...
After:- https://www.awesomescreenshot.com/video/32055728?key=f41ee0d7125da876ff5...
Thanks

Note to self, here is a snippet from @webflo on how he addressed this before: https://gist.github.com/webflo/67a380250486f8ab2ccacc73efe21406

This issue is really annoying for websites using dialogs on front-end because gin_toolbar does not take permissions like view the administration theme into account when loading its CSS. The result is broken dialogs for non-admin users.
The MR suggests a workaround to disable all gin dialog-related styles on frontend but it kind of defeats the purpose of the module for admin users. Would it be possible to implement something like @webflo's suggestion?

I agree with @duaelfr, something should be done as this breaks frontend of non-admin users which can be problematic

I have re-rolled the merge request. The merge request should be switched to target 2.0.x.

I landed here also having issues, in my case there ajax.css was being leaked to anonymous users on the frontend.

I think the should be that the library information alter should be context-aware, which is what weblo's approach on https://www.drupal.org/project/gin_toolbar/issues/3293209#comment-15892466 🐛 Library information alter should not be context-aware Closed: outdated does. @jurgenhaas I am curious on how you got to this gist. Excellent! I used this approach and works perfectly and it's a great TIL.

I can see why this cannot be added as an MR to this module, as the context can really be anything particular to the use case of the user, and having a module that alters a core's services is not the best approach.

I went ahead and added ✨ Make library discovery context aware Active as I think this can be a good addition to core.

Funny enough, I was just made aware of the fact that I am the original reporter of this issue, go figure! Re-reading what I wrote 3 years ago allowed me to remember that what happened to be back then was the other side of the issue. Anonymous traffic was caching the library discovery, so gin_toolbar CSSs were not added, failing to work for authenticated users.

Either way, webflo's approach sorts out both things and maybe the core issue get some feedback or solution.

And now I also understand my original title, changing yet once more to something better.