Bug 270854

Summary:

AX: AccessibilityNodeObject::determineAccessibilityRoleFromNode makes many unnecessary dynamicDowncasts.

Product:

WebKit

Reporter:

Andres Gonzalez <andresg_22>

Component:

Accessibility

Assignee:

Andres Gonzalez <andresg_22>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aboxhall, andresg_22, apinheiro, cfleizach, dmazzoni, ews-watchlist, jcraig, samuel_white, tyler_w, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
Patch	none

Andres Gonzalez

Reported 2024-03-12 08:47:50 PDT

Attachments
Patch (11.86 KB, patch) 2024-03-12 09:13 PDT, Andres Gonzalez	no flags	Details Formatted Diff Diff
Patch (11.83 KB, patch) 2024-03-12 10:47 PDT, Andres Gonzalez	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2024-03-12 08:47:58 PDT

<rdar://problem/124455559>

Andres Gonzalez

Comment 2 2024-03-12 09:13:25 PDT

Created attachment 470321 [details] Patch

Tyler Wilcock

Comment 3 2024-03-12 10:00:00 PDT

Comment on attachment 470321 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=470321&action=review r+ with passing EWS! > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:411 > + if (is<HTMLLabelElement>(element.get())) I think dereferencing *element here would save a nullcheck. > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:461 > + if (RefPtr summaryElement = dynamicDowncast<HTMLSummaryElement>(element.get()); summaryElement && summaryElement->isActiveSummary()) I think dereferencing *element here would save a null check. > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:477 > + if (is<HTMLVideoElement>(element.get())) > return AccessibilityRole::Video; > - if (is<HTMLAudioElement>(node.get())) > + if (is<HTMLAudioElement>(element.get())) I think dereferencing *element here would save a null check.

Andres Gonzalez

Comment 4 2024-03-12 10:47:57 PDT

Created attachment 470324 [details] Patch

Andres Gonzalez

Comment 5 2024-03-12 10:55:13 PDT

(In reply to Tyler Wilcock from comment #3) > Comment on attachment 470321 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=470321&action=review > > r+ with passing EWS! > > > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:411 > > + if (is<HTMLLabelElement>(element.get())) > > I think dereferencing *element here would save a nullcheck. > > > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:461 > > + if (RefPtr summaryElement = dynamicDowncast<HTMLSummaryElement>(element.get()); summaryElement && summaryElement->isActiveSummary()) > > I think dereferencing *element here would save a null check. > > > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:477 > > + if (is<HTMLVideoElement>(element.get())) > > return AccessibilityRole::Video; > > - if (is<HTMLAudioElement>(node.get())) > > + if (is<HTMLAudioElement>(element.get())) > > I think dereferencing *element here would save a null check. Addressed all instances, thanks.

EWS

Comment 6 2024-03-12 14:56:05 PDT

Committed 276000@main (8c357cd24a0e): <https://commits.webkit.org/276000@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 470324 [details].

Note You need to log in before you can comment on or make changes to this bug.