Problem/Motivation
I create some SDC components, when using theme, the js and CSS file not load correctly.
I got error in chrome console. this is the url generated by SDC:
/yiysites/core/modules/sdc/../modules%5Ccontrib%5Cblocktabs_jquery_ui%5Ccomponents%5Ctabs-default%5Ctabs-default.js?rwh76n
it should be:
/yiysites/modules/contrib/blocktabs_jquery_ui/components/tabs-default/tabs-default.js?rwh76n
Steps to reproduce
1, using a win10 OS.
2, install xampp, XAMPP is the most popular PHP development environment, https://www.apachefriends.org/index.html
3, create a site in sub directory of htdocs
4, install SDC module
5, create a component in custom module.
Proposed resolution
Windows using a different file path compare linux, it use "\\" instead of "/". we need add some logic code for windows condition.
here is some dirty code that I fixed it quickly in makePathRelativeToLibraryRoot of \Drupal\sdc\ComponentPluginManager :
$path_from_root=str_replace('\\','/',$path_from_root);
//return $dots . $path_from_root;
$path_from_root= "/" . $path_from_root;
return $path_from_root;
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #25 | drupal-windows_sdc-3367556-25.patch | 720 bytes | danchadwick |
| #22 | Screenshot 2024-10-02 090531.png | 13.41 KB | newaytech |
| #6 | 3367556.component-on-windows.patch | 703 bytes | detroz |
Issue fork drupal-3367556
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3367556-sdc-components-css
changes, plain diff MR !9645
Comments
Comment #2
g089h515r806 commentedComment #3
dieterholvoet commentedComment #5
laborouge commented+1 Subscribe
Same problem on Drupal 10.3.0
Comment #6
detroz commentedSame 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.
Comment #7
newaytech commentedSame issue here - manifested once I updated Bario theme - with new components added.
Path in #6 applied and fixed for me - thanks!
Comment #12
vensiresI created MR!9645 and added a if {} for Windows machines specifically - same as core does in other places too.
Comment #13
smustgrave commentedprobably should have coverage
Comment #14
groendijk commented@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 forstr_replace(DIRECTORY_SEPARATOR, '\\'they don't do the OS check. Is it necessary?Comment #15
vensiresYou are correct but here we are doing the opposite:
$path_from_root = str_replace('\\', DIRECTORY_SEPARATOR, $path_from_root);so I think it is.
Comment #16
groendijk commentedSounds valid, can you confirm its working?
Comment #17
vensiresUnfortunately 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.
Comment #18
newaytech commentedHi 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)
Comment #19
vensiresThanks 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 diffand 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 crotherwise I have the same issues. I don't really think it's related to this one.Comment #20
newaytech commentedThe merge request uses a backslash "DIRECRTORY_SEPARATOR" - which renders the wrong file path to the href link.
Replacing with "/" fixes it up.
Are you able to re-roll as another merge request ?
Comment #21
vensiresIt's ready @newaytech!
Comment #22
newaytech commentedyes - looks good that.
Comment #23
vensiresPerfect, 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.
Comment #24
marc.bauPatch does not work properly. I still see
/../in paths.Comment #25
danchadwick commentedIf 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.
Comment #26
danchadwick commentedChanging 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.
Comment #27
fotispanpatch #25 fixed the issue for me on a windows machine.
Thanks
Comment #28
vensiresI changed the MR to match what's already proposed in #25 along with a small comment why this line of code is needed. I also I set it as RTBC based on #27, since it's decided we won't need tests.
Merry Christmas to everyone :D
Comment #29
jaroslav červený commentedI doing my first SCD commponent in custom theme on WAMP sever.
Aftter apply patch #25, I get in source code:
It works.
Comment #33
catchCommitted/pushed to 11.x, 11.1.x and 10.5.x, thanks!