SuffolkLITLab / docassemble-ALWeaver

A tool to help quickly generate draft interviews from an existing document (pdf or DOCX) for the docassemble platform.
https://apps-test.suffolklitlab.org/start/ALWeaver/assembly_line/#/1
MIT License
19 stars 5 forks source link

Wrap download_list_html in <p> so VoiceRSS sees it #918

Closed BryceStevenWilley closed 8 months ago

BryceStevenWilley commented 8 months ago

Originally found in https://github.com/mplp/docassemble-mlhframework/issues/62, docassemble only sends parts of the page to Voice RSS to be read. It has a fairly complicated set of rules, including the fact that it skips over divs and the contents of any tag that isn't only a string. All that means that the download list buttons and document titles aren't read out.

It's fairly minor, and the voice RSS has a bunch of other issues as well (see https://github.com/mplp/docassemble-mlhframework/issues/62#issuecomment-1874630556), but wrapping the entire download_list_html in paragraph tags stops docassemble from skipping over those buttons. It's a bit hacky, but it's not something that's worth fixing upstream, and this is a fairly simple change.

nonprofittechy commented 8 months ago

Are other block level elements allowed to be in a <p> tag?

BryceStevenWilley commented 8 months ago

Hm, not technically. Browsers seem to handle it by automatically closing the paragraph tag before the div, and ignoring the later closing p tag. But yeah, not to spec.

I don't think there's another way around the issue other than tricking docassemble like this or trying to change the screenreader code extensively, which itself is finicky (I tried to format everything such that the docassemble wouldn't skip the text, but it either skips almost everything, or duplicates what is read due to how it reads text content). I'm convinced that turning off VoiceRSS is the better option, but eh. I'll close this.

nonprofittechy commented 8 months ago

Hm, not technically. Browsers seem to handle it by automatically closing the paragraph tag before the div, and ignoring the later closing p tag. But yeah, not to spec.

I don't think there's another way around the issue other than tricking docassemble like this or trying to change the screenreader code extensively, which itself is finicky (I tried to format everything such that the docassemble wouldn't skip the text, but it either skips almost everything, or duplicates what is read due to how it reads text content). I'm convinced that turning off VoiceRSS is the better option, but eh. I'll close this.

I'm open to hearing more about voicerss and other ways it might break things... we've not gotten a lot of favorable feedback on it. Can you say more?

plocket commented 8 months ago

We might want to change this just a tad upstream in download_list_html if this is a pervasive problem. We'll then also be able to adjust styles if there's a problem with those.

BryceStevenWilley commented 8 months ago

Less that it breaks other things, and more that it's just not going to be great at reading everything on the page. I didn't know how it parsed the page until yesterday when I looked into it.

We might want to change this just a tad upstream in download_list_html

I thought about that. We could; changing the divs to tables and putting icons under an additional span might help. But this is just one case, and the more I look into it, the more screenreader.py doesn't behave great in other places as well.

Essentially, I'm not confident that it'll do a good job the more components we use on a page. To fix things would mean we'd start iteratively implement browser accessibility rules, which seems ironic given that we're saying that this isn't an accessibility feature. We could just say it's not intended to read all page content, but why have a feature that doesn't actually read you the full page? There might be some point between where it is now and something much too complicated, but I don't know where that is. All of that pushes me to say it's not worth the effort to polish the feature.