Bug 256077 - AX: Reduce unnecessary reliance on renderers in AccessibilityListBox, AccessibilitySlider, AccessibilityTableCell, AccessibilityTableRow, AccessibilityTable, and AccessibilityARIAGrid
Summary: AX: Reduce unnecessary reliance on renderers in AccessibilityListBox, Accessi...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tyler Wilcock
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-04-27 23:01 PDT by Tyler Wilcock
Modified: 2023-04-28 12:44 PDT (History)
10 users (show)

See Also:


Attachments
Patch (15.55 KB, patch)
2023-04-27 23:06 PDT, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (22.15 KB, patch)
2023-04-28 00:45 PDT, Tyler Wilcock
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tyler Wilcock 2023-04-27 23:01:43 PDT
This will make it easier to support display:contents for these classes.
Comment 1 Radar WebKit Bug Importer 2023-04-27 23:01:56 PDT
<rdar://problem/108644964>
Comment 2 Tyler Wilcock 2023-04-27 23:06:41 PDT
Created attachment 466121 [details]
Patch
Comment 3 Tyler Wilcock 2023-04-28 00:45:14 PDT
Created attachment 466123 [details]
Patch
Comment 4 chris fleizach 2023-04-28 10:25:37 PDT
Comment on attachment 466123 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=466123&action=review

> Source/WebCore/accessibility/AccessibilityTableCell.cpp:336
> +    if (static_cast<int>(rowRange.second) == -1) {

how would this be -1 if the original declaration is unsigned?

std::pair<unsigned, unsigned> rowRange
Comment 5 Tyler Wilcock 2023-04-28 10:34:50 PDT
(In reply to chris fleizach from comment #4)
> Comment on attachment 466123 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=466123&action=review
> 
> > Source/WebCore/accessibility/AccessibilityTableCell.cpp:336
> > +    if (static_cast<int>(rowRange.second) == -1) {
> 
> how would this be -1 if the original declaration is unsigned?
> 
> std::pair<unsigned, unsigned> rowRange
axRowSpan() will assign rowRange.second to -1, but since it's unsigned that will actually assign a value of 4294967295. But the check here does static_cast<int>, turning 4294967295 back into -1. So I think this should be OK
Comment 6 chris fleizach 2023-04-28 10:38:10 PDT
(In reply to Tyler Wilcock from comment #5)
> (In reply to chris fleizach from comment #4)
> > Comment on attachment 466123 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=466123&action=review
> > 
> > > Source/WebCore/accessibility/AccessibilityTableCell.cpp:336
> > > +    if (static_cast<int>(rowRange.second) == -1) {
> > 
> > how would this be -1 if the original declaration is unsigned?
> > 
> > std::pair<unsigned, unsigned> rowRange
> axRowSpan() will assign rowRange.second to -1, but since it's unsigned that
> will actually assign a value of 4294967295. But the check here does
> static_cast<int>, turning 4294967295 back into -1. So I think this should be
> OK

Yea seems reasonable as long as overflow math continues to work
Comment 7 EWS 2023-04-28 12:44:45 PDT
Committed 263511@main (3228c83a428f): <https://commits.webkit.org/263511@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 466123 [details].