Asciidoctor 1.5.6 introduced a rendering regression for data-background new-style images

[obilodeau]

The update of asciidoctor to 1.5.6 (in #129), introduced a regression. The test data-background-newstyle.adoc no longer renders properly with Asciidoctor 1.5.6. Rolling back to 1.5.4 fixed it.

Found while implementing doctest support in #116.

Plan:

I'm going to fix master, pin asciidoctor to 1.5.4 and do a release (w/o doctest or #131). This way people pulling 1.0.3 or current latest will have it working. As will people allowing bugfix updates only.

Then I'll fix this bug (hopefully fixable locally), increase dependencies to 1.5.6, merge #116, #131 and do a 1.1.0 release.

@mojavelinux, thoughts?

[mojavelinux]

Can you describe what the regression is?

[obilodeau]

• doctest output changes: 94ca263#diff-a1ebfbddb67e07d43a0ab710c95b2d33
• regression probably happens here: https://github.com/asciidoctor/asciidoctor-reveal.js/blob/master/templates/slim/section.html.slim#L16

[obilodeau]

Searching for the first image block with the background attribute inside a section no longer works.

[obilodeau]

1.0.3 has been released: https://github.com/asciidoctor/asciidoctor-reveal.js/releases/tag/v1.0.3

[mojavelinux]

I see the problem. We're now always coercing the attribute name passed to attr and attr? to a string. It used to only coerce symbols. The template attempts to use these methods to access the first positional attribute. It's now necessary to access positional attributes via the attributes map.

attributes[1]

We were missing a test case in core, which is why it slipped through.

I've sent PR #135 to fix, which will also work with 1.5.4.

[mojavelinux]

This risk of incompatibility with upstream can be avoided if we set up triggered builds on asciidoctor-reveal.js when changes are made to upstream.

You'll need to add a profile to the Travis build to run against upstream instead of a release. You can see this implemented in asciidoctor-diagram (https://github.com/asciidoctor/asciidoctor-diagram/blob/master/.travis.yml#L23-L25). Then, I just need to add it to the list of triggered projects (https://github.com/asciidoctor/asciidoctor/blob/master/Rakefile#L143-L147).

[mojavelinux]

Btw, I just added a test to upstream. Of course, the test asserts that a positional (i.e., indexed) attribute can only be accessed from the attributes hash. These attributes are considered to be internal(-ish) and therefore it makes sense they are only accessible via the raw table. I don't want to hide them...but they do require a slightly lower-level of access.

[jirutka]

@obilodeau Your plan is very reasonable! 👍

[obilodeau]

@mojavelinux:

Then, I just need to add it to the list of triggered projects (https://github.com/asciidoctor/asciidoctor/blob/master/Rakefile#L143-L147).

You may proceed. See #136.