[DictQuickLookup] NT: fix utf8 byte drift during text selection by Commodore64user · Pull Request #15015 · koreader/koreader

Commodore64user · 2026-02-21T02:11:34Z

In TextBoxWidget, highlight_start_idx / highlight_end_idx are character indices.
In DictQuickLookup:_performLookupOnSelection, we use:

selection_widget.text:sub(start_idx, end_idx)

but string.sub uses byte indices. So with UTF-8 content (the IPA /mænˈkaɪnd/ etc.), extraction shifts left and returns the wrong token (e.g. "with Sc").

bug fix

Refactored onTextSelectorPress and onTextSelectorModifierPress to extract the selected text via a callback from onHoldReleaseText, and pass it directly to _performLookupOnSelection.
Changed _performLookupOnSelection to accept the selected text as its argument, rather than the selection widget, simplifying its interface and removing internal logic for extracting text from the widget.

This change is

…ookups

poire-z · 2026-02-21T08:48:28Z

Good that you found the reason ! Did you get joy and a wonderful sleep ?

But it's a bit ugly that you have to do that and know about the internals.
You have this:

koreader/frontend/ui/widget/dictquicklookup.lua

Lines 2006 to 2010 in 7de4f4c

    
           -- second press, 
        
           -- process the hold release event which finalizes text selection 
        
           selection_widget:onHoldReleaseText(nil, self:_createTextSelectionGesture("hold_release")) 
        
           self:_performLookupOnSelection(selection_widget, false) 
        
           self:onStopTextSelectorIndicator()

You pass nil as the onHoldReleaseText callback, but if you passed something, it would get the selected_text as its first arg, and you wouldn't have to do that yourself.

I have no idea if this would work with the html widget, but logically, you could have something cleaner with:

-    selection_widget:onHoldReleaseText(nil, self:_createTextSelectionGesture("hold_release"))
-    self:_performLookupOnSelection(selection_widget, false)
+    local selected_text
+    selection_widget:onHoldReleaseText(function(text) selected_text = text end, self:_createTextSelectionGesture("hold_release"))
-    self:_performLookupOnSelection(selected_text, false)

-function DictQuickLookup:_performLookupOnSelection(selection_widget, switch_domain)
+function DictQuickLookup:_performLookupOnSelection(selected_text, switch_domain)
+ -- use selected_text directly instead of re-fetching it

poire-z · 2026-02-21T14:13:07Z

+            function(text) selected_text = text end,
+            self:_createTextSelectionGesture("hold_release")
+        )
+        self:_performLookupOnSelection(selected_text, true) -- switch_domain, "i'm master of my domain" ¯\_(ツ)_/¯


How old are you?

lol, I don't want him to go... leave him alone

poire-z · 2026-02-21T14:17:00Z

I have no idea if this would work with the html widget, but logically, you could have something cleaner with:

What ?! It actually worked ? That's unexpected, I just wanted to send you down the rabbit hole so you'd be busy for a while.
And I just got a 👍 ?!! Even the Copilot AI is more grateful, with warm "You are absolutely right!".

Commodore64user · 2026-02-21T14:38:54Z

I have no idea if this would work with the html widget, but logically, you could have something cleaner with:

What ?! It actually worked ? That's unexpected, I just wanted to send you down the rabbit hole so you'd be busy for a while. And I just got a 👍 ?!! Even the Copilot AI is more grateful, with warm "You are absolutely right!".

so you want a somewhat obsequious response in a rather loquacious manner? "you absolute legend, they should make you archbishop of Canterbury or something."

Commodore64user · 2026-02-21T15:23:00Z

@poire-z, all this could be simply:

  args = function(text, hold_duration)
-     -- do this lookup in the same domain (dict/wikipedia)
-     local lookup_wikipedia = self.is_wiki
-     if hold_duration >= time.s(3) then
-         -- but allow switching domain with a long hold
-         lookup_wikipedia = not lookup_wikipedia
-     end
-
-     local new_dict_close_callback = function()
-         self:clearDictionaryHighlight()
-     end
-
-     -- We don't pass self.highlight to subsequent lookup, we want the
-     -- first to be the only one to unhighlight selection when closed
-     if lookup_wikipedia then
-         self:lookupWikipedia(false, text, nil, nil, new_dict_close_callback)
-     else
-         self.ui:handleEvent(Event:new("LookupWord", text, nil, nil, nil, nil, new_dict_close_callback))
-     end
+    -- short hold: same domain; long hold (>=3s): switch domain
+    self:_performLookupOnSelection(text, hold_duration >= time.s(3))
  end

Do you want me to add it here?

poire-z · 2026-02-21T15:55:34Z

Nothing against.
(But get rid of the leading _, and maybe find another name for the method, performLookupOnSelection is a bit too performative :) and it's no longer "on the selection", but is provided the text to lookup.)

poire-z · 2026-02-21T17:53:52Z

+function DictQuickLookup:searchDictionaryOrWikipedia(selected_text, switch_domain)
    if not selected_text then return false end
    local new_dict_close_callback = function() self:clearDictionaryHighlight() end
    local use_wiki = switch_domain and not self.is_wiki or not switch_domain and self.is_wiki


