flavorjones
commented
6 months ago @marcoroth I spent some time on this today, and I have a potential workaround for you that I'd like your feedback on, and if it works for Stimulus Reflex then I'll put some work in to make it easier to use.
first some questions about the use case
The "fragment" above is not a fragment -- it's a complete document. If possible, in these cases, it's better to parse it with Document.parse than DocumentFragment.parse.
I'd like to understand the use case of Stimulus Reflex a bit better to understand if there's a better API that Nokogiri could be offering here. Do you really not know if the content is a document or a fragment?
transform the code into an equivalent
First let me convert the code above to use DocumentFragment.new instead of .parse:
#!/usr/bin/env ruby
require "bundler/inline"
gemfile do
source "https://rubygems.org"
gem "nokogiri", path: "."
end
html = <<~HTML
<html>
<body class="foo">
<div>hello</div>
</body>
</html>
HTML
puts Nokogiri::HTML4::DocumentFragment.new(Nokogiri::HTML4::Document.new, html).to_html
puts "---"
puts Nokogiri::HTML5::DocumentFragment.new(Nokogiri::HTML5::Document.new, html).to_htmlNote that the output is the same:
<body>
<div>hello</div>
</body>
---
<div>hello</div>
diagnosis
By default, both parsers will try to parse the fragment in the context of a body tag.
The HTML4 behavior that we see is due to a hack made a long time ago in c23eb88b: if a context node isn't specified, and there happens to be a line that matches /^\s*<body/i, then we return the tree including the body node.
If you change the script above to format the HTML slightly differently:
html = <<~HTML
<html><body class="foo">
<div>hello</div>
</body>
</html>
HTMLthe output changes for the HTML4 fragment parser:
<div>hello</div>
---
<div>hello</div>
I really don't like this code very much, for a few reasons:
- there's no way to parse
headfragments as part of the head - the "body" tag should be more consistently returned, and IMHO it should never be returned in the situations where we assume a "context node" of
body
but it's been there for 14 years now and I can't really change it without breaking people's code.
We see what we see for the HTML5 fragment parser because it's working exactly as intended: we parse in the context of a body tag, and we return the children of that context node. IMHO this is the correct implementation and I'm hesitant to make it more complicated to try to match the hacky HTML4 implementation.
potential workaround
The HTML5::DocumentFragment.new method supports a "context node" argument, so we can tell the parser more exactly what we want to do. In the above case, because it's a complete document and not a fragment, I think what we really want to do is parse in the context of an html tag, so let's try that:
puts Nokogiri::HTML5::DocumentFragment.new(Nokogiri::HTML5::Document.new, html, "html").to_htmlwill output
<head></head><body class="foo">
<div>hello</div>
</body>A couple of things to call out:
- good: the original body tag including attributes is present
- the head tag is also present, meaning that if the input had head content, it would be here, too:
html = <<~HTML
<html>
<head>
<link rel="stylesheet" href="mystyle.css">
</head>
<body class="foo">
<div>hello</div>
</body>
</html>
HTML
# ...outputs
<body>
<link rel="stylesheet" href="mystyle.css">
<div>hello</div>
</body>
---
<head>
<link rel="stylesheet" href="mystyle.css">
</head>
<body class="foo">
<div>hello</div>
</body>Perhaps naively, the HTML5 parser's output matches the input more exactly than the HTML4 parser's output. But I have no idea whether this is helpful for Stimulus Reflex's use case.
Does this work for you? If not, why not? If so, then I'll do some work to make the context node input available to callers of DocumentFragment.parse.
marcoroth
stevecheckoway
Please describe the bug
emits:
Expected behavior
I would expect:
Additional context
@marcoroth reported at RubyConf! woot