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

Double/triple click selection of long words does not select all characters #4960

Closed
4 tasks done
jupjohn opened this issue Nov 11, 2023 · 4 comments
Closed
4 tasks done
Labels
bug Something isn't working as intended, or works in a confusing/unintuitive way for the user

Comments

@jupjohn
Copy link
Contributor

jupjohn commented Nov 11, 2023

Checklist

  • I'm reporting a problem with Chatterino
  • I've verified that I'm running the most recent nightly build or stable release
  • I've looked for my problem on the wiki
  • I've searched the issues and pull requests for similar looking reports

Describe your issue

When double clicking on a long word in a message, the selection does not span the entire word. This is also exhibited by triple clicking the message.

Example message:

@user-type=;color=#FF0000;emotes=;turbo=0;returning-chatter=0;client-nonce=2513080064f6a1335c3c4b460849e7ac;badges=glhf-pledge/1;user-id=130496419;id=e96f7752-d2d6-45c1-b23f-dd7a6173d535;rm-received-ts=1699732253387;flags=;tmi-sent-ts=1699732253150;first-msg=0;room-id=46673989;subscriber=0;mod=0;display-name=jendrina22;badge-info= :jendrina22!jendrina22@jendrina22.tmi.twitch.tv PRIVMSG #masayoshi widepepohappycliminghardo4housewithcomodohypeontopholdingapepegadragonpoganonactionfigure

Screenshots

Double click selection:

image

Expected text selection: widepepohappycliminghardo4housewithcomodohypeontopholdingapepegadragonpoganonactionfigure
Actual text selection: widepepohappycliminghardo4housewithcomodohypeontopholdingapepega

The following videos demonstrate double clicking then triple clicking behaviour.

Chatterino2_SelectionBug_Singleline.mp4
Chatterino2_SelectionBug_Multiline.mp4

OS and Chatterino Version

Chatterino 2.4.6 (commit 95620e6) built with Qt 6.6.0 Running on Arch Linux, kernel: 6.6.1-zen1-1-zen

@jupjohn jupjohn added the issue-report An issue reported by a user. label Nov 11, 2023
@pajlada pajlada added bug Something isn't working as intended, or works in a confusing/unintuitive way for the user and removed issue-report An issue reported by a user. labels Nov 11, 2023
@kornes
Copy link
Contributor

kornes commented Nov 12, 2023

double-click selection ends selection on the line's end rather than word end, i tested c2 2.4.4 and it behaves the same so likely it never worked as expected.

But your videos shows more problems:

  • your double-click selection doesn't match whole row and skips last 2-3 characters, while also selecting beginning of next row (vid 2), seems like some width calculation is wrong (issue not present on windows)
  • triple-click selection also matches username, but shouldn't (Fix: dont select mod buttons at triple click #4961)

@KleberPF
Copy link
Contributor

KleberPF commented Mar 6, 2024

Biggest problem I see here is that, because of the way line breaks are done, I don't think there is a (good) way to know the difference between these

image

Any ideas on how to fix this issue without completely remaking the line break logic?

@Nerixyz
Copy link
Contributor

Nerixyz commented Mar 6, 2024

Any ideas on how to fix this issue without completely remaking the line break logic?

When adding a TextLayoutElement with line breaks, this element can have flags set (on the MessageLayoutElement) that indicate, which direction the text continues (if any).

The following would be done when calling (a new) MessageLayout::getWordBounds(pos = clickedPos):

# this is pseudocode
startIndex, startElement = getElementAt(pos)
endElement = startElement
endIndex = startIndex + endElement.getSelectionIndexCount()

while startElement.hasPrev:
  startElement = elementBefore(startElement)
  startIndex -= startElement.getSelectionIndexCount()

while endElement.hasNext:
  endElement = elementAfter(endElement)
  endIndex += endElement.getSelectionIndexCount()

if endElement.hasTrailingSpace():
  endIndex -= 1

return startIndex, endIndex

I think this could extend MessageLayoutContainer::getSelectionIndex.
In ChannelView, we wouldn't call the anonymous getWordBounds, but the one from MessageLayoutElement.
This can get quite fun with off-by-one errors.


Example: Say you have the message "b³ a⁴² b³", which could get broken up like this:

bbb
aaaaaaaaaa
aaaaaaaaaa
aaaaaaaaaa
aaaaaaaaaa
aa bbb

This would result in the following layout elements:

[0]: "bbb"        [trailingSpace]
[1]: "aaaaaaaaaa" [hasNext]
[2]: "aaaaaaaaaa" [hasNext, hasPrev]
[3]: "aaaaaaaaaa" [hasNext, hasPrev]
[4]: "aaaaaaaaaa" [hasNext, hasPrev]
[5]: "aa"         [hasPrev, trailingSpace]
[6]: "bbb"        [trailingSpace]

Suppose the user double-clicks on element 2.

  • getElementAt would get the element 2 and its starting index, so $\verb|startIndex| = 3 + 1 + 10 = 14$.
  • endIndex would be $14 + 10 = 24$.
  • The first while loop would look at the following elements:
    • [1]: $\verb|startIndex| = 14 - 10 = 4$
  • The second while loop would look at the following elements:
    • [3]: $\verb|endIndex| = 24 + 10 = 34$
    • [4]: $\verb|endIndex| = 34 + 10 = 44$
    • [5]: $\verb|endIndex| = 44 + 3 = 47$
  • The element has a trailing space, so $\verb|endIndex| = 47 - 1 = 46$

Now, $46 - 4 = 42$ characters are selected.

@KleberPF
Copy link
Contributor

KleberPF commented Mar 7, 2024

Good idea. I looked into it a bit more yesterday and quickly hacked together a way to get the length of the original word and just return that as the selection length, which surprisingly works a lot of the times but would break in the example you gave (the selection would only go forward from the line).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as intended, or works in a confusing/unintuitive way for the user
Projects
None yet
Development

No branches or pull requests

5 participants