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

Feature Request: Option to Treat noscript Tags as Script Enabled #3178

Closed
taisehub opened this issue Apr 19, 2024 · 4 comments · Fixed by #3231
Closed

Feature Request: Option to Treat noscript Tags as Script Enabled #3178

taisehub opened this issue Apr 19, 2024 · 4 comments · Fixed by #3231
Labels
Milestone

Comments

@taisehub
Copy link

Nokogiri parses <noscript> tags as script disabled mode.
Could you consider adding a option in the parsing settings to handle <noscript> tags as script is enabled mode?
This could allow for more flexible parsing depending on the user's needs.

Ref: https://html.spec.whatwg.org/multipage/scripting.html#the-noscript-element

@flavorjones
Copy link
Member

@taisehub Thanks for opening this issue. We'll look into it!

@flavorjones
Copy link
Member

@stevecheckoway Any thoughts on whether this is a good idea and/or how challenging it would be to support script mode?

@stevecheckoway
Copy link
Contributor

@flavorjones I think this should be easy to do. The parsing change itself should be completely straight forward. I think there are three places it matters: in head, in body, and fragment parsing with a noscript element as the context.

Adding a new option makes me think we should consider cleaning up the parse and fragment functions so as to not keep adding additional arguments. That said, these functions are purely internal so maybe that doesn't matter so much. Do you have an opinion on that?

@flavorjones
Copy link
Member

flavorjones commented May 21, 2024

@stevecheckoway I've got a draft branch that cleans up those methods to use keyword arguments, I should be able to clean it up and create a PR in the morning (along with a few other smaller pieces of HTML5 fragment cleanup I also had on a branch).

Edit: that PR is #3199 and it does not include additional cleanup.

flavorjones added a commit that referenced this issue May 24, 2024
)

**What problem is this PR intended to solve?**

Refactor `Gumbo.parse` and `Gumbo.fragment` to use keyword arguments for
parse options (instead of positional arguments).

These methods are internal and not part of the public API, so this isn't
considered a breaking change. We're changing them to be more extensible
going forward.

See comments at #3178 for context

This PR also

- introduces checking for the Gumbo options passed in
- takes care to delete deprecated kwarg `max_parse_errors`
- cleans up options passed around in tests


**Have you included adequate test coverage?**

It's a refactor, existing test coverage should be sufficient.


**Does this change affect the behavior of either the C or the Java
implementations?**

No behavior changes, but the HTML5 parser is only available in CRuby.
@flavorjones flavorjones changed the title Feature Request: Option to Treat <noscript> Tags as Script Enabled Feature Request: Option to Treat noscript Tags as Script Enabled Jun 20, 2024
@flavorjones flavorjones added this to the v1.17.0 milestone Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants