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

Keep the selection begin index updated #539

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

ShawnCZek
Copy link
Contributor

The selection API, introduced thanks to #419, has a problem retrieving the cursor position if no text is selected. In particular, this is a problem with the WidgetTextInput::GetSelection() method, which is publicly accessible from classes such as ElementFormControlInput.

If you select some text, unselect text, and call the method, you will get the previous index, which may no longer be valid.

This pull request aims to address this issue. Constantly updating selection_begin_index is safe to do; when we want to work with some selection, we first check whether selection_length is greater than zero. Because of this reason, selection_begin_index may have whatever value, and it will not break anything... except for the getter mentioned above.

@mikke89 mikke89 added the bug Something isn't working label Nov 21, 2023
@mikke89 mikke89 merged commit 6f793e2 into mikke89:master Nov 21, 2023
14 checks passed
@mikke89
Copy link
Owner

mikke89 commented Nov 21, 2023

Yeah, this makes a lot of sense to me and looks correct. Thanks for the good description!

@ShawnCZek ShawnCZek deleted the fix_selection_begin_index branch November 21, 2023 19:47
alml pushed a commit to alml/RmlUi that referenced this pull request Jan 2, 2024
@huiseliming
Copy link

This will cause a clipboard copy error in 5.1. Is it possible to integrate the modifications and release a 5.2 version ? I want to use rmlui from vcpkg.

@mikke89
Copy link
Owner

mikke89 commented Jan 29, 2024

We don't really maintain old releases. If you need this fix, then you could make a patch for the vcpkg port.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants