search

OrchardCMS / OrchardCore

Orchard Core is an open-source modular and multi-tenant application framework built with ASP.NET Core, and a content management system (CMS) built on top of that framework.
https://orchardcore.net
BSD 3-Clause "New" or "Revised" License
7.23k stars 2.34k forks source link

liquid script tag does not render custom attributes #16348

Open bashuss opened 2 weeks ago

bashuss commented 2 weeks ago

Describe the bug

liquid template line {% script name:"pdf.js", src:"/Vendor/pdfjs/build/pdf.js", type:"module", defer:"defer", at:"head", depends_on:"jQuery" %}

renderes in <head> as:

<script src="/t4h/OrchardCore.Resources/Scripts/jquery.min.js"></script>
<script src="/Vendor/pdfjs/build/pdf.js"></script>

, so defer and type attributes are missing.

When you look at the code of https://github.com/OrchardCMS/OrchardCore/blob/main/src/OrchardCore.Modules/OrchardCore.Resources/Liquid/ScriptTag.cs it collects the custom attributes. Somewhere on the way to HTML, they get lost.

Orchard Core version

2.0.0-preview-18232

To Reproduce

Steps to reproduce the behavior:

  1. Add this line to a backend template: {% script name:"somelocalscript", src:"/somelocalscript.js", type:"module", defer:"defer", at:"head", depends_on:"jQuery" %}
  2. Call a front end page that uses the template
  3. check source code for the created script tag
  4. Check if custom properties "type" and "defer" are there.

Edit: Not enough to reproduce: Please check this comment: https://github.com/OrchardCMS/OrchardCore/issues/16348#issuecomment-2183259612

Expected behavior

<script src="/t4h/OrchardCore.Resources/Scripts/jquery.min.js"></script>
<script src="/somelocalscript.js" type="module", defer="defer"></script>
mdameer commented 2 weeks ago

@bashuss I can't reproduce this issue with the latest code from main, I think everything is rendered as expected, as you can see in this image:

image

bashuss commented 2 weeks ago

@mdameer What was the exact liquid code you used? Maybe I have a typo... I just tried with the latest nightly build - still no success.

Piedone commented 2 weeks ago

Same here; maybe we fixed it in the two weeks since that preview version. I get this:

<script src="/blog2/OrchardCore.Resources/Scripts/jquery.min.js"></script>
<script defer="defer" src="/Vendor/pdfjs/build/pdf.js" type="module"></script>

With:

{% script name:"pdf.js", src:"/Vendor/pdfjs/build/pdf.js", type:"module", defer:"defer", at:"head", depends_on:"jQuery" %}

I used the Blog recipe, and put this into a template named Widget__Blockquote, created from the admin.

bashuss commented 2 weeks ago

Thanks for testing! I'll wait a while and try again.

bashuss commented 2 weeks ago

As it is ok for you, I'll reopen, if I cannot make it work within the next month. Not in a hurry here...

hishamco commented 2 weeks ago

Why this should be open if the issue not exist?

Piedone commented 2 weeks ago

Yeah, please just confirm, I wouldn't like to keep a dangling issue open.

bashuss commented 2 weeks ago

Is there a quick way to switch between full git checkout and nuget packages? With a fresh git checkout of "main" and a new project it also works on my machine.

I would like a quick way to see, if the nuget to checkout makes the difference, or if I have to search for something specific in my project.

bashuss commented 2 weeks ago

Is there a quick way to switch between full git checkout and nuget packages? With a fresh git checkout of "main" and a new project it also works on my machine.

I would like a quick way to see, if the nuget to checkout makes the difference, or if I have to search for something specific in my project.

Piedone commented 2 weeks ago

It's not quick, but you can configure a local NuGet folder and dotnet pack the latest OC source to it. Otherwise, you can simply reference the latest preview NuGet packages from Cloudsmith.

bashuss commented 2 weeks ago

Not sure if you meant that: I copied the OrchardCore.Cms.Web.csproj file and renamed it to OrchardCore.Cms.Web.Nuget.csproj, replaced the cms.Targets project reference by the preview nuget package a')nd configured a different build path ('/bin-nuget'). This allows to run the same code and AppData with source and with nuget. Unfortunately everything works fine - so there must be something wrong specifically with my project.

The other difference I see between my project and the fresh checkout is, that the required jQuery has a cache busting version appended to the url, which is not present in my project.

<script src="/OrchardCore.Resources/Scripts/jquery.js?v=a4kg606ehDzlqOuGT25zpwTVCNBlY_nLGtlOpOZcMLg"></script> - in fresh checkout <script src="/t4h/OrchardCore.Resources/Scripts/jquery.min.js"></script> - in my project

As there is also the difference between minified and non minified version, I thought, that the reason could be a difference between release and debug build. Trying to test this, I stumbled over this: https://github.com/OrchardCMS/OrchardCore/issues/16358

Can you point me to the code that renders the script tag? I'd like to check, if there are any configurations, which may cause the difference between my project and the clean checkout.

Piedone commented 2 weeks ago

My only guess is that your theme has no reference to either OrchardCore.DisplayManagement or OrchardCore.ResourceManagement. If indeed, check if adding them will help.

You can start from here: https://github.com/OrchardCMS/OrchardCore/blob/main/src/OrchardCore/OrchardCore.ResourceManagement/TagHelpers/ScriptTagHelper.cs

bashuss commented 2 weeks ago

My only guess is that your theme has no reference to either OrchardCore.DisplayManagement or OrchardCore.ResourceManagement. If indeed, check if adding them will help.

I thought, that it might be my theme, so I switched to different front end themes - no success within my project. I guess it must either be an activated feature or some setting or code within my web app.

bashuss commented 2 weeks ago

My theme had used OrchardCore.ResourceManagement.Abstraction. I replaced it, but no change, as expected from the tests with other themes. I tried in another tenant in the same application and it worked.

So it is a tenant specific setting or feature, that creates the problem.

Piedone commented 2 weeks ago

From this I can't help, sorry.

bashuss commented 1 week ago

Found the problem: {% script name:"pdf.js", src:"/Vendor/pdfjs/build/pdf.js", type:"module", defer:"defer", at:"head", depends_on:"jQuery" %}

works fine, as long as no script depends on it. As soon as you have a following

{% scriptblock name:"depending", depends_on:"pdf.js", at:"foot" %}
// some script
{% endscriptblock %}

the extra attributes are not rendered anymore. This also is the only thing that makes sense with the existing code - without have found the exact location of the problem yet.

Piedone commented 1 week ago

I don't think that code block makes sense, since you can't include an external script and add inline code at the same time.

We could throw an exception in this case.

bashuss commented 1 week ago

I don't understand your reasoning. You include one library script like pdf.js and then have a dependent custom script that works with it. This should be rather standard. Sure, I could write the code of the custom script in a .js file and host it with my app, but with the ability to create templates on the frontend, you may like to do something like this.

Also adding an inline script within a template allows the script to have code variations depending on the Model of the shape and the custom settings. In my case this provides an easy way to handle the pdf.worker.mjs url for pdfjsLib, which is initialized by passing the path to the lib and not by adding another script to the ResourceManagement. So when you have tenants with prefixes and some without, you can use liquid to always have the correct path.

SzymonSel commented 1 week ago

There's a missing clause in the ScriptTag.cs in the case when !string.IsNullOrEmpty(name) && string.IsNullOrEmpty(src).