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

Update the metadata testing for nonvisual content display #639

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

mattgarrish
Copy link
Member

@mattgarrish mattgarrish commented Jan 30, 2025

This is my best effort to capture as much as we can about nonvisual reading ability, not accounting for the various edge cases we discussed in the last editor's call.

I've changed the existing variables follows:

  • I added a has_nonvisual_metadata variable that checks if any accessMode or accessModeSufficient properties are set. If not, then we don't know anything about the content and this triggers the no information available message.
  • I modified all_necessary_content_textual to add a check that there is only one accessMode property and it has the value textual. This fills in the gap of accessModeSufficient not being required metadata
  • I changed the variable non_textual_content_images to non_textual_content and updated it to check for any accessMode that is not textual so we capture audio and video in addition to images.
  • I added a check for transcripts to textual_alternatives variable since these could be used for audio or video with images of text.

With those changes, the only major modification I had to make to the instructions is to put the "if" check that has_nonvisual_metadata is not true first. This stops us from commenting on any publications for which the publisher hasn't said anything about the access modes and lets the final else capture the ambiguous cases where they have.

The check on all_necessary_content_textual will now better capture all books we know have full text alternatives.

The check on non_textual_content without textual_alternatives will capture the case that some non-textual content is not going to be accessible. (There are ways this can fail, of course, but it relies on the publisher having only provided partial metadata.)

And finally is the catch-all case where we just don't know. There's non-textual content with some kinds of text alternatives, but for whatever reason the publisher isn't claiming the text is fully readable as text.

But if you spot anything that doesn't seem right, let me know.

Note that I added the new ID ways-of-reading-nonvisual-no-metadata for the new "No information about nonvisual reading is available" string. I didn't modify the guidelines to add this.


Preview | Diff

@mattgarrish
Copy link
Member Author

mattgarrish commented Jan 30, 2025

Hm, something's gone really wrong with this pull request. I think I may have undone some of my work and reverted to before I opened the new branch. I'll see if I can fix it, but I might have to open a new pull request. For now, ignore all the hundreds of changes it's flagging in the diff after the nonvisual section.

All fixed now.

</li>

<li>
<b>LET</b> <var>has_nonvisual_metadata</var> be the result of calling <a href="#check-for-node">check for node</a> on <var>package_document</var>, <code class="xpath">/package/metadata/meta[@property=("<i>schema:accessMode</i>", "<i>schema:accessModeSufficient</i>")]</code>.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what this line is doing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It checks that there is at least one meta tag that has a property of accessMode or accessModeSufficient so we don't attempt to process a value when they haven't stated anything about the modes. The value doesn't matter.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but if accessMode="visual" is present wont this variable has_nonvisual_metadata be set?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's not the variable that checks if the content is all available as text. This is checking if the publisher has set the nonvisual metadata we need to test for completeness.

</li>

<li>
<b>LET</b> <var>non_textual_content</var> be the result of calling <a href="#check-for-node">check for node</a> on <var>package_document</var>, <code class="xpath">/package/metadata/meta[@property="<i>schema:accessMode</i>" and not(normalize-space() = "textual")]</code>.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-textual shouldn't it be <>"textual"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, there's a not(...) around the value test to capture anything but textual values.

@clapierre clapierre self-requested a review January 30, 2025 19:49
@mattgarrish
Copy link
Member Author

Also, as I wrote in #633, if we accept this change the descriptive version for alternative text should not include "for images" in it. When the visual content is a video with a transcript, this description would be inaccurate.

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