Bug 184919 – AX: soft link libAccessibility.dylb

• Component: Accessibility
• Status: RESOLVED FIXED

[Nan Wang]

Crash on some machine
Dyld Error Message:
  Library not loaded: /usr/lib/libAccessibility.dylib
  Referenced from: /System/Library/Frameworks/WebKit.framework/Versions/A/WebKit
  Reason: image not found

<rdar://problem/39669349>

[Nan Wang]

Created attachment 338655 [details]
patch

[WebKit Commit Bot]

Comment on attachment 338655 [details]
patch

Clearing flags on attachment: 338655

Committed r230971: <https://trac.webkit.org/changeset/230971>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

Comment on attachment 338655 [details]
patch

This is not an appropriate way to resolve this sort of issue. The correct fix would have been to use $(WK_MACOS_WEAK_FRAMEWORK) instead of -framework, and to perform the appropriate nil checks.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 184939

[Nan Wang]

(In reply to mitz from comment #4)
> Comment on attachment 338655 [details]
> patch
> 
> This is not an appropriate way to resolve this sort of issue. The correct
> fix would have been to use $(WK_MACOS_WEAK_FRAMEWORK) instead of -framework,
> and to perform the appropriate nil checks.

Ok I rolled it out and will fix that again

[Nan Wang]

Created attachment 338695 [details]
patch

weak link instead

Comment on attachment 338695 [details]
patch

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

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:3079
> +    if (functionPtr)

You generally can’t null-check function pointers like this. Consider using isNullFunctionPointer() from <https://trac.webkit.org/browser/webkit/trunk/Source/WTF/wtf/darwin/WeakLinking.h>.

[Nan Wang]

Created attachment 338706 [details]
patch

update from review

Comment on attachment 338706 [details]
patch

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

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:706
> +        CFNotificationCenterAddObserver(CFNotificationCenterGetDarwinNotifyCenter(), (__bridge const void *)(self), accessibilityEventsEnabledChangedCallback, kAXSWebAccessibilityEventsEnabledNotification, 0, CFNotificationSuspensionBehaviorDeliverImmediately);

Not new to this patch, but why is it OK to add this observer and never remove it?

[Nan Wang]

Comment on attachment 338706 [details]
patch

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

>> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:706
>> +        CFNotificationCenterAddObserver(CFNotificationCenterGetDarwinNotifyCenter(), (__bridge const void *)(self), accessibilityEventsEnabledChangedCallback, kAXSWebAccessibilityEventsEnabledNotification, 0, CFNotificationSuspensionBehaviorDeliverImmediately);
> 
> Not new to this patch, but why is it OK to add this observer and never remove it?

Good catch, there is CFNotificationCenterRemoveObserver in dealloc but it's guarded in PLATFORM(IOS).
I will move that into #if ENABLE(ACCESSIBILITY_EVENTS) since that's the only place with CFNotification

[Nan Wang]

Committed r230987: <https://trac.webkit.org/changeset/230987>