SDC components CSS & JS generated wrong url in windows / XAMPP

+1 Subscribe

Same problem with Laragon on Windows and Drupal 10.3.1

Here a patch tested on Drupal 10.3.1 and based on the proposed resolution by g089h515r806.

Same issue here - manifested once I updated Bario theme - with new components added.

Path in #6 applied and fixed for me - thanks!

vensires → changed the visibility of the branch 3367556-sdc-components-css to hidden.

vensires → changed the visibility of the branch 3367556-sdc-components-css to active.

I created a MR and added a if {} for Windows machines specifically - same as core does does in other places too.

probably should have coverage

@vensires Just for understanding, looking at the if (!str_starts_with(PHP_OS, 'WIN')) in core. It's always connected with a directory/file permission command. If you search for str_replace(DIRECTORY_SEPARATOR, '\\' they don't do the OS check. Is it necessary?

You are correct but here we are doing the opposite:
path_from_root = str_replace('\\', DIRECTORY_SEPARATOR, $path_from_root); so I think it is.

Sounds valid, can you confirm its working?

Unfortunately no. We will need someone with Windows to be able to review it; maybe one of the previous commenters in this thread.
For now, we have the "Needs tests" tag so we are good; I don't have any experience with unit testing though and I don't know how unit testing in Linux OS could check for this bug which is targeting Windows only.

Hi folks. I use a XAMP stack on Windows 11. Can confirm patch in #6 applied and fixed for me - do you need me to test any other patches? How do I apply the merge request to my setup - I'm using the patch via composer.json right now.

I do get an error message when restoring a production site (linux based) to my local dev - so I need to perform a drush full cache clear - otherwise I can't access the site. Not a showstopper - but just something I've never had to do before - and seems related.

Warning: file_get_contents(/etc/apache2/htdocs/drupal-8/neway-drupal-project/web/themes/contrib/bootstrap_barrio/components/alert\alert.twig): Failed to open stream: No such file or directory in Drupal\Core\Template\Loader\ComponentLoader->getSourceContext() (line 93 of core\lib\Drupal\Core\Template\Loader\ComponentLoader.php).
Drupal\Core\Template\Loader\ComponentLoader->getSourceContext('bootstrap_barrio:alert') (Line: 71)
Twig\Loader\ChainLoader->getSourceContext('bootstrap_barrio:alert') (Line: 380)
Twig\Environment->loadTemplate('__TwigTemplate_5fa659091c48341a12c1ba2127d9481d', 'bootstrap_barrio:alert') (Line: 343)
Twig\Environment->load('bootstrap_barrio:alert') (Line: 466)
Twig\Environment->resolveTemplate('bootstrap_barrio:alert') (Line: 1439)
Twig\Extension\CoreExtension::include(Object, Array, 'bootstrap_barrio:alert', Array) (Line: 45)
__TwigTemplate_bfe96b157a4c1d7d5465c2e2e9c307c6->doDisplay(Array, Array) (Line: 393)
Twig\Template->yield(Array, Array) (Line: 349)
Twig\Template->display(Array) (Line: 364)
Twig\Template->render(Array) (Line: 35)
Twig\TemplateWrapper->render(Array) (Line: 33)
twig_render_template('themes/contrib/bootstrap_barrio/templates/misc/status-messages.html.twig', Array) (Line: 348)
Drupal\Core\Theme\ThemeManager->render('status_messages', Array) (Line: 491)
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: 228)
Drupal\Core\Render\Renderer->renderPlaceholder('callback=Drupal%5CCore%5CRender%5CElement%5CStatusMessages%3A%3ArenderMessages&args%5B0%5D&token=_HAdUpwWmet0TOTe2PSiJuMntExoshbm1kh2wQzzzAA', Array) (Line: 697)
Drupal\big_pipe\Render\BigPipe->renderPlaceholder('callback=Drupal%5CCore%5CRender%5CElement%5CStatusMessages%3A%3ArenderMessages&args%5B0%5D&token=_HAdUpwWmet0TOTe2PSiJuMntExoshbm1kh2wQzzzAA', Array) (Line: 524)
Drupal\big_pipe\Render\BigPipe->Drupal\big_pipe\Render\{closure}()
Fiber->start() (Line: 531)
Drupal\big_pipe\Render\BigPipe->sendPlaceholders(Array, Array, Object) (Line: 283)
Drupal\big_pipe\Render\BigPipe->sendContent(Object) (Line: 113)
Drupal\big_pipe\Render\BigPipeResponse->sendContent() (Line: 423)
Symfony\Component\HttpFoundation\Response->send() (Line: 20)

Thanks for volunteering @newaytech!
You need to test the diff from the MR: https://git.drupalcode.org/project/drupal/-/merge_requests/9645.diff. Use that URL instead of the patch's. As for how you may find this URL in other issues too, just search in this page for plain diff and you'll find it.

As for the restoration process, this is something I do quite often. Especially when I copy the database from a system using another URL (ex. Beta) to my own local system (ex. DDEV); I have to execute drush cr otherwise I have the same issues. I don't really think it's related to this one.

The merge request uses a backslash "DIRECRTORY_SEPARATOR" - which renders the wrong file path to the href link.

Replacing with "/" fixes it up.

    -  $path_from_root = str_replace('\\', DIRECTORY_SEPARATOR, $path_from_root);
    + $path_from_root = str_replace('\\', "/", $path_from_root);

Are you able to re-roll as another merge request ?

It's ready @newaytech!

yes - looks good that.

Perfect, so the major thing to decide here is what kind of tests should we expect for this small change targeting Windows-only machines...
I add the tag "Needs Review Queue Initiative" in order to get some further guidance.

Patch does not work properly. I still see /../ in paths.

<link rel="stylesheet" media="all" href="/web/core/../themes/contrib/bootstrap/components/menu_columns/menu_columns.css?skxqv7" />
<link rel="stylesheet" media="all" href="/web/core/../themes/contrib/bootstrap/components/menu_main/menu_main.css?skxqv7" />

If we use DIRECTORY_SEPARATOR properly, then the code works equally well for all platforms. The previous commit had the arguments reversed. It simply normalized the separator to the expected drupal slash. Then we don't need to test for windows.

I also submit no additional tests are needed. If existing tests pass, then we haven't harmed Linux. And I can confirm that I've manually tested on Windows (WampServer, Windows 11), both by stepping through the code and by seeing it work correctly.

Patch was made on 10.3, but I expect it would apply to 11.x.

Changing to Major, since the site is visually unusable for all users. Might warrant Critical.

Changing to needs review since I don't believe tests are needed. I did not make a commit to the MR without approval of the other authors.