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

darwinkit: issue with native string conversion #188

Closed
progrium opened this issue Aug 19, 2023 · 1 comment
Closed

darwinkit: issue with native string conversion #188

progrium opened this issue Aug 19, 2023 · 1 comment
Labels
help wanted Extra attention is needed
Milestone

Comments

@progrium
Copy link
Owner

progrium commented Aug 19, 2023

Any argument or return value that would return an NSString pointer is converted to a native Go string. This usually is fine, but because the NSString is an object and therefore a pointer, it can be nil. I've found one case so far where this is a problem which happens to be in one of the examples we have.

The webview example uses a webkit custom helper we inherited called AddScriptMessageHandlerWithReply which uses a custom implementation of webkit.PScriptMessageHandlerWithReply implementing the method UserContentControllerDidReceiveScriptMessageReplyHandler, which takes a replyHandler callback that looks like this:

func(reply objc.Object, errorMessage string)

Here is the documentation for this method with the underlying callback signature:
https://developer.apple.com/documentation/webkit/wkscriptmessagehandlerwithreply/3585111-usercontentcontroller?language=objc

Reply is an Object since it can be any value, and errorMessage is a string since it would be an NSString *. Keep in mind NSString is always a pointer. But in this case, this API says if there is no error, you should pass nil. The best we can do on the Go side given the string type is an empty string, which is not nil, so the webview DOM API (as used in the webview example's assets/index.html) thinks there is an error. Always.

As an experiment, I have a temporary change where an empty string will be passed as nil. However you can imagine this breaks other APIs where you might want an empty string. In fact, I changed some of the examples where they did pass an empty string to instead pass a single space character so they still work. So this is not the solution, but I'm not sure what is.

I'm pretty sure we still do usually want the string conversion, but clearly in some cases we don't. The only general rule I can think might be worth trying is that callback arguments are not converted to Go native strings. That would solve this particular case, and there is likely at least one other case like this, and I think it's not a terrible tradeoff / special case to have to manually convert to native strings in callbacks. They already have different rules than methods in that they take struct values instead of interfaces for class types.

Alternatively, callback arguments that have NSString could become a Go *string. Alternatively, though more work, we could find a way to create symbol specific override rules. Given this is the only case I've found, if that were easy I'd do that for now since it would not impact other binding APIs as much. I just don't have time after all this work recently to try to figure that out.

Are there methods that use this pattern of a nil NSString to mean something specific? I'm not sure. I just get the feeling this is more likely to be found in callbacks, but I could be totally wrong.

@progrium progrium added the help wanted Extra attention is needed label Aug 19, 2023
@progrium progrium added this to the 0.5.0 milestone Aug 19, 2023
@progrium
Copy link
Owner Author

Since we can only find two examples of this both in webkit, I'm going to see about just having it skip these and we can provide a custom implementation with *string and I'll have objc convert to nil or NSString*. And I'll change back the conversion of empty strings to nil.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant