Add an options parameter to all the methods that allow creation of an HTML document fragment

[JackMc]

We are facing an issue where we cannot provide a DocumentFragment with the XML::ParseOptions::HUGE option because it doesn't take the options int like the regular Document class does. This adds an optional parameter for passing that down.

Let me know if there need to be extra tests to ensure this is working :-)

[flavorjones]

There are failing tests here related to the jruby implementation that will need to be addressed.

I would like test coverage for this, yes, to ensure we're exercising this code path. Not sure which parse option to test with; maybe NOENT or NOERROR would be good candidates?

[JackMc]

Thanks for the prompt response @flavorjones! I ran the tests locally, but I guess it only runs the plain Ruby implementation. Was just writing the Ruby test now. I'll look at the Java tests too :)

[JackMc]

So after looking at this a bit, it turns out that there's a difference in the behaviour of the Java and C implementations. When instantiating a XmlDocumentFragment, the C implementation yields the provided block with an argument of itself (see https://github.com/JackMc/nokogiri/blob/058f5700fbff4b264b4727da484bfc419db1eb32/ext/nokogiri/xml_document_fragment.c#L28), whereas the Java implementation does not. This is causing trouble when using MRI and

This is not documented or tested and is used internally in one place as far as we can tell (d18f1ac#diff-8b98857b6a2cb5117b8b1de836357ecdR59). I'd argue that it would be best to rewrite this one usage so the block parameter can be freed up for this purpose (bringing it in line with XmlDocument's functionality) and so we can bring the Java and C implementations to functional equivalence.

@flavorjones what do you think?

[JackMc]

Actually, it looks like that previous use is only for Node, so it doesn't seem to be used anywhere.

[JackMc]

Bump :) This is mostly done I think. The tests that are failing here match the tests that are failing on master.

[JackMc]

Bump :) I've rebased upon master now so this should be easier to review

[JackMc]

Bump :)

[flavorjones]

Sorry for the terrible response time on this. I'd like to get this into v1.9.0, and so I'm going to rebase these commits and submit a separate PR to make sure they pass CI. I'll continue any conversation that needs to happen here, but will link to the other branch for visibility.

[flavorjones]

Rebase went green at #1843.

I noticed the test added for this, test_passed_options_have_an_effect only seems to test that the call accepts a block. I'm going to increase test coverage to ensure options set either by argument or in block take effect.

[flavorjones]

Ok - so this changeset doesn't actually work in the JRuby implementation, and so it's not going to make it into v1.9.0.

It's also becoming apparent, as I added test coverage, that the JRuby implementation has some other pre-existing warts that I need to fix. Here's how I'd like to approach:

1. [net new behavior] have all XML::Document and HTML::Document underlying parse methods (e.g., #read_io, #read_memory) save the ParseOptions as a Document attribute, to make testing a bit easier
2. [partially implemented by this PR] have all XML::DocumentFragment and HTML::DocumentFragment parse methods take options and an options-configuration block
3. [fixes] ensure the JRuby impl matches the CRuby behaviors

I'm going to open a new issue, linking to this one, to track all the additional work. I'll make sure your commit appears in master so you get contributor credit.

Changing target milestone to v1.10.0

[flavorjones]

See #1844 for progress on this work.

[flavorjones]

Please note that the latest CI failure is a problem with the new PR pipeline and not an issue with this PR, which previously was green. Still targetting for v1.11.0.

[flavorjones]

See #2399 for an updated version of this branch.

[flavorjones]

Will ship in v1.13.0 ... thanks for your patience!