Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parse <img> tags #438

Merged
merged 1 commit into from
Apr 21, 2020
Merged

Parse <img> tags #438

merged 1 commit into from
Apr 21, 2020

Conversation

mgeier
Copy link
Member

@mgeier mgeier commented Apr 18, 2020

Sadly, with the current Pandoc-based implementation I couldn't make the align attribute work. I had to use "inline" images (because Pandoc stores the <img> tag in a RawInline object), but :align: is not supported in reST for inline images.

This fixes (at least partially) #52, #284, #437 and probably #321.

Rendered HTML: https://208-210404706-gh.circle-artifacts.com/0/html/markdown-cells.html#Using-the-HTML-%3Cimg%3E-tag

Rendered PDF: https://208-210404706-gh.circle-artifacts.com/0/nbsphinx.pdf#subsubsection.3.5.1

And rendered HTML on RTD: https://nbsphinx--438.org.readthedocs.build/en/438/markdown-cells.html#Using-the-HTML-%3Cimg%3E-tag

@mgeier mgeier merged commit afa0ab2 into spatialaudio:master Apr 21, 2020
@mgeier mgeier deleted the parse-img branch April 21, 2020 13:51
@jayaddison
Copy link

Hi @mgeier - an unusual quirk of this feature is that when an <img> element without an alt attribute is processed from a notebook, docutils picks up the emitted inline image substitution and uses the 'name' of that substitution -- the UUID4 value -- as the alt-text.