Convoluted and hard to grasp, the way it was done in the code you removed was a lot clearer.

happy with

local use_wiki = self.is_wiki if switch_domain then use_wiki = not use_wiki end

?

Very happy.

poire-z

Fine with me.
(I would still like a better name for searchDictionaryOrWikipedia - we don't really use the word "search" in there, mostly "lookup". :lookup() might be too simple/obvious for something that is not the real thing?, :doLookup ? :doNewLookup ?)

Commodore64user · 2026-02-24T14:56:23Z

do you still need a new name or was that just a 'hopefully we could...'? it is a little contradictory to give an "approved" but still sort of ask for changes...

poire-z · 2026-02-24T15:24:48Z

= I have no more comment about the logic and I'm fine with it, so I approved so you don't feel like I'm harrassing you every day :)
But if anybody who cares a bit about our naming (ie @Frenzie) have a better idea for that name, good, but it's not blocking.

Frenzie · 2026-02-24T15:52:46Z

What/where was the question exactly?

Commodore64user · 2026-02-24T15:57:11Z

self:searchDictionaryOrWikipedia(text, hold_duration >= time.s(3))

#15015 (review)

Frenzie · 2026-02-24T16:01:01Z

lernaLookup 👼

But yes, just a simple doLookup seems like it'd fit in best with the code?

Commodore64user · 2026-02-24T18:57:50Z

lookupDictionaryOrWikipedia?

Frenzie · 2026-02-24T19:05:09Z

I suppose.

poire-z · 2026-02-24T19:20:48Z

It's still quite verbose.
What matters is not really "Dictionary or Wikipedia" , we know we lookup one of these :)
I would maybe use doNewLookup - it feels the point is really the fact that it is a new lookup that is being done.

Commodore64user · 2026-02-24T19:23:01Z

they are all new lookups, what matters is the idea the the true will switch you from one to the other

poire-z · 2026-02-24T19:39:29Z

they are all new lookups

So, if that's the main thing it always does, then doNewLookup :)

what matters is the idea the the true will switch you from one to the other

So, the idea should be in that parameter name: doNewLookup(text, switch_dict_or_wikipedia).

Anyway, if you don't "feel" it, I don't really care, go ahead.

…ader#15015) * refactor handling of faked event HoldRelease for text selection on NT, make sure we close the cycle properly so we don't get any byte drift when dealing with non-ascii characters.

Use getSourceIndex() to convert char indices to byte indices before l…

0bef139

…ookups

Commodore64user requested a review from poire-z February 21, 2026 02:11

Commodore64user added bug NT Non Touch devices labels Feb 21, 2026

poire-z reviewed Feb 21, 2026

View reviewed changes

Comment thread frontend/ui/widget/dictquicklookup.lua Outdated

refactor handling of onHoldRelease on NT

4041577

Commodore64user changed the title ~~[DictQuickLookup] NT: convert char indices to byte indices~~ [DictQuickLookup] NT: fix utf8 byte drift during text selection Feb 21, 2026

Commodore64user requested a review from poire-z February 21, 2026 14:03

poire-z approved these changes Feb 21, 2026

View reviewed changes

rename function

5da1fd7

poire-z reviewed Feb 21, 2026

View reviewed changes

make @poire-z 'very happy'

5670973

poire-z approved these changes Feb 21, 2026

View reviewed changes

rename method

6210177

Commodore64user merged commit 34c7723 into koreader:master Feb 24, 2026
4 checks passed

Commodore64user added this to the 2026.02 milestone Feb 24, 2026

Commodore64user deleted the index-byte branch February 25, 2026 13:51

Uh oh!

Uh oh!

Conversation

Commodore64user commented Feb 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

bug fix

Uh oh!

Uh oh!

poire-z commented Feb 21, 2026

Uh oh!

poire-z Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Commodore64user Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

poire-z commented Feb 21, 2026

Uh oh!

Commodore64user commented Feb 21, 2026

Uh oh!

Commodore64user commented Feb 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

poire-z commented Feb 21, 2026

Uh oh!

poire-z Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Commodore64user Feb 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

poire-z Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

poire-z left a comment

Choose a reason for hiding this comment

Uh oh!

Commodore64user commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

poire-z commented Feb 24, 2026

Uh oh!

Frenzie commented Feb 24, 2026

Uh oh!

Commodore64user commented Feb 24, 2026

Uh oh!

Frenzie commented Feb 24, 2026

Uh oh!

Commodore64user commented Feb 24, 2026

Uh oh!

Frenzie commented Feb 24, 2026

Uh oh!

poire-z commented Feb 24, 2026

Uh oh!

Commodore64user commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

poire-z commented Feb 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Commodore64user commented Feb 21, 2026 •

edited

Loading

Commodore64user commented Feb 21, 2026 •

edited

Loading

Commodore64user Feb 21, 2026 •

edited

Loading

Commodore64user commented Feb 24, 2026 •

edited

Loading

Commodore64user commented Feb 24, 2026 •

edited

Loading