search

senny / sablon

Ruby Document Template Processor based on docx templates and Mail Merge fields.
MIT License
447 stars 128 forks source link

Improve jruby support of ComplexFields #185

Open scottmtp opened 2 years ago

scottmtp commented 2 years ago

Due to underlying differences in the jruby and MRI xml libraries in nokogiri, sablon doesn't currently work well in jruby. This PR improves the situation by fixing the ComplexField class in mail_merge.rb.

Currently the ComplexField class contains xpath searches that use namespace prefixed attributes in the search expression, which appears to not work in all cases with jruby/nokogiri. Using local-name and ignoring the attribute namespace prefix appears to work well for both jruby and MRI.

With this change, many of the test documents in 'test/sandbox/' appear to be generated correctly, even though tests continue to fail due to minor differences in xml serialization.

stadelmanma commented 2 years ago

Hi @scottmtp this is an interesting fix, so basically the change ignores the particular namespace and something like “z:fldChar” would also match? I don’t know how much overlap there is on element names between namespaces but I’m not a huge fan of matching more than we should because it allows sneaky edge cases into the code that would be very very difficult to debug due to the complexity and opaqueness of the docx file type.

Maybe we can verify the namespace separately from the xpath query?

You also mentioned tests failing, can you check if the tests pass when run with regular Ruby?

I’m still not certain we can ever say jruby is 100% supported but I’m not at all opposed to improving support where possible.

scottmtp commented 2 years ago

This change only relaxes the namespace check on attributes, not element names, so <z:fldChar w:fldCharType="end"/> would not match, but <w:fldChar z:fldCharType="end"/> would.

The test suite passes when run with cruby.