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

ignoring noscript #446

Closed
echolimazulu opened this issue Jun 27, 2022 · 9 comments
Closed

ignoring noscript #446

echolimazulu opened this issue Jun 27, 2022 · 9 comments

Comments

@echolimazulu
Copy link
Contributor

echolimazulu commented Jun 27, 2022

Bug report

Actual Behavior

Expected Behavior

How Do We Reproduce?

Please paste the results of npx webpack-cli info here, and mention other relevant information


Hello html-loader developers,

Today I noticed that the content of supported tags wrapped in noscript is completely ignored and not available for creating html-loader imports to pass them to the next loader.

In my configuration before the html-loader, I also use posthtml-loader and the HTML elements inside noscript are available to it.

Please clarify if this loader behavior is expected and where to dig to fix or change this behavior.

Searching the repository turned up no results on this issue.

<noscript>
    <figure>
        <img src="/images/picture.jpg" alt="">
    </figure>
</noscript>

html-loader version is 4.0.0

@echolimazulu
Copy link
Contributor Author

@alexander-akait Hello! I would be glad to know your vision on this issue. Thank you!

@alexander-akait
Copy link
Member

I think this should be help https://parse5.js.org/interfaces/parse5.ParserOptions.html#scriptingEnabled (and I think we should add this option)

@echolimazulu
Copy link
Contributor Author

echolimazulu commented Jun 27, 2022

Hello Alexander,

Thanks for your answer and thanks for the solution you suggested.

I also thought about this earlier, but unfortunately it did not help.

I mean the following changes that I made manually (sources-plugin.js):

const document = (0, _parse.parse)(html, {
    sourceCodeLocationInfo: true,
    scriptingEnabled: true
});

By debugging the executable code of html-loader. I see that parse5 is passing the contents of the noscript tag as #text to the html-loader with scriptingEnabled: true.

But unfortunately this does not affect the processing of html content inside noscript.

@alexander-akait
Copy link
Member

hm, strange, looks like parse5 doesn't out AST (example https://astexplorer.net/#/gist/94acd53dbf527e8f8f735b5a28053ab0/3ba4cb0c2a734d61e0b868b6c3b879d509149472), that is why we don't handle it, when scriptingEnabled is false (not true) it should output contents of noscript as AST, not as text

@echolimazulu
Copy link
Contributor Author

echolimazulu commented Jun 27, 2022

Ahh, now I understand. Thanks for the explanation about AST and text, it helped a lot.

When set scriptingEnabled: false the noscript content is processed successfully.

@alexander-akait
Copy link
Member

@echolimazulu I think we should add this option to our loader (so feel free to send a PR), and maybe set to true by default for the next major release

@echolimazulu
Copy link
Contributor Author

echolimazulu commented Jul 5, 2022

@alexander-akait Good evening, Alexander,

I apologize for such a long response on my part. I proposed a solution in a PR: #448

Unfortunately, I don't want to sign the Linux Foundation's CLA for signing the PR, besides, they fills out the form incorrectly with the specified e-mail, which I also reported to them.

Happy to help resolve this issue. If CLA is required and if my PR cannot be accepted without it, then you can pull this commit from the master branch of my repository.

Thank you very much!

@echolimazulu
Copy link
Contributor Author

@alexander-akait Good morning, Alexander,

With CLA, the issue is resolved, PR is ready for consideration.

Glad to interact with you.

@alexander-akait
Copy link
Member

Fixed #448, thank you again

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

No branches or pull requests

2 participants