Summary: | AX: Stop returning AccessibilityUIElements backed by a null pointer | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tyler Wilcock <tyler_w> | ||||||||||||||||
Component: | Accessibility | Assignee: | Tyler Wilcock <tyler_w> | ||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||
Severity: | Normal | CC: | andresg_22, webkit-bug-importer | ||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||
Attachments: |
|
Description
Tyler Wilcock
2021-11-15 11:03:54 PST
Created attachment 444279 [details]
Patch
Created attachment 444287 [details]
Patch
Created attachment 444288 [details]
Patch
(In reply to Tyler Wilcock from comment #2) > Created attachment 444279 [details] > Patch --- a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.cpp +++ a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.cpp -Ref<AccessibilityUIElement> AccessibilityUIElement::create(PlatformUIElement uiElement) +RefPtr<AccessibilityUIElement> AccessibilityUIElement::create(PlatformUIElement uiElement) I think it is odd that a class create method can return null. Haven't seen examples of that in WebKit, but I can be convinced otherwise. Instead I think we shouldn't try to create an AccessibilityUIElement if the platform element is null. + if (!uiElement) + return { }; If this is the way to go, return nullptr here. --- a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h +++ a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h - static Ref<AccessibilityUIElement> create(PlatformUIElement); + static RefPtr<AccessibilityUIElement> create(PlatformUIElement); Again, create shouldn't return null IMHO. Created attachment 444290 [details]
Patch
(In reply to Andres Gonzalez from comment #5) > (In reply to Tyler Wilcock from comment #2) > > Created attachment 444279 [details] > > Patch > > --- a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.cpp > +++ a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.cpp > > -Ref<AccessibilityUIElement> > AccessibilityUIElement::create(PlatformUIElement uiElement) > +RefPtr<AccessibilityUIElement> > AccessibilityUIElement::create(PlatformUIElement uiElement) > > I think it is odd that a class create method can return null. Haven't seen > examples of that in WebKit, but I can be convinced otherwise. Instead I > think we shouldn't try to create an AccessibilityUIElement if the platform > element is null. > > + if (!uiElement) > + return { }; > > If this is the way to go, return nullptr here. > > --- a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h > +++ a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h > > - static Ref<AccessibilityUIElement> create(PlatformUIElement); > + static RefPtr<AccessibilityUIElement> create(PlatformUIElement); > > Again, create shouldn't return null IMHO. Yeah, I think that's reasonable. I checked, and you're right that every create method everywhere returns a Ref. I'll change it. Created attachment 444304 [details]
Patch
Created attachment 444310 [details]
Patch
(In reply to Tyler Wilcock from comment #9) > Created attachment 444310 [details] > Patch --- a/Tools/WebKitTestRunner/InjectedBundle/win/AccessibilityControllerWin.cpp +++ a/Tools/WebKitTestRunner/InjectedBundle/win/AccessibilityControllerWin.cpp -Ref<AccessibilityUIElement> AccessibilityController::focusedElement() +RefPtr<AccessibilityUIElement> AccessibilityController::focusedElement() { notImplemented(); return AccessibilityUIElement::create(nullptr); we should return nullptr here for consistency. Created attachment 444321 [details]
Patch
Committed r285859 (244285@main): <https://commits.webkit.org/244285@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 444321 [details]. |