Migrate lang attributes to iframe documents and remove default #764
Conversation
Security InsightsNo security relevant content was detected by automated scans. Action Items
Questions or Comments? Reach out on Slack: #support-infosec. |
austinmatherne-wk
force-pushed
the
lang-detection
branch
from
0c10844 to
c2c0ab3
Compare
austinmatherne-wk
force-pushed
the
lang-detection
branch
from
c2c0ab3 to
eaf28a0
Compare
| if ($('html').attr("lang") === undefined) { | ||
| $('html').attr("lang", "en-US"); | ||
| if ($('html').attr("xml:lang") !== undefined ) { | ||
| $(iframe).contents().find('html').attr("xml:lang", $('html').attr("xml:lang")); |
There was a problem hiding this comment.
I'm confused. The original logic was added in commit c3d517e, which I thought was specifically adding lang="" for accessibility tools, so it seems like we need to keep $('html').attr('lang',?
I don't understand why we need to change $(iframe) at all. If that's also needed for accessibility, it seems like it should also be done in load?
(Whatever is decided, a comment of some kind would be helpful.)
There was a problem hiding this comment.
The lang attribute was likely added to pass an automated accessibility checker that ensures a lang value is defined, but it didn’t verify whether the lang attribute accurately reflects the content.
There’s an open support ticket regarding this issue. I don’t have strong preferences for the final implementation, as long as we stop labeling French content as American English.
There was a problem hiding this comment.
The lang="en-US" seems like the correct default for the application. It was always correct prior to commit 9cb2512, but perhaps it's not correct after i18nInit runs. Perhaps setLanguage should also be updating $('html').attr('lang') to match the selected application language?
If we need to copy attributes into the iframe (I'm not sure if xml:lang has any meaning given that we know from the accessibility ticket that xml:lang wasn't good enough.. ?), it seems like we should copy all attributes like we do for body below.
There was a problem hiding this comment.
I tested all major desktop screen readers and observed three distinct behaviors:
- All languages are treated the same, using a single, user selected voice.
- Screen readers inspect the content to determine the language, ignoring both lang and xml:lang attributes (though these might be fallback options if the language can’t be inferred from the content).
- The lang attribute is used, while the xml:lang attribute is ignored.
In none of the tools did I find the xml:lang attribute being used. Additionally, the presence of content within an iframe or serving the document as XHTML made no difference to the behavior.
Based on these findings, I propose the following approach:
- For the non-stub viewer, migrate all attributes on the root html node to the html element of the report iframe (including xml:lang).
- For both stub and non-stub viewers, set the lang attribute of the report iframe’s html element to the value of the report’s xml:lang attribute (if present).
- Set the lang attribute of the root html node to match the application UI language.
- If we don't know the report language because the report lacks an xml:lang value, set the lang attribute of the report iframe’s html element to an empty string. This effectively “resets” the language and prompts the screen reader to use its default voice instead of the application language.
- Within the inspector, override the lang attribute on taxonomy content elements with the selected taxonomy language.
There was a problem hiding this comment.
- Screen readers inspect the content to determine the language, ignoring both lang
- The lang attribute is used
These seem contradictory. Can you clarify?
- Within the inspector, override the lang attribute on taxonomy content elements with the selected taxonomy language.
I didn't follow this. Could you give an example?
Otherwise, 1-4 sound good to me, thanks.
There was a problem hiding this comment.
These seem contradictory. Can you clarify?
They are. The listed behaviors are from different screen readers. A few examples:
- NVDA - All languages are treated the same
- macOS VoiceOver - inspects the content to determine the language
- JAWS - Uses the lang attribute
I didn't follow this. Could you give an example?
Taxonomy labels, references, etc. There are three languages in the viewer and the lang attributes should reflect that.
- The report content (report iframe)
- The selected application UI language (inspector panel)
- The selected taxonomy language (labels )
| $('html').attr("lang", "en-US"); | ||
| if ($('html').attr("xml:lang") !== undefined ) { | ||
| $(iframe).contents().find('html').attr("xml:lang", $('html').attr("xml:lang")); | ||
| $('html').removeAttr("xml:lang"); |
There was a problem hiding this comment.
I don't understand why this is needed.
There was a problem hiding this comment.
The xml:lang value defined for the report doesn't necessarily match the language of the viewer UI, so keeping it at the root html level seems inappropriate. The report xml:lang value is being moved to the iframe HTML element were the report body is also "reparented".
There was a problem hiding this comment.
Ah, so this seems like the real fix: we want to set lang on the root of the application for accessibility, but to avoid confusing (since there's seemingly no real impact?), we should remove the leftover xml:lang from the reparented content document.
ixbrl-viewer only supports two application languages, so the user should only get lang="en" or lang="es" when looking at the DOM, so we probably need to explain that the user is looking in the wrong place to see the language of their report.
Reason for change
Reopening #684. The previous discussion moved into changing the application language. That feature is being handled by another effort.
Description of change
We currently set the lang attribute to a default of en-US for non stub viewers. This change removes the default and migrates the attribute from the root html element to the report iframe.
Steps to Test
review:
@Arelle/arelle
@paulwarren-wk