I guess it seemed reasonable for docutils to pick the substitution-name as fallback-alt-text because usually subsitution names are somewhat-human-readable, but in the case of these ephemeral substitutions it causes document reproducibility problems (ref: Debian bug #1064575) and I doubt it's useful for accessibility either.

Do you have any thoughts on how we could/should handle that better?

@mgeier
Copy link
Member Author

mgeier commented Mar 6, 2024

Thanks for the report @jayaddison!
I wasn't aware of that behavior.

But doesn't that sound more like a bug in docutils?
Why would it set the alt attribute to anything if nothing is given?

I guess it seemed reasonable for docutils to pick the substitution-name as fallback-alt-text

Maybe it seemed reasonable to someone, but it isn't.
The whole concept of a "fallback-alt-text" doesn't make any sense. The alt text should be written specifically for the purpose, because it is supposed to describe that image for people who can't see it. If no intentional text is available, the alt attribute should be omitted.

I doubt it's useful for accessibility either.

Definitely not.
It's actively harmful.


Maybe related: #500, #162, jgm/pandoc#6194

@jayaddison
Copy link

You're welcome @mgeier - thanks for the considered response.

I guess it seemed reasonable for docutils to pick the substitution-name as fallback-alt-text

Maybe it seemed reasonable to someone, but it isn't. The whole concept of a "fallback-alt-text" doesn't make any sense. The alt text should be written specifically for the purpose, because it is supposed to describe that image for people who can't see it. If no intentional text is available, the alt attribute should be omitted.

Yep, alt-text is best written for-purpose. When that doesn't happen, though, it'd be nice not to introduce accessibility/comprehensibility regressions or other unintended side-effects.

People are capable of making the most of the info provided to them - but UUIDs are -- by-design -- low-information, while human-language labels are likely (hopefully) to be more meaningful.

Are pandoc and docutils the only two downstream processors for nbsphinx's markdown cell processing, or are there other places to check for compatibility for any suggested changes?

@mgeier
Copy link
Member Author

mgeier commented Mar 7, 2024

Yep, alt-text is best written for-purpose. When that doesn't happen, though, it'd be nice not to introduce accessibility/comprehensibility regressions or other unintended side-effects.

Exactly.
AFAIK, the only reasonable thing to do in absence of an intentional alt text, is omitting the alt attribute in the generated HTML.

People are capable of making the most of the info provided to them - but UUIDs are -- by-design -- low-information, while human-language labels are likely (hopefully) to be more meaningful.

They might be more meaningful, but that doesn't necessarily mean that they are suitable as alt text.
The image directive has an :alt: option, and if that's not specified, docutils shouldn't emit an alt attribute either, IMHO.

Are pandoc and docutils the only two downstream processors for nbsphinx's markdown cell processing, or are there other places to check for compatibility for any suggested changes?

Yes, Markdown cells are processed with Pandoc via markdown2rst:

{{ output.data['text/markdown'] | markdown2rst | indent }}

... which is defined here:

def markdown2rst(text):

The Markdown cells are converted to reST, which is then parsed by docutils.
Note that I consider the detour via reST as a temporary solution, see #36.

The intermediate reST document for <img src="images/notebook_icon.png" width="300"> looks something like this:

|38d18b0a1469490f86f4570a48f1693d|

...

.. |38d18b0a1469490f86f4570a48f1693d| image:: images/notebook_icon.png
    :width: 300
    :class: no-scaled-link

With alt attribute, i.e. <img src="images/notebook_icon.png" alt="Jupyter notebook icon" width="300">, it looks like this:

|0689d12a29f54e41a224a6998da89e99|

...

.. |0689d12a29f54e41a224a6998da89e99| image:: images/notebook_icon.png
    :alt: Jupyter notebook icon
    :width: 300
    :class: no-scaled-link

@jayaddison
Copy link

Thanks @mgeier - yep, I agree with your assessment, and the illustrative examples of the intermediate rST format look correct too.

AFAIK, the only reasonable thing to do in absence of an intentional alt text, is omitting the alt attribute in the generated HTML.

I've been wondering about -- but haven't yet confirmed -- two ways to do this on the docutils -> Sphinx path, the output I currently care about (although I don't want to forget the pandoc path too): either begin including an empty :alt: option in the intermediate rST (ugly, and not perfect - docutils would emit an alt="" attribute on the HTML, I believe, but could be updated to drop those), AND/OR remove the fallback-alt-text logic in docutils entirely.

@mgeier
Copy link
Member Author

mgeier commented Mar 9, 2024

ugly, and not perfect - docutils would emit an alt="" attribute on the HTML, I believe, but could be updated to drop those),

That might be very problematic!

The important thing to know is that alt="" has very different semantics compared to omitting the alt attribute:

alt="" means that the image is purely decorative: https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/alt#decorative_images
This in turn means that screen readers will not even mention it.

A non-existing alt attribute means that the author was too lazy to provide help.
This in turn means that screen readers will try to provide other helpful information about the image, for example they might read the file name instead (but different screen readers might behave differently).

Therefore, alt="" should never be used as an automatic fallback, because it will essentially hide the image from users of screen readers.

AND/OR remove the fallback-alt-text logic in docutils entirely.

Yes, as I said above (#438 (comment)), the only reasonable fallback is to completely omit the alt attribute from the generated HTML.

@jayaddison
Copy link

Thank you @mgeier - I wasn't previously aware of the meaning of an empty-string alt attribute. In that case, I agree with omitting the attribute from the downstream HTML when none was configured at-source.

@jayaddison
Copy link

I began looking at docutils to try to figure out what changes might be required -- but then decided to look at the history of the relevant lines.

It seems that the alt option handling changed here: https://sourceforge.net/p/docutils/code/1236

In particular: the logic that only sends an alt= kwarg to self.directive(match, ...) if the alt option is set on the parent element is roughly the change that I had in mind.

@jayaddison
Copy link

It seems that the alt option handling changed here: https://sourceforge.net/p/docutils/code/1236

Nope, ignore that; the test coverage of alt attributes didn't change, so I believe that revision is irrelevant.

In any case: I'll try to develop a docutils patch for this and may take that into a thread on the relevant docutils project mailing list(s).

@jayaddison
Copy link

In any case: I'll try to develop a docutils patch for this and may take that into a thread on the relevant docutils project mailing list(s).

Mailing list discussion/question opened here: https://sourceforge.net/p/docutils/mailman/message/58746996/

@mgeier
Copy link
Member Author

mgeier commented Mar 10, 2024

Thanks for taking care of this! Let's hope for a positive response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants