rst: use CSS for image alignment.

[flying-sheep]

fixes #163

[kayyymarieee]

content = File.read('test2.txt')
detection = CharlockHolmes::EncodingDetector.detect(content)
utf8_encoded_content = CharlockHolmes::Converter.convert content, detection[:encoding], 'UTF-8'

[kayyymarieee]

9173626

[terencehonles]

@flying-sheep do you know if any of the other options are not working? You may want to update the branch so the CI can re-run (it now uses GitHub actions).

[flying-sheep]

Well, you use the HTML::Pipeline::SanitizationFilter without options:

https://github.com/github/markup/blob/master/test/markup_test.rb#L44-L47

This means that these defaults are used: https://github.com/gjtorikian/html-pipeline/blob/0e3d84eb13845e0d2521ef0dc13c9c51b88e5087/lib/html/pipeline/sanitization_filter.rb#L133

Changing the config in the tests here won’t change what GitHub does.

And the defaults are bad: They allow the obsolete align on all elements, but not style="float: left".

We have to make GitHub parametrize its use of HTML::Pipeline::SanitizationFilter to allow some specific styles. Or update the defaults of that library to allow that: gjtorikian/html-pipeline#302

[flying-sheep]

Regarding you question: alt and target work, those do not:

• width and height are done as CSS and stripped by sanitation (boo! But we could make this library emit width and height attributes instead)
• scale is ignored by the rst2html command in the first place (I assume there’s some option to make it read the image and add width and height with the correct values, because it was written before CSS scale() existed)

[terencehonles]

We have to make GitHub parametrize its use of HTML::Pipeline::SanitizationFilter to allow some specific styles.

Would that be too hard to do? It seems like it must be done in order for the tests to pass. That does point out that it might still end up being stripped by GitHub in their prod pipeline, but as stated in html-pipeline GitHub uses a similar pattern, but not that gem.

This project was started at GitHub. While GitHub still uses a similar design and pattern for rendering content, this gem should be considered standalone and independent from GitHub.

So either way they need to be aware they should not be stripping the inline styles.

[terencehonles]

From this on the README:

2. The HTML is sanitized, aggressively removing things that could harm you and your kin—such as script tags, inline-styles, and class or id attributes.

this fix may never work unless they do allow some inline styles.

[flying-sheep]

Pretty much only this one. width and height are OK as attributes, but align is deprecated in favor of style="float: …".

But we need someone from GitHub providing transparency here.

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[flying-sheep]

Not my fault that there has been no recent activity

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[flying-sheep]

oh my god.

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[flying-sheep]

hey @team can you PLEASE pin this or fix this?

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[flying-sheep]

still no.

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[flying-sheep]

continues to be no.

thinking about making a bot to bump this.

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[flying-sheep]

Please, anyone help.

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[flying-sheep]

been a month already huh?

[Copilot]

Pull request overview

Updates the reStructuredText (.rst) renderer to preserve image alignment after sanitization (fixes #163), and adds a fixture-based test case covering right-aligned images.

Changes:

• Override the Docutils HTML translator’s visit_image to inject alignment styling into generated <img> tags.
• Add an .. image:: example with :align: right to the RST test document.
• Update the expected sanitized HTML fixture to include the aligned <img> output.

Show a summary per file

File	Description
test/markups/README.rst	Adds an aligned image directive to exercise the alignment behavior.
test/markups/README.rst.html	Updates expected sanitized HTML output to include the aligned <img>.
lib/github/commands/rest2html	Implements custom visit_image logic to apply alignment via injected styling.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 3/3 changed files
• Comments generated: 2

[zkoppert]

Hey @flying-sheep — thanks for putting this together, and sorry it's been sitting for so long without a clear resolution.

After taking a close look at this PR with fresh eyes, I believe the core approach here won't work in production due to how GitHub's sanitization pipeline handles the output. Specifically:

The style attribute is stripped by HTML::Pipeline::SanitizationFilter. The rendered HTML from rest2html produces <img style="float: right" ... />, but the sanitizer aggressively removes all inline styles before the HTML reaches users. The alignment would never actually be visible. You can see this documented in the README:

The HTML is sanitized, aggressively removing things that could harm you and your kin—such as script tags, inline-styles, and class or id attributes.

The deprecated HTML align attribute does survive sanitization today (it's in the allowlist), so <img align="right" ...> would technically work — but that would require a different approach and comes with its own trade-offs since align is deprecated HTML.

There are also a couple of secondary issues:

• RST's top, middle, bottom, and center align values don't map to valid CSS float values — only left and right do
• The test expectations include style= attributes that the sanitizer would strip, so the tests would fail against the full pipeline

I think the right move here is to close this PR given how long it's been open and how much rework would be needed. If RST image alignment is something we want to revisit, a fresh issue would be a better starting point — potentially exploring the align attribute approach or a sanitizer allowlist update.

Thanks again for the contribution and the thoughtful discussion in the thread over the years. 🙏

[zkoppert]

Closing per the review above. If RST image alignment is still desired, a fresh issue would be the best path forward.