Fixed #32819 -- Added aria-describedby for help_text.

[nimra200]

This PR continues the discussion here by incorporating the suggested code change.
Release notes & version changed annotations were updated as well.

ticket-32819

[github-actions]

Hello! Thank you for your contribution 💪

As it's your first contribution be sure to check out the patch review checklist.

If you're fixing a ticket from Trac make sure to set the "Has patch" flag and include a link to this PR in the ticket!

If you have any design or process questions then you can ask in the Django forum.

Welcome aboard ⛵️!

[smithdc1]

Thank you soo much for picking this up. 🙏

I've left an initial set of comments against the proposed patch.

I think there's still some questions about how this should be implemented from the previous pr that needs to be addressed.

#15246 (comment)

Have you any particular thoughts on these? (I'm not sure what to recommend here)

Thanks again. 👍

[smithdc1]

Thanks for the updates. Could you have a look at the test failure(s)?

It seems that atleast the test_custom_descrubedby test is failing.

https://djangoci.com/job/pull-requests-focal/database=sqlite3,label=focal-pr,python=python3.9/17702/testReport/junit/forms_tests.tests.test_forms/FormsTestCase/test_custom_describedby/

Did you manage to get the test suite running locally yet?

[nimra200]

Thank you soo much for picking this up. 🙏

I've left an initial set of comments against the proposed patch.

I think there's still some questions about how this should be implemented from the previous pr that needs to be addressed.

#15246 (comment)

Have you any particular thoughts on these? (I'm not sure what to recommend here)

Thanks again. 👍

I'm not sure what to suggest here either unfortunately. @carltongibson could you provide some guidance here please?

[nimra200]

Thanks for the updates. Could you have a look at the test failure(s)?

It seems that atleast the test_custom_descrubedby test is failing.

https://djangoci.com/job/pull-requests-focal/database=sqlite3,label=focal-pr,python=python3.9/17702/testReport/junit/forms_tests.tests.test_forms/FormsTestCase/test_custom_describedby/

Did you manage to get the test suite running locally yet?

I can try to look at the test failures. I'm not too familiar with the test suite so I was looking at https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/unit-tests/ . I'm a little lost on how to get the test suite running locally.

EDIT: I managed to get the test suite running and I'm looking into why the test_custom_describedby test is failing

[nimra200]

I ran ./runtests.py forms_tests and I see the following test failing test_custom_describedby with this error:
File "/Users/Nimra/django/django/forms/boundfield.py", line 285, in build_widget_attrs attrs['aria-describedby'] = f"{helptext_id} {attrs['aria-describedby']}" KeyError: 'aria-describedby'
How can I run a debugging session in an IDE (I'm using VSCode) to check the contents of the attr dictionary?
@smithdc1 @felixxm

[smithdc1]

A few more comments. Sorry I don't have time this week to sit down with it for a full review.

We still need to address the comments from the accessibility team on the previous PR.

[smithdc1]

This doesn't look quite right any more. I think the patch now adds both the default and the custom attribute?

[carltongibson]

Hey @thibaudcolas @smithdc1 — I don't suppose I can ask you to comment on #15246 (comment) can I please?

@thibaudcolas began:

I find it problematic we’d make it possible to override the aria-describedby...

I'm not clear what the conclusion was. 🤔 Do we have a way forward? Is it a blocker? Is there a test case/example we need to make pass? Any other thoughts?

Thanks! 🙏

[smithdc1]

Hi @carltongibson

I'll try and summarise the current status and help give some next steps and some concrete decisions which need to be made. However, I'm unable to give an expert recomendation on a way forward.

• Folk will already be assigning aria-describedby in their own code, see example. There's therefore a backward compatibility concern if Django overwrites user code; however, there's an accessibility concern we allow customisation.
• Django allows html in help_text. However aria-describedby references items should only contain text. ref-1, ref-2: avoid links.

So in terms of next steps, maybe:

1. After ~forever is it too backward compatible to deprecate allowing html inside help text? If we can deprecate it then do so and use aria-describedby otherwise detect if html tags are in the help text and switch to aria-details in those cases.

2. After ~forever is it too backward compatible to deprecate allowing users to add aria-describedby in user code. Further, is it a good idea to not allow any user customisation? If we can deprecate this and decide user code can safely be ignored, then through a deprecation period move to just Django's own style. If not then we'll need to allow customisation, and have a guess at some sort of ordering. (Django first, user code later).

3. I think it would be rather wise to include the fix for ticket-32819 too. That's the ticket to reference errors with aria-describedby. Given the potential disruption above, let's make both changes at the same time so it's in the same release. We've also not really looked at the implications of doing this, so that may impacted our thinking on 1 and 2 above.

[carltongibson]

Hi @smithdc1 — thank you for the summary. Very helpful! 🎁

So initial take on your thoughts:

1. I don't think we can deprecate using HTML in help_text. Using aria-details when help_text contains < chars (for example) seems like a good fallback. 🤔
2. I'm inclined to say that if aria-describedby is set in user-code, we should just use that?
3. I take it you mean ticket-32820 being workied on by demestav in Fixed #32820 --- Improved form accessibility on errors. #15036. Yes, these look like they belong together.

@thibaudcolas it would be good if you could input too, to see if we have a way forward here?

Thanks! 🙏

[thibaudcolas]

On principle it sounds good to me to give priority to aria-describedby if defined in userland. Re HTML in help_text — though potentially problematic, I don't see this as a blocker.

[felixxm]

Superseded by #16920.