WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
184919
AX: soft link libAccessibility.dylb
https://bugs.webkit.org/show_bug.cgi?id=184919
Summary
AX: soft link libAccessibility.dylb
Nan Wang
Reported
2018-04-24 09:32:52 PDT
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
>
Attachments
patch
(7.49 KB, patch)
2018-04-24 09:42 PDT
,
Nan Wang
mitz: review-
Details
Formatted Diff
Diff
patch
(2.94 KB, patch)
2018-04-24 18:40 PDT
,
Nan Wang
no flags
Details
Formatted Diff
Diff
patch
(3.17 KB, patch)
2018-04-24 21:43 PDT
,
Nan Wang
mitz: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Nan Wang
Comment 1
2018-04-24 09:42:33 PDT
Created
attachment 338655
[details]
patch
WebKit Commit Bot
Comment 2
2018-04-24 14:27:06 PDT
Comment on
attachment 338655
[details]
patch Clearing flags on attachment: 338655 Committed
r230971
: <
https://trac.webkit.org/changeset/230971
>
WebKit Commit Bot
Comment 3
2018-04-24 14:27:08 PDT
All reviewed patches have been landed. Closing bug.
mitz
Comment 4
2018-04-24 14:51:19 PDT
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
Comment 5
2018-04-24 15:05:03 PDT
Re-opened since this is blocked by
bug 184939
Nan Wang
Comment 6
2018-04-24 15:05:31 PDT
(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
Comment 7
2018-04-24 18:40:42 PDT
Created
attachment 338695
[details]
patch weak link instead
mitz
Comment 8
2018-04-24 20:10:26 PDT
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
Comment 9
2018-04-24 21:43:41 PDT
Created
attachment 338706
[details]
patch update from review
mitz
Comment 10
2018-04-24 21:49:51 PDT
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 11
2018-04-24 21:55:43 PDT
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
Comment 12
2018-04-24 22:52:59 PDT
Committed
r230987
: <
https://trac.webkit.org/changeset/230987
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug