-
Notifications
You must be signed in to change notification settings - Fork 60
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
Fix crash in CompletionEntry #53
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks - just asking what you think about a potential modification.
@@ -162,6 +162,9 @@ func newNavigableList(items []string, entry *widget.Entry, setTextFromMenu func( | |||
fn(i, o) | |||
return | |||
} | |||
if i < 0 || i >= widget.ListItemID(len(n.items)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this fix, but I do wonder - should we guard the fn
call above as well? It won't crash immediately but we may be leading the callback to crash or get unexpected callback data...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think @sharki13 could you move it so the fn
callback does not get invalid data as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this questions be answered @sharki13 ? It does seem like a good thing to have fixed.
Just pinging on this to see if the suggested change could be made so this can be landed? |
Fix for #38
From time to time there is a crash during typing something to entry, while list of options is modified basis on entered text.
While
UpdateItem
is executed,n.items
slice is shorter than indexi
indicates, which lead to crash.