Bug 251849

<rdar://problem/105128584>

Created attachment 464885 [details] Patch

Comment on attachment 464885 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=464885&action=review > COMMIT_MESSAGE:1 > +AX: Remove unnecessary TextMrker methods from WebAccessibilityObjectWrapper. Typo — TextMrker should be TextMarker

Comment on attachment 464885 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=464885&action=review > COMMIT_MESSAGE:1 > +AX: Remove unnecessary TextMrker methods from WebAccessibilityObjectWrapper. Typo here in TextMarker. > Source/WebCore/accessibility/mac/AXObjectCacheMac.mm:535 > + [change setObject:marker.bridgingAutorelease() forKey:NSAccessibilityTextChangeValueStartMarker]; Never need autorelease when passing a value to a method, only when returning. Wastefully puts an object into the autorelease pool. Instead this should use get() and bridge_cast. You can probably find an example by searching for bridge_cast.

Set back to review? by accident.

(In reply to Darin Adler from comment #4) > Comment on attachment 464885 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=464885&action=review > > > COMMIT_MESSAGE:1 > > +AX: Remove unnecessary TextMrker methods from WebAccessibilityObjectWrapper. > > Typo here in TextMarker. > > > Source/WebCore/accessibility/mac/AXObjectCacheMac.mm:535 > > + [change setObject:marker.bridgingAutorelease() forKey:NSAccessibilityTextChangeValueStartMarker]; > > Never need autorelease when passing a value to a method, only when > returning. Wastefully puts an object into the autorelease pool. Instead this > should use get() and bridge_cast. You can probably find an example by > searching for bridge_cast. Thanks Darin. Tried that but getting error: no matching function for call to 'bridge_cast' [change setObject:bridge_cast(marker.get()) forKey:NSAccessibilityTextChangeValueStartMarker]; ^~~~~~~~~~~ /OpenSource/WebKitBuild/Release/usr/local/include/wtf/cocoa/TypeCastsCocoa.h:59:113: note: candidate template ignored: could not match 'RetainPtr<T>' against 'WTF::RetainPtr<const __AXTextMarker>::PtrType' (aka 'const __AXTextMarker *') template<typename T> inline RetainPtr<std::remove_pointer_t<typename CFTollFreeBridgingTraits<T>::BridgedType>> bridge_cast(RetainPtr<T>&& object) ^ /OpenSource/WebKitBuild/Release/usr/local/include/wtf/cocoa/TypeCastsCocoa.h:54:79: note: candidate template ignored: substitution failure [with T = const __AXTextMarker *]: implicit instantiation of undefined template 'WTF::CFTollFreeBridgingTraits<const __AXTextMarker *>' template<typename T> inline typename CFTollFreeBridgingTraits<T>::BridgedType bridge_cast(T object) ~~~~~~~~~~~~~~~~~~~~~~~~ ^ /Users/ag/s/web/OpenSource/WebKitBuild/Release/usr/local/include/wtf/cocoa/TypeCastsCocoa.h:49:113: note: candidate template ignored: could not match 'RetainPtr<T>' against 'WTF::RetainPtr<const __AXTextMarker>::PtrType' (aka 'const __AXTextMarker *') template<typename T> inline RetainPtr<typename NSTollFreeBridgingTraits<std::remove_pointer_t<T>>::BridgedType> bridge_cast(RetainPtr<T>&& object) ^ /Users/ag/s/web/OpenSource/WebKitBuild/Release/usr/local/include/wtf/cocoa/TypeCastsCocoa.h:44:102: note: candidate template ignored: substitution failure [with T = const __AXTextMarker *]: implicit instantiation of undefined template 'WTF::NSTollFreeBridgingTraits<const __AXTextMarker>' template<typename T> inline typename NSTollFreeBridgingTraits<std::remove_pointer_t<T>>::BridgedType bridge_cast(T object) ~~~~~~~~~~~~~~~~~~~~~~~~ ^ Maybe because AXTextMarkerRef is an opaque pointer? Can get it to compile with: [change setObject:(__bridge id)(marker.get()) forKey:NSAccessibilityTextChangeValueStartMarker]; which perhaps is the right thing in this case.

Created attachment 464923 [details] Patch

Created attachment 464924 [details] Patch

Comment on attachment 464924 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=464924&action=review > Source/WebCore/accessibility/mac/AXObjectCacheMac.mm:535 > + [change setObject:(__bridge id)marker.get() forKey:NSAccessibilityTextChangeValueStartMarker]; Great as is. To make code like this look more elegant and perhaps be more type safe we also have bridge_id_cast for cases where there is not an Objective-C equivalent. Or if there is an Objective-C toll free bridged version of AXTextMarkerRef, we could add it to the toll free bridging header and then use bridge_cast. But none of that is required, this is fine.

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