Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

liquid script tag does not render custom attributes #16348

Closed
bashuss opened this issue Jun 19, 2024 · 19 comments
Closed

liquid script tag does not render custom attributes #16348

bashuss opened this issue Jun 19, 2024 · 19 comments
Labels

Comments

@bashuss
Copy link

bashuss commented Jun 19, 2024

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:
#16348 (comment)

Expected behavior

<script src="/t4h/OrchardCore.Resources/Scripts/jquery.min.js"></script>
<script src="/somelocalscript.js" type="module", defer="defer"></script>
@mdameer
Copy link
Contributor

mdameer commented Jun 19, 2024

@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
Copy link
Author

bashuss commented Jun 19, 2024

@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
Copy link
Member

Piedone commented Jun 19, 2024

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
Copy link
Author

bashuss commented Jun 20, 2024

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

@bashuss bashuss closed this as completed Jun 20, 2024
@bashuss
Copy link
Author

bashuss commented Jun 20, 2024

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

@bashuss bashuss reopened this Jun 20, 2024
@hishamco
Copy link
Member

Why this should be open if the issue not exist?

@Piedone
Copy link
Member

Piedone commented Jun 20, 2024

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

@bashuss
Copy link
Author

bashuss commented Jun 20, 2024

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 bashuss closed this as completed Jun 20, 2024
@bashuss
Copy link
Author

bashuss commented Jun 20, 2024

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
Copy link
Member

Piedone commented Jun 20, 2024

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
Copy link
Author

bashuss commented Jun 21, 2024

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: #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
Copy link
Member

Piedone commented Jun 21, 2024

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
Copy link
Author

bashuss commented Jun 21, 2024

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
Copy link
Author

bashuss commented Jun 21, 2024

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
Copy link
Member

Piedone commented Jun 21, 2024

From this I can't help, sorry.

@bashuss bashuss reopened this Jun 21, 2024
@bashuss
Copy link
Author

bashuss commented Jun 21, 2024

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
Copy link
Member

Piedone commented Jun 22, 2024

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
Copy link
Author

bashuss commented Jun 22, 2024

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants