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

Cocoa MultilineTextInput.value returns an ObjCStrInstance #1779

Closed
mhsmith opened this issue Feb 14, 2023 · 3 comments · Fixed by #2046
Closed

Cocoa MultilineTextInput.value returns an ObjCStrInstance #1779

mhsmith opened this issue Feb 14, 2023 · 3 comments · Fixed by #2046
Labels
bug A crash or error in behavior. good first issue Is this your first time contributing? This could be a good place to start! macOS The issue relates to Apple macOS support.

Comments

@mhsmith
Copy link
Member

mhsmith commented Feb 14, 2023

rubicon-objc       0.4.5
toga-cocoa         0.3.0
toga-core          0.3.0
Python 3.8.10 (v3.8.10:3d8993a744, May  3 2021, 09:09:08) 
[Clang 12.0.5 (clang-1205.0.22.9)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import toga
>>> mti = toga.MultilineTextInput(value="hello")
[Cocoa] Not implemented: MultilineTextInput.set_on_change()
>>> mti.value
<rubicon.objc.collections.ObjCStrInstance 0x1110204c0: NSBigMutableString at 0x111639ae0: hello>

This can cause apparently impossible situations like a string changing its value:

>>> x = mti.value
>>> print(x)
hello
>>> mti.value = "world"
>>> print(x)
world

When we come to do the backend tests for this widget, or any other widget with string attributes, we should assert that the value returned through the Toga public API is actually a Python str.

@mhsmith mhsmith added bug A crash or error in behavior. macOS The issue relates to Apple macOS support. labels Feb 14, 2023
@bruno-rino
Copy link
Contributor

I just ran into this.

TextInput casts the value to str:
https://github.com/beeware/toga/blob/main/cocoa/src/toga_cocoa/widgets/textinput.py#L211-L212

while MultilineTextInput does not:
https://github.com/beeware/toga/blob/main/cocoa/src/toga_cocoa/widgets/multilinetextinput.py#L78-L79

Seems like a simple fix; are you waiting to address this during the widget audit?

@mhsmith
Copy link
Member Author

mhsmith commented Jul 16, 2023

It looks like we missed this during the audit. We should go through all the widget properties that return strings, and add isinstance checks to the testbed.

@freakboy3742 freakboy3742 added the good first issue Is this your first time contributing? This could be a good place to start! label Jul 22, 2023
@tswinb
Copy link
Contributor

tswinb commented Jul 22, 2023

I will look at this one!

freakboy3742 added a commit that referenced this issue Jul 23, 2023
Fix casting str type properties + add tests (#1779)
@mhsmith mhsmith mentioned this issue Nov 30, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A crash or error in behavior. good first issue Is this your first time contributing? This could be a good place to start! macOS The issue relates to Apple macOS support.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants