fix(compiler): Add support for aria-invalid#63748
fix(compiler): Add support for aria-invalid#63748JeanMeche wants to merge 1 commit intoangular:mainfrom
aria-invalid#63748Conversation
The `aria-invalid` attribute was supported but the `ariaInvalid` property was unknown by the schema. fixes angular#63744
|
@JeanMeche can we please add support for |
Review appreciated, but is not blocking.
josephperrott
left a comment
There was a problem hiding this comment.
LGTM
Reviewed-for: fw-security
jelbourn
left a comment
There was a problem hiding this comment.
LGTM
Reviewed-for: fw-security
|
I see that you just added the If you want your PR to be merged, it has to pass all the CI checks. If you can't get the PR to a green state due to flakes or broken |
8867eac to
77afb7e
Compare
| 'tabindex': 'tabIndex', | ||
|
|
||
| // https://www.w3.org/TR/wai-aria-1.2/#accessibilityroleandproperties-correspondence | ||
| // https://www.w3.org/TR/wai-aria-1.2/#accessibilityroleandproperteies-correspondence |
There was a problem hiding this comment.
| // https://www.w3.org/TR/wai-aria-1.2/#accessibilityroleandproperteies-correspondence | |
| // https://www.w3.org/TR/wai-aria-1.2/#accessibilityroleandproperties-correspondence |
77afb7e to
1f5c30d
Compare
leonsenft
left a comment
There was a problem hiding this comment.
These attribute/property pairs with mismatched names (e.g. aria-controls, ariaControlsElements) break an assumption made in #62630.
#62630 made it so that Angular would always render a property whose name begins with aria as an attribute.
This was largely facilitated by the fact that one can relatively easily convert from ARIA property name to the corresponding attribute name–without needing to retain an exhaustive mapping:
These mismatched names break this assumption. Attempting to bind [ariaControlsElements]="controls" will incorrectly set the aria-controlselements attribute, when the attribute should just be aria-controls.
In summary, the resulting behavior from this change would be:
| Binding | Result | Notes |
|---|---|---|
aria-control="string" |
✅ | Static attribute |
[attr.aria-control]="expr" |
✅ | Explicit attribute binding |
[aria-control]="expr" |
✅ | Implicit attribute binding |
[ariaControlsElements]="expr" |
❌ | Explicit property binding, this gets mapped to the incorrect property name. |
[ariaControls]="expr" |
This would get mapped to the correct attribute, but isn't a valid property name. |
I'm not certain yet what the best solution here is, just pointing out the problem I see at the moment.
| const SCHEMA: string[] = [ | ||
| '[Element]|textContent,%ariaAtomic,%ariaAutoComplete,%ariaBusy,%ariaChecked,%ariaColCount,%ariaColIndex,%ariaColSpan,%ariaCurrent,%ariaDescription,%ariaDisabled,%ariaExpanded,%ariaHasPopup,%ariaHidden,%ariaKeyShortcuts,%ariaLabel,%ariaLevel,%ariaLive,%ariaModal,%ariaMultiLine,%ariaMultiSelectable,%ariaOrientation,%ariaPlaceholder,%ariaPosInSet,%ariaPressed,%ariaReadOnly,%ariaRelevant,%ariaRequired,%ariaRoleDescription,%ariaRowCount,%ariaRowIndex,%ariaRowSpan,%ariaSelected,%ariaSetSize,%ariaSort,%ariaValueMax,%ariaValueMin,%ariaValueNow,%ariaValueText,%classList,className,elementTiming,id,innerHTML,*beforecopy,*beforecut,*beforepaste,*fullscreenchange,*fullscreenerror,*search,*webkitfullscreenchange,*webkitfullscreenerror,outerHTML,%part,#scrollLeft,#scrollTop,slot' + | ||
| export const SCHEMA: string[] = [ | ||
| '[Element]|textContent,%ariaActiveDescendant,%ariaAtomic,%ariaAutoComplete,%ariaBusy,%ariaChecked,%ariaColCount,%ariaColIndex,%ariaColSpan,%ariaControlsElement,%ariaCurrent,%ariaDescribedByElements,%ariaDescription,%ariaDetailsElements,%ariaDisabled,%ariaErrorMessageElements,%ariaExpanded,%ariaFlowToElements,%ariaHasPopup,%ariaHidden,%ariaInvalid,%ariaKeyShortcuts,%ariaLabel,%ariaLabelledByElements,%ariaLevel,%ariaLive,%ariaModal,%ariaMultiLine,%ariaMultiSelectable,%ariaOrientation,%ariaOwnsElements,%ariaPlaceholder,%ariaPosInSet,%ariaPressed,%ariaReadOnly,%ariaRelevant,%ariaRequired,%ariaRoleDescription,%ariaRowCount,%ariaRowIndex,%ariaRowSpan,%ariaSelected,%ariaSetSize,%ariaSort,%ariaValueMax,%ariaValueMin,%ariaValueNow,%ariaValueText,%classList,className,elementTiming,id,innerHTML,*beforecopy,*beforecut,*beforepaste,*fullscreenchange,*fullscreenerror,*search,*webkitfullscreenchange,*webkitfullscreenerror,outerHTML,%part,#scrollLeft,#scrollTop,slot' + |
There was a problem hiding this comment.
| '[Element]|textContent,%ariaActiveDescendant,%ariaAtomic,%ariaAutoComplete,%ariaBusy,%ariaChecked,%ariaColCount,%ariaColIndex,%ariaColSpan,%ariaControlsElement,%ariaCurrent,%ariaDescribedByElements,%ariaDescription,%ariaDetailsElements,%ariaDisabled,%ariaErrorMessageElements,%ariaExpanded,%ariaFlowToElements,%ariaHasPopup,%ariaHidden,%ariaInvalid,%ariaKeyShortcuts,%ariaLabel,%ariaLabelledByElements,%ariaLevel,%ariaLive,%ariaModal,%ariaMultiLine,%ariaMultiSelectable,%ariaOrientation,%ariaOwnsElements,%ariaPlaceholder,%ariaPosInSet,%ariaPressed,%ariaReadOnly,%ariaRelevant,%ariaRequired,%ariaRoleDescription,%ariaRowCount,%ariaRowIndex,%ariaRowSpan,%ariaSelected,%ariaSetSize,%ariaSort,%ariaValueMax,%ariaValueMin,%ariaValueNow,%ariaValueText,%classList,className,elementTiming,id,innerHTML,*beforecopy,*beforecut,*beforepaste,*fullscreenchange,*fullscreenerror,*search,*webkitfullscreenchange,*webkitfullscreenerror,outerHTML,%part,#scrollLeft,#scrollTop,slot' + | |
| '[Element]|textContent,%ariaActiveDescendantElement,%ariaAtomic,%ariaAutoComplete,%ariaBusy,%ariaChecked,%ariaColCount,%ariaColIndex,%ariaColSpan,%ariaControlsElement,%ariaCurrent,%ariaDescribedByElements,%ariaDescription,%ariaDetailsElements,%ariaDisabled,%ariaErrorMessageElements,%ariaExpanded,%ariaFlowToElements,%ariaHasPopup,%ariaHidden,%ariaInvalid,%ariaKeyShortcuts,%ariaLabel,%ariaLabelledByElements,%ariaLevel,%ariaLive,%ariaModal,%ariaMultiLine,%ariaMultiSelectable,%ariaOrientation,%ariaOwnsElements,%ariaPlaceholder,%ariaPosInSet,%ariaPressed,%ariaReadOnly,%ariaRelevant,%ariaRequired,%ariaRoleDescription,%ariaRowCount,%ariaRowIndex,%ariaRowSpan,%ariaSelected,%ariaSetSize,%ariaSort,%ariaValueMax,%ariaValueMin,%ariaValueNow,%ariaValueText,%classList,className,elementTiming,id,innerHTML,*beforecopy,*beforecut,*beforepaste,*fullscreenchange,*fullscreenerror,*search,*webkitfullscreenchange,*webkitfullscreenerror,outerHTML,%part,#scrollLeft,#scrollTop,slot' + |
1f5c30d to
64c363c
Compare
|
We chatted offline with Leon, there needs to be some design discussion to support props that don't realy match their corresponding attribute. We'll keep this PR a fix to support |
The `aria-invalid` attribute was supported but the `ariaInvalid` property was unknown by the schema. fixes angular#63744 PR Close angular#63748
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
The
aria-invalidattribute was supported but theariaInvalidproperty was unknown by the schema.fixes #63744