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

Screen readers announce blank paragraphs. #366

Open
rkingett opened this issue Aug 11, 2020 · 21 comments
Open

Screen readers announce blank paragraphs. #366

rkingett opened this issue Aug 11, 2020 · 21 comments

Comments

@rkingett
Copy link

rkingett commented Aug 11, 2020

Using NVDA, the free and open source screen reader, whenever I read a blog with double returns,

For example, this post,

the screen reader always announces the blank paragraph between the paragraphs.

For example. On here, if one were to write something, then press enter 2 times,

NVDA would announce the next paragraph without announcing the blank line above. Could this be done in write.as because it is almost impossible to read anything on the site this way. It makes it extremely difficult to keep reading, hearing, blank, when skipping to a new paragraph.

Wordpress.com allows users to write in markdown but their blogs are far easier to read because paragraphs appear on a new line, yet, don't have that annoying "blank" in between them.

@thebaer
Copy link
Member

thebaer commented Aug 11, 2020

Thanks for bringing this up. Without knowing for sure, I would imagine this is due to the fact that we use pre-formatted text styling (CSS: white-space: pre-wrap;) for the post body. Do you know if we can make any modifications to tell screen readers to not respect this setting?

@rkingett
Copy link
Author

I am, unfortunately, unaware of any fix we could make to the CSS to make screen readers ignore that carriage return while keeping it visually spaced.

Is it possible we could remove that CSS attribute and just have standard HTML from the markdown?

I'm thinking we need to specify the spacing through
CSS. I'd be happy to test pages, though!

@zaclittleberry
Copy link

oof. yeah. the html actually looks to be pretty well marked up. the css though is doing some real funky stuff. paragraph tags have been marked inline and all as pre-formatted text, which I'm guessing is telling the screen reader that the spaces in the markup are very important, which is really a problem when it comes to spaces between the markup (it's saying, "oh, there's a space between your paragraph tags, that must be important!").

I could help fix this css if someone is willing to accept those fixes and get them merged.

One compromising fix would be to only have the white-space: pre-wrap; on elements with text content directly inside of them (like paragraphs). If we take it off of the article itself and fix the spacing between elements with margin, that should eliminate most of the weirdness.

@rkingett
Copy link
Author

I think I've found a possible solution for now, but I agree. CSS may need to be completely reworked to avoid using ARIA in the future.

Direct Toot URL. It is also copied below.

It doesn't actually create a HTML tag for that blank line, does it? Otherwise, you could add \aria-hidden="true" to it:
Article on that from Mozilla
Generally, you don't need witchcraft to space out elements without impeding screenreaders (it's the default behavior), but it sounds like their existing CSS rules are causing this, so it probably requires rewriting that CSS.

@zaclittleberry
Copy link

@rkingett I think it was clarified in the tootstream, but maybe not here. I have confirmed, there is no extra element, so aria-hidden=true wouldn't work. It is (unfortunately) reading out the empty line between paragraph tags. "But wait, HTML doesn't care about (more than one) whitespace", you say. True, except when white-space:pre-wrap (or the corresponding pre element) is used. If that CSS attribute is moved to only apply to elements that directly contain text (like p tags themselves), and not on elements that contain other elements (right now the attribute is applied to the whole article, which contains div tags and p tags etc and all the spaces between them), that should mostly reduce the screen reader issue while minimizing any display issues that could arise from the assumption that text is preformatted. P tags et al will need to have display block added back in (/inline display removed) and margin added (/margin removal removed) to account for the space around them no longer being preformatted.

@arigaud-ca
Copy link

I made a quick test with browsers inspectors (Firefox and Chrome) with NVDA.
Solution would be to put h (headings) and p element in display:block and apply some margin, to have the same visual effect.
NVDA reads it properly.
@zaclittleberry I will try your solution (remove white-space:pre-wrap from divs and non-text elements).

@arigaud-ca
Copy link

I wonder if the p elements in the article (p without any classes) are automatically generated or if they were formatted by the author/contributor.
In my experiments with white-space:pre-wrap and white-space I figured out NVDA doesn't read any space (or 'blank') as long as it's in the same HTML element.
Saying that I realize it won't be a good practice as far as semantics is concerned, And as @zaclittleberry said the HTML markup here is really clean.

@rkingett
Copy link
Author

Is there a PR I can review?

@zaclittleberry
Copy link

Definitely no PR on my part yet. I don't actually use writeas at the moment so not really familiar enough with the project to make a PR that I'd be confident wouldn't break things. Also, I don't know GO, so it'd definitely be easier if a contributor chimed in on how markup happens and how much variability there can be in the output. If this markup is standard, the fix is likely pretty easy/safe-ish. If this markup is best case scanario, then maybe something else needs to be considered or looked in to.

So, @arigaud-ca do you think it would be helpful to take that test that you did locally and throw a static page up with the changes for others to test? Not sure how variable screen readers are, but maybe @rkingett and some other people could test it. And if some contributors could chime in and comment on how reasonable a path to a fix this is, and where we can go from here, that would be good.

I think I found the template for the relevant post code in /templates/post.tmpl around line 56 there's .IsPlainText and then it seems to switch printing .Content or .HTMLContent. But I'd need a contributor to clarify how what comes out in that section can vary.

@thebaer
Copy link
Member

thebaer commented Aug 31, 2020

Thanks for looking into this, everyone! To fill in some details:

  • The white-space: pre-wrap setting was chosen deliberately, to be more friendly to plain text entry and enable creative spacing, as you might see in poetry (example).

  • I can't remember exactly why pre-wrap was set at the article level (perhaps for more flexibility with hrs, headers, line items, etc. -- it's been a long time since implementing), but I'd love to narrow its scope if we can keep everything looking the same. Besides keeping it on paragraphs, I'm guessing we'll also want it on list items. Let us know what you discover while experimenting with this!

  • If we're going to get rid of pre-wrap entirely, we'll need to change post rendering. We use this library to render posts from Markdown (@arigaud-ca, it's responsible for inserting <p> tags -- though users can also manually insert HTML). The library supports a configuration flag that we would set in this func that would insert hard line breaks. Otherwise I'm guessing we'll need to modify it to insert &nbsp;s to maintain horizontal spacing.

  • @zaclittleberry that template file is for Draft posts, and .IsPlainText simply indicates whether we're outputting the text as-is (.Content) or rendering Markdown (.HTMLContent).

When changing core CSS, it's important to note that WriteFreely enables writers to apply raw custom CSS to their blog and create themes like these. So any pull request should ensure that either:

  • existing themes don't break, or
  • there's a migration path for anyone with a custom design (e.g. a "version 2" stylesheet with these fixes that users can manually upgrade to).

If it looks like we will have major breaking changes, I'll write up some ideas on how to implement this migration path for Custom CSS users. Otherwise please let me know how I can assist.

@rkingett
Copy link
Author

rkingett commented Sep 2, 2020

This point really makes me pause.

there's a migration path for anyone with a custom design (e.g. a "version 2" stylesheet with these fixes that users can manually upgrade to).

It came across that, in order for screen reader users to get rid of this issue, they would need to pay to get that bit of CSS that fixes the problem?

I apologize if I misunderstood point 2.

Or did you just mean to make sure users would be able to upgrade their custom design without hastles?

@thebaer
Copy link
Member

thebaer commented Sep 2, 2020

Or did you just mean to make sure users would be able to upgrade their custom design without hassles?

This is what I meant. If we change all the core CSS without consideration for custom themes, any writers using them will potentially have their blog designs break once their admins deploy this change. This would heap new unwanted work on writers (i.e. fix your broken theme now!) without their consent -- which we always want to avoid, even if it means folks still using outdated code.

So ideally, writers with custom themes will be able to choose whether or not to apply the new core CSS to their blog(s). If they decide to, they should be able to tweak their custom CSS to make sure it works with the new core CSS, and revert back to the old core CSS if needed.

@rkingett
Copy link
Author

@thebaer
Copy link
Member

thebaer commented Jul 2, 2021

@rkingett I'm experimenting with a new layout engine that addresses (among other things) the accessibility issues you brought up here.

Here's the post you mentioned earlier, with some new changes applied: https://tiny.write.as/matt/monetizing-writing. Does that post work better?

@rkingett
Copy link
Author

rkingett commented Jul 2, 2021

The new post is much, much, easier to read, especially on mobile, so yay!

@rkingett
Copy link
Author

rkingett commented Jul 6, 2021

I checked out your whole blog and, yes. It is far easier to read now. Much easier when doing a say all command, as well. :)

@thebaer
Copy link
Member

thebaer commented Jul 6, 2021

Thank you for confirming that! We should be able to use that as a starting point for a fix, then.

@MarcoZehe
Copy link

What's the status of this issue?

@rkingett
Copy link
Author

From what I can gather, a lab experiment has been created that would make this issue it's own theme. The original Write.as creator wants the CSS this way, how it currently is now, because people write poetry on the site, but there has to be a way we can compromise on this very important issue.

@rkingett
Copy link
Author

@MarcoZehe Here is a live preview of the new labs feature. I think it should be an option for all blogs, not just a theme people can use. That link explains, pretty well, where this issue is headed.

@Flameborn
Copy link

Are these changes available to add to an instance? I know that this is not officially supported yet, but I am currently hosting an instance of Writefreely which is used mostly by screen reader users where these changes would make a huge difference.

I can also test run features to see how they stack up in a real-life situation as far as accessibility is concerned, if needed.

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

No branches or pull requests

6 participants