fix: consistently parse tabindex, following HTML 5 spec#4637
fix: consistently parse tabindex, following HTML 5 spec#4637WilcoFiers merged 6 commits intodequelabs:developfrom
Conversation
Ensures tabindex is correctly parsed as an integer using regex instead of parseInt to comply with HTML standards. Related to dequelabs#4632
There was a problem hiding this comment.
This is a good start, but I think we can do a little better. Going over the code base, it looks like axe has an awful lot of places where it's parsing tabindex. I think it would be better if we put the code for this in one place. Normally we'd do this in a commons function, but because one of the places is in core, we'll need to do this in core/utils. So how about this:
In lib/core/utils/ create a file parse-tabindex.js with a parseTabindex function that accepts either a string or null. And which returns an integer if the string is a valid tabindex value, or null if not valid or if the value is null. This would need tests in a new test/core/utils/parse-tabindex.js.
This function could then be used in the following files, instead of the current ad-hoc methods:
lib/checks/keyboard/focusable-no-name-evaluate.jslib/checks/keyboard/no-focusable-content-evaluate.jslib/checks/keyboard/tabindex-evaluate.jslib/commons/dom/get-tabbable-elements.jslib/commons/dom/inserted-into-focus-order.jslib/commons/dom/is-focusable.jslib/commons/dom/is-in-tab-order.jslib/core/base/context/create-frame-context.jslib/rules/autocomplete-matches.jslib/rules/no-negative-tabindex-matches.js
Add parseTabindex method in lib/core/utils and reused it in multiple places across the codebase Related to dequelabs#4632
WilcoFiers
left a comment
There was a problem hiding this comment.
This is looking great! The change required a few additional updates, this is looking really good.
Resolving pull request comments Related to dequelabs#4637
WilcoFiers
left a comment
There was a problem hiding this comment.
This is looking great. @ahmedhalac thank you very much for the contribution!
For the books: I reviewed this PR for security.
Ensures tabindex is correctly parsed as an integer using regex instead of parseInt to comply with HTML standards. Closes #4632
### Bug Fixes - **aria-allowed-role:** Add math to allowed roles for img element ([#4658](#4658)) ([f6dddd9](f6dddd9)), closes [#4657](#4657) - **captions:** fix grammar in captions check incomplete message ([#4661](#4661)) ([3ef7058](3ef7058)) - consistently parse tabindex, following HTML 5 spec ([#4637](#4637)) ([3b0a361](3b0a361)), closes [#4632](#4632) - **core:** measure perf for async checks ([#4609](#4609)) ([e7dc26e](e7dc26e)) - **locale:** fixed typos in german (DE) locale ([#4631](#4631)) ([0740980](0740980)) - **locale:** proofread and updated de.json ([#4643](#4643)) ([910cdb2](910cdb2)) - **no-autoplay-audio:** don't timeout for preload=none media elements ([#4684](#4684)) ([b7f1ad1](b7f1ad1)) - **target-size:** do not treat focusable tabpanels as targets ([#4702](#4702)) ([67d4e4f](67d4e4f)), closes [#4421](#4421) [#4701](#4701)
## [4.11.0](v4.10.3...v4.11.0) (2025-10-07) ### Features - add RGAA tags to rules ([#4862](#4862)) ([53a925a](53a925a)) - **aria-prohibited-attr:** add support for fallback roles ([#4325](#4325)) ([62a19a9](62a19a9)) - **axe.d.ts:** add nodeSerializer typings ([#4551](#4551)) ([a2f3a48](a2f3a48)), closes [#4093](#4093) - **DqElement:** deprecate fromFrame function ([#4881](#4881)) ([374c376](374c376)), closes [#4093](#4093) - **DqElement:** Truncate large `html` strings when the element has a large outerHTML string ([#4796](#4796)) ([404a4fb](404a4fb)), closes [#4544](#4544) - **get-xpath:** return proper relative selector for id ([#4846](#4846)) ([1035f9e](1035f9e)), closes [#4845](#4845) - **i18n:** Add Portugal Portuguese translation ([#4725](#4725)) ([5b6a65a](5b6a65a)) - incomplete with node on which an error occurred ([#4863](#4863)) ([32ed8da](32ed8da)) - **locale:** Added ru locale ([#4565](#4565)) ([067b01d](067b01d)) - **tap:** some best practice rules map to RGAA ([#4895](#4895)) ([bc33f4c](bc33f4c)) - **td-headers-attr:** report headers attribute referencing other <td> elements as unsupported ([#4589](#4589)) ([ec7c6c8](ec7c6c8)), closes [#3987](#3987) ### Bug Fixes - **aria-allowed-role:** add form to allowed roles of form element ([#4588](#4588)) ([8aa47ac](8aa47ac)), closes [/github.com/dequelabs/axe-core/blob/develop/lib/standards/html-elms.js#L264](https://github.com/dequelabs//github.com/dequelabs/axe-core/blob/develop/lib/standards/html-elms.js/issues/L264) - **aria-allowed-role:** Add math to allowed roles for img element ([#4658](#4658)) ([95b6c18](95b6c18)), closes [#4657](#4657) - **autocomplete-valid :** Ignore readonly autocomplete field ([#4721](#4721)) ([491f4ec](491f4ec)), closes [#4708](#4708) - **autocomplete-valid:** treat values "xon" and "xoff" as non-WCAG-violations ([#4878](#4878)) ([52bc611](52bc611)), closes [#4877](#4877) - **axe.d.ts:** add typings for preload options object ([#4543](#4543)) ([cfd2974](cfd2974)) - **button-name,input-button-name,input-img-alt:** allow label to give accessible name ([#4607](#4607)) ([a9710d7](a9710d7)), closes [#4472](#4472) [#3696](#3696) [#3696](#3696) - **captions:** fix grammar in captions check incomplete message ([#4661](#4661)) ([11de515](11de515)) - **color-contrast:** do not run on elements with font-size: 0 ([#4822](#4822)) ([d77c885](d77c885)), closes [#4820](#4820) - consistently parse tabindex, following HTML 5 spec ([#4637](#4637)) ([645a850](645a850)), closes [#4632](#4632) - **core:** measure perf for async checks ([#4609](#4609)) ([7e9bacf](7e9bacf)) - fix grammar when using "alternative text" in a sentence ([#4811](#4811)) ([237a586](237a586)), closes [#4394](#4394) - **get-ancestry:** add nth-child selector for multiple siblings of shadow root ([#4606](#4606)) ([1cdd6c3](1cdd6c3)), closes [#4563](#4563) - **get-ancestry:** don't error when there is no parent ([#4617](#4617)) ([a005703](a005703)) - **locale:** fix typos in japanese (ja) locale ([#4856](#4856)) ([3462cc5](3462cc5)) - **locale:** fixed typos in german (DE) locale ([#4631](#4631)) ([b7736de](b7736de)) - **locale:** proofread and updated de.json ([#4643](#4643)) ([8060ada](8060ada)) - **meta-viewport:** lower impact to moderate ([#4887](#4887)) ([2f32aa5](2f32aa5)), closes [#4714](#4714) - **no-autoplay-audio:** don't timeout for preload=none media elements ([#4684](#4684)) ([cdc871e](cdc871e)) - **performanceTimer:** throwing in axe catch clause ([#4852](#4852)) ([a4ade04](a4ade04)), closes [/github.com/dequelabs/axe-core/blob/e7dae4ec48cbfef74de9f833fdcfb178c1002985/lib/core/base/rule.js#L297-L300](https://github.com/dequelabs//github.com/dequelabs/axe-core/blob/e7dae4ec48cbfef74de9f833fdcfb178c1002985/lib/core/base/rule.js/issues/L297-L300) - **performanceTimer:** work in frames ([#4834](#4834)) ([d7dfebc](d7dfebc)) - **rules:** Change "alternate text" to "alternative text" ([#4582](#4582)) ([b03ada3](b03ada3)) - **target-size:** do not treat focusable tabpanels as targets ([#4702](#4702)) ([60d11f2](60d11f2)), closes [#4421](#4421) [#4701](#4701) - **type:** correct RuleError type ([#4893](#4893)) ([d1aa8e2](d1aa8e2)) - **types:** correct raw types ([#4903](#4903)) ([3eade11](3eade11)) This PR was opened by a robot 🤖 🎉
Implementação da correção do bug onde isFocusable() usava parseInt() incorretamente para validar tabindex, não seguindo a spec HTML5. Solução aplicada: - Substituído parseInt() por regex /^\s*([-+]?\d+)/ - Seguindo spec: https://html.spec.whatwg.org/#rules-for-parsing-integers - Arquivo modificado: lib/commons/dom/is-focusable.js Comparação com solução oficial (PR dequelabs#4637): - Minha solução: corrige o bug mas apenas em 1 arquivo - Solução oficial: criou função reutilizável parseTabindex() aplicada em 13 arquivos + testes unitários Análise completa em: ANALISE-ISSUE-4632.md Referências: - Issue: dequelabs#4632 - PR oficial: dequelabs#4637 - Commit oficial: 645a850
Ensures tabindex is correctly parsed as an integer using regex instead of parseInt to comply with HTML standards.
Closes #4632