Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AppKit] Add missing Accessibility protocols. #7105

Merged

Conversation

mandel-macaque
Copy link
Member

This commit adds a few protocols that were missing. It is interesting to
mention that there are two of the protocols that are decorated with
NS_PROTOCOL_REQUIRES_EXPLICIT_IMPLEMENTATION, as per Apple documentation
this means that:

'Unlike normal protocols, when you adopt one of the role-specific protocols,
Xcode may ask you to reimplement methods that have already been implemented
by one of your ancestors.

In order to ensure that your control returns accurate and useful information,
some methods are tagged with the NS_PROTOCOL_REQUIRES_EXPLICIT_IMPLEMENTATION
attribute. For these methods, you need to override your superclass’s
implementation with your own.
'

In this case, we need to add the methods from all the parent classes and
set them to be [Abstract] since they are required. Not all methods have
to be added ONLY the required ones.

fixes: #7079

This commit adds a few protocols that were missing. It is interesting to
mention that there are two of the protocols that are decorated with
NS_PROTOCOL_REQUIRES_EXPLICIT_IMPLEMENTATION, as per Apple documentation
this means that:

'Unlike normal protocols, when you adopt one of the role-specific protocols,
 Xcode may ask you to reimplement methods that have already been implemented
 by one of your ancestors.

 In order to ensure that your control returns accurate and useful information,
 some methods are tagged with the NS_PROTOCOL_REQUIRES_EXPLICIT_IMPLEMENTATION
 attribute. For these methods, you need to override your superclass’s
 implementation with your own.
'

In this case, we need to add the methods from all the parent classes and
set them to be [Abstract] since they are required. Not all methods have
to be added ONLY the required ones.

fixes: dotnet#7079
@mandel-macaque
Copy link
Member Author

@mandel-macaque
Copy link
Member Author

build

Copy link
Contributor

@spouliot spouliot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Want to check the API diff before approving

@@ -25489,11 +25498,43 @@ interface NSAccessibilityTable : NSAccessibilityGroup {
}

[Mac (10,10)]
[Protocol]
interface NSAccessibilityOutline : NSAccessibilityTable {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

normally that would break things... but since there's no [BaseType] I have the feeling that this was never generated

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was never, correct.

src/appkit.cs Outdated
interface NSAccessibilityOutline : NSAccessibilityTable {
[Abstract]
[NullAllowed, Export ("accessibilityLabel")]
new string AccessibilityLabel { get; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why re-define it (and others) ?

This is an interface so it inherits the same member from NSAccessibilityTable, so the contract remains the same.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing, if contract is the same.

@mandel-macaque
Copy link
Member Author

Introspection tests are failing due to metal, I'll fix those in a diff branch.

@mandel-macaque mandel-macaque merged commit 2cea356 into dotnet:xcode11.1 Sep 26, 2019
@mandel-macaque mandel-macaque deleted the add-missing-accessibility branch September 26, 2019 13:47
@monojenkins
Copy link
Collaborator

Build failure

!!! Couldn't read commit file !!!

@chamons
Copy link
Contributor

chamons commented Oct 1, 2019

@monojenkins backport to d16-4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants