Report author-provided accessible labels in Chrome that are not visible elsewhere on the page#8352
Conversation
…_2::relationTargetsOfType rather than the custom navdirRelation constant (specific to Firefox), so that issue #4773 is also fixed for Chrome, not just Firefox.
| IAccessible2* targetAcc; | ||
| CComQIPtr<IAccessible2_2> pacc2_2=pacc2; | ||
| if(!pacc2_2) return false; | ||
| IUnknown** ppUnk=NULL; |
There was a problem hiding this comment.
Can you use nullptr here instead please.
| res = target.pdispVal->QueryInterface(IID_IAccessible2, (void**)&targetAcc); | ||
| VariantClear(&target); | ||
| if (res != S_OK) | ||
| res=pacc2_2->get_relationTargetsOfType(IA2_RELATION_LABELLED_BY,1,&ppUnk,&nTargets); |
There was a problem hiding this comment.
Might as well declare and initialise res on the one line. I.e. put HRESULT before the res
| // Whether this node has a visible label somewhere else in the tree | ||
| const bool labelVisible = canDetectLabelVisibility // Not all browsers support getting a node's labelledBy node | ||
| && nameIsExplicit && name && name[0] //this node must actually have an explicit name, and not be just an empty string | ||
| const bool labelVisible = nameIsExplicit && name && name[0] //this node must actually have an explicit name, and not be just an empty string |
There was a problem hiding this comment.
Do we care what character name[0] is? If it is required to be visible, do we need to test that it is not a "non-printable" character?
There was a problem hiding this comment.
I think this question is still valid, even though github has hidden it.
There was a problem hiding this comment.
I believe what we have is correct, as if nameIsExplicit, then the author has provided some kind of accessible label, and if it contains anything (even a space) clearly they are wanting to inforce the content, E.g. overriding existing descendant content with a space.
| // then ensure that the label is always reported along withe the node | ||
| // We must exclude tables from this though as table summaries / captions are handled very specifically | ||
| if(canDetectLabelVisibility&&nameIsExplicit&&!nameIsContent&&(role!=ROLE_SYSTEM_TABLE)&&!labelVisible) { | ||
| if(nameIsExplicit&&!nameIsContent&&(role!=ROLE_SYSTEM_TABLE)&&!labelVisible) { |
There was a problem hiding this comment.
I understand this is consistent with the style of the rest of the file, however I would find this much easier to read with spaces around the boolean operators.
| @@ -552,14 +558,13 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(IAccessible2* pacc, | |||
| || role == ROLE_SYSTEM_BUTTONMENU | |||
| || ((role == ROLE_SYSTEM_CHECKBUTTON || role == ROLE_SYSTEM_RADIOBUTTON) && !isLabelVisible(pacc)); | |||
There was a problem hiding this comment.
isLabelVisible() is called here and on line 563. I wonder if this is intentional, should the bool labelVisible which is created at line 561 be used instead?
This is a follow-on from pr #7513. This was originally fixed for Firefox but not Chrome, as chrome did not yet implement the necessary APIs. Now it does, this standardizes the code to use a technique supported by both browsers.
Link to issue number:
Fixes #4773
Summary of the issue:
Sometimes a web author will use aria-label on a node to convey screen-reader specific information, however the node also has useful and valid content itself (E.g. an edit field, or an iFrame).
NVDA already reports this label when moving to these controls with quick navigation or tabbing, but it should also report it when reading the control with the browse mode caret, as this label will not be rendered anywhere else.
Of course care must be taken to ensure this is in deed a label set by aria-label and not say a label from aria-labelledby or an associated html label tag. The one exception to this is if the associated label node is hidden from the screen.
Description of how this pull request fixes the issue:
In Chrome we now convey to NVDA that these labels must always be reported, by exposing the 'alwaysReportName' attribute on these nodes from the vbufbackend.
Testing performed:
Tested Firefox and Chrome browsers by arrowing up and down the page with NVDA's screen layout mode off, using the document from
sr_only_labels.html.txt
Note: the needed functionality was introduced to Chrome in version 66.
Known issues with pull request:
This is now implemented for Firefox and Chrome, but
it is not supported in other browsers because:
Change log entry:
Bug fixes:
Accessible labels for controls in Google Chrome are now more readily reported in browse mode when the label does not appear as content itself. (#4773)