feat(annotate_citations): use aria-attributes #4932
Conversation
![semgrep-app[bot]](https://avatars.githubusercontent.com/in/60555?s=60&v=4){kind=small}
Solves #1178 Use aria-description on the anchor tag for a resolved citation. The description uses the case name, truncated if it is too long
grossir
force-pushed
the
1178-aria-attributes-for-citations
branch
from
454457a to
da7cd1f
Compare
From looking at |
|
Process-wise, I assigned this to you, Bill, for triage or to review, as you see fit. Gianfranco, I removed you as assignee, so that it will show up in Bill's sprint view. Bill, if you're not going to review you should assign it to somebody else AND make them reviewer. |
| @@ -61,8 +63,12 @@ def generate_annotations( | |||
| "</span>", | |||
| ] | |||
| else: # If successfully matched... | |||
| case_name = trunc(best_case_name(opinion.cluster), 60, "...") | |||
There was a problem hiding this comment.
I mean I understand why we do it with the metadata field but an aria attribute is for accessibility and that user may want the full name that is being linked no? for the description we are already on the page ...
There was a problem hiding this comment.
Why TRUNCate this?
In case it's too long; since we expect the aria-* to be useful specially for "Id", "Supra", "Ibid" where there is no further textual reference; but could be too repetitive, worst in case of long case names.
Should I remove this? I am not sure about the statistics on case name lenght
I mean I understand why we do it with the metadata field but an aria attribute is for accessibility and that user may want the full name that is being linked no? for the description we are already on the page ...
The targeted case is a screen reader. Setting aria-description makes both the attribute value and the link content be read. You can check a longer comment about that here
|
Sorry, random thought: Do we need to worry about special characters in the aria-attribute and encode them? |
|
@mlissner I would think so. but how many random characters do you expect in our name strings? Are you worried about characters messing up readers or messing up the HTML |
|
@flooie, yes, I'm worried about a case with a name like, "A&M Univesity v. Foo" that would mess up our HTML. |
|
no - apologies |
|
@mlissner - in my limited testing - ampersands do not cause an issue with the HTML. |
|
What's your testing, because I don't think you can put am ampersand into an HTML attribute without it being invalid. It's possible that it renders correctly, but that'd just be the browser fixing your error, and it's probably possible for worse errors to cause real problems (like what if there's a double quote in the case name, for example). |
|
This says in html5 ampersands are kind of OK, but mostly not: https://stackoverflow.com/a/19442133/64911. But other characters like double quotes will definitely cause issues. |
|
I tested the ampersand and saw no issue extracting it back out - but Im sure you are right about quotes being an issue - ill make the change to escape theHTML |
Solves #1178
Use aria-description on the anchor tag for a resolved citation. The description uses the case name, truncated if it is too long