NSViewAttachment's accessibilityParent bug

General JUCE discussion
1

We ran into a problem that I need to fix so thought to ask here. As I never used Objective-C before, please feel free to correct me…

NSViewAttachment is saving the NSView’s AccessibilityParent and overwriting it with its own handler:

if (@available (macOS 10.10, *))
{
    previousAccessibilityParent = [view accessibilityParent];
    [view setAccessibilityParent:static_cast<id> (owner.getAccessibilityHandler()->getNativeImplementation())];
}

At destruction time it tries to revert to the original, but in certain cases setAccessibilityParent crashes:

void removeFromParent()
{
    if (@available (macOS 10.10, *))
        if (previousAccessibilityParent != nil) {
            [view setAccessibilityParent: previousAccessibilityParent];
        }

Resetting the property causes crashes depending on how the host is closing the window… (clicking the X will work perfectly, but other methods when the DAW closes the window by not user interaction will fail).

I suspect it’s because the original accessibilityParent is a weak property and JUCE is not tracking it as weak, so it was probably already destroyed when JUCE tried to set it back.

the crash goes away if I do something like this, but it’s obviously just a quick hack:

void removeFromParent()
{
    if (@available (macOS 10.10, *))
        if (previousAccessibilityParent != nil) {
            previousAccessibilityParent = nil;
            [view setAccessibilityParent: previousAccessibilityParent];
        }

Am I wrong that it might be a JUCE bug? @reuk

2

Actually, defining previousAccessibilityParent as __weak solves the crash.

id __weak previousAccessibilityParent = nil;

However the nil-check in removeFromParent() will prevent it from setting nil back, and the NSView will keep the JUCE reference instead of nil. This applies also to the current code: if it was originally nil, we never revert back to that because of the nil-check.

3

I believe this is already fixed on develop:

1 Like
4

Sorry, my bad for not checking develop first… it seems yes, that is a fix!
Thank you for the quick response as always!