RESOLVED FIXED 268997
AX: Backing object can be destroyed in _accessibilityActivate 
https://bugs.webkit.org/show_bug.cgi?id=268997
Summary AX: Backing object can be destroyed in _accessibilityActivate
Joshua Hoffman
Reported 2024-02-08 09:51:23 PST
By calling press() on the backing object, we may destroy the object.
Attachments
Patch (1.87 KB, patch)
2024-02-08 09:58 PST, Joshua Hoffman 		no flags
DetailsFormatted DiffDiff
Patch (1.87 KB, patch)
2024-02-08 10:23 PST, Joshua Hoffman 		no flags
DetailsFormatted DiffDiff
Patch (3.88 KB, patch)
2024-02-08 20:21 PST, Joshua Hoffman 		no flags
DetailsFormatted DiffDiff
Patch (3.83 KB, patch)
2024-02-09 10:52 PST, Joshua Hoffman 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2024-02-08 09:51:35 PST
<rdar://problem/122554193>
Joshua Hoffman
Comment 2 2024-02-08 09:51:42 PST
rdar://122491194
Joshua Hoffman
Comment 3 2024-02-08 09:58:25 PST
Created attachment 469773 [details] Patch
Joshua Hoffman
Comment 4 2024-02-08 10:23:29 PST
Created attachment 469775 [details] Patch
chris fleizach
Comment 5 2024-02-08 12:43:01 PST
Comment on attachment 469775 [details] Patch feels like we might be able to get a test for this by destroying a button on a press
Joshua Hoffman
Comment 6 2024-02-08 14:40:13 PST
(In reply to chris fleizach from comment #5) > Comment on attachment 469775 [details] > Patch > > feels like we might be able to get a test for this by destroying a button on > a press Good idea, I can add one here.
Joshua Hoffman
Comment 7 2024-02-08 20:21:04 PST
Created attachment 469784 [details] Patch
Tyler Wilcock
Comment 8 2024-02-09 10:46:56 PST
Comment on attachment 469784 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=469784&action=review > Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:2011 > + RefPtr<AccessibilityObject> backingObject = self.axBackingObject; Nit: I'd personally remove the newline before this line, grouping the statements of preparing, and subsequently protecting if said preparation is successful > LayoutTests/accessibility/ios-simulator/destroy-on-press.html:7 > +<body id="body"> Is this id necessary? > LayoutTests/accessibility/ios-simulator/destroy-on-press.html:26 > + let output = "This makes sure that we don't crash when activating an element that destroys itself.\n\n" > + > + if (window.accessibilityController) { > + // Should not crash here > + accessibilityController.accessibleElementById("summary1").childAtIndex(0).press(); > + output += "Did not crash when pressing #summary1."; > + debug(output); > + } > + > + function destroySummary() { > + document.getElementById("summary1").remove(); > + } I prefer not to indent inside a script tag in layout tests (easier to read).
Joshua Hoffman
Comment 9 2024-02-09 10:52:09 PST
Created attachment 469798 [details] Patch
Joshua Hoffman
Comment 10 2024-02-09 10:52:39 PST
(In reply to Tyler Wilcock from comment #8) > Comment on attachment 469784 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=469784&action=review > > > Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:2011 > > + RefPtr<AccessibilityObject> backingObject = self.axBackingObject; > > Nit: I'd personally remove the newline before this line, grouping the > statements of preparing, and subsequently protecting if said preparation is > successful > > > LayoutTests/accessibility/ios-simulator/destroy-on-press.html:7 > > +<body id="body"> > > Is this id necessary? > > > LayoutTests/accessibility/ios-simulator/destroy-on-press.html:26 > > + let output = "This makes sure that we don't crash when activating an element that destroys itself.\n\n" > > + > > + if (window.accessibilityController) { > > + // Should not crash here > > + accessibilityController.accessibleElementById("summary1").childAtIndex(0).press(); > > + output += "Did not crash when pressing #summary1."; > > + debug(output); > > + } > > + > > + function destroySummary() { > > + document.getElementById("summary1").remove(); > > + } > > I prefer not to indent inside a script tag in layout tests (easier to read). Addressed all of these in the latest attachment
EWS
Comment 11 2024-02-12 11:57:23 PST
Committed 274477@main (fcc36e12e4cb): <https://commits.webkit.org/274477@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 469798 [details].
Note You need to log in before you can comment on or make changes to this bug.