[ReaderHighlight, Config, Menu] NT: stop text selection before any reflow action occurs by Commodore64user · Pull Request #14949 · koreader/koreader

Commodore64user · 2026-02-07T16:36:34Z

what's new

Added calls to self.ui.highlight:StopHighlightIndicator() in ReaderConfig:onShowConfigMenu(), ReaderMenu:onShowMenu(), and HotKeys:onHotkeyAction() to ensure text selection is stopped before any reflow actions that could invalidate highlight coordinates.
Updated ReaderHighlight:onStopHighlightIndicator() to return early if there is no current indicator position, preventing unnecessary processing and errors.

This change is

poire-z · 2026-02-07T17:26:43Z

 function ReaderMenu:onShowMenu(tab_index, do_not_show)
+    -- Note: on devices with DPad, we could start text selection and then trigger a reflow
+    --       (e.g., change typeface) which will cause pandemonium to ensue (invalid coordinates).
+    self.ui:handleEvent(Event:new("StopHighlightIndicator", {args=true}))


Feels a bit a too quick solution to "infect" these other modules with something quite unrelated (currently, StopHighlightIndicator is used only in readerhighlight).
May be you could, in readerhighlight, if ifHasDpad, add (ie. in init(), like the one we discovered in InputText) handlers for these events (self.onShowMenu = function() self:onStopHighlightIndicator(true) end). It may work as readerhighlight is loaded before many other modules in readerui.

It may work

it doesn't

But maybe a wrapper could be used, i'll have a look later as i have places to be now ;)

(I could not find a good idea, but have a go. But a user-patch-like wrapper might be a bit ugilier than your current solution.)

hius07 · 2026-02-07T20:12:49Z

Can the indicator be stopped in the ReaderHighlight:onDocumentRerendered() handler?

Commodore64user · 2026-02-07T22:38:53Z

Although reflow is the main concern, is not just that. The crosshairs need to be stopped before going anywhere else really, book map, toc, etc.

Edit: for a second i was wondering, why is @poire-z giving a thumbs up to his own post? Then i realised the truth ;)

poire-z · 2026-02-07T22:55:00Z

Although reflow is the main concern, is not just that. The crosshairs need to be stopped before going anywhere else really, book map, toc, etc.

So, we need to find another solution...
Was going to write: And how can we get there ? Only via your hotkey.koplugin, so it could be it that would send an event on any keypress. But I realize the emulator has that crosshair, so we could use gestures... So, not the complete solution... Writing this in case it can initiate other ideas...

Commodore64user · 2026-02-07T23:40:13Z

Have you got any other brilliant ideas though? This seems the lesser of two evils. And I don’t even see how it’s “infecting” anything.

poire-z · 2026-02-08T00:01:42Z

If you need it for ToC, Bookmap... you'll need to put your self.ui:handleEvent(Event:new("StopHighlightIndicator", {args=true})) in many other handlers (ie. onShowToc), as we can trigger them with a gesture without going thru the top menu.
That's not infection but infestation.

And with your current solution, on any touch device, we would get that event send to all the modules for nothing (sending events costs a bit of cpu).

No brilliant idea, but it's your PR and for you to find it :)

Commodore64user · 2026-02-08T00:43:00Z

I have deliberately kept Gestures out of this as I believe it is much more unlikely for someone to combine text selection through DPad and Gestures.. in any case, it would be as simple as sending another event similar to Hotkeys..
incidentally we, touch-starved-device owners have to tolerate this self.ui:handleEvent(Event:new("HandledAsSwipe")) -- cancel any pan scroll made that is wasting my cpu

No brilliant idea, but it's your PR and for you to find it :)

already did ;)

poire-z · 2026-02-08T08:32:36Z

incidentally we, touch-starved-device owners have to tolerate this self.ui:handleEvent(Event:new("HandledAsSwipe")) -- cancel any pan scroll made that is wasting my cpu

Right, sorry for that :)

Anyway, I find the wrapping uglier.
My Event for HandledAsSwipe was an event because there may be multiple possible catcher (rolling and panning).
As your only catcher is in readerhighlight, and readerhighlight is always loaded, you could just use
self.ui.highlight:StopHighlightIndicator(true) -- cancel any DPAD in-progress text selection
(We access self.ui.highlight like that in many other Reader* modules, so it wouldn't feel odd.)

Commodore64user · 2026-02-08T13:16:56Z

    end

    main_menu.touch_menu_callback = function ()
        self.ui:handleEvent(Event:new("CloseConfigMenu"))


even here we still send an event.. but whatever

I agree this one does not need to be an event, its name is the name of its target, we could call it directly. (But not for you to do in this PR.)
(Events are good for a generic... event, that may interest multiple modules (even if there is currently only one catcher, thinking about the future if the event is generic)

Commodore64user · 2026-02-08T13:21:35Z

and, I am not much interested in how you handle wiping you' ass but #FreeTheCPU ;)

poire-z · 2026-02-08T14:09:23Z

 function HotKeys:onHotkeyAction(hotkey)
+    -- Note: we could have started text selection and then trigger a reflow through hotkeys (e.g., increase
+    --       font-size) which will cause pandemonium to ensue (invalid coordinates).
+    self.ui:handleEvent(Event:new("StopHighlightIndicator", {args=true}))


Your plugin can also access self.ui.highlight.

yes, but that seems a lot less clean...

You are already not the only one (if the plugin works when in filemanager, it should additionally have if self.ui.highlight then):

plugins/qrclipboard.koplugin/main.lua: if self.ui.highlight then plugins/qrclipboard.koplugin/main.lua: self.ui.highlight:addToHighlightDialog("12_generate_qr_code", function(this) plugins/hotkeys.koplugin/main.lua: self.ui.highlight.key_events.StartHighlightIndicator = nil -- remove 'H' shortcut used for highlight indicator plugins/vocabbuilder.koplugin/main.lua: if settings.with_context and self.ui.highlight then plugins/vocabbuilder.koplugin/main.lua: prev_context, next_context = self.ui.highlight:getSelectedWordContext(15) plugins/vocabbuilder.koplugin/main.lua: if self.ui.highlight.selected_text and self.ui.highlight.selected_text.text then plugins/vocabbuilder.koplugin/main.lua: highlight = util.cleanupSelectedText(self.ui.highlight.selected_text.text)

I am guilty of the qr one, but then again that is adding an element to that menu. the hotkeys ones have been the hacks to solve the overriding, and now to move the hardcoded shortcuts you are making me move there ;)

But this is absolutely not an issue. You were guily of nothing.
But you'd be guilty now to use an event when the canonic thing (what I've shown :)) is to access self.ui.highlight directly, including in plugins.

tight coupling, don't talk to strangers (Law of something or other), off the top of my head

still here?

Yes.
(These principles are good!)

I don't know man, seems like reaching across the dinner table and grabbing someone else's wine, but okay ;), if that's the French style, well..

Bisous 😘

Commodore64user · 2026-02-16T13:57:06Z

As this is a first, heads up, squashing and merging in five 😬

poire-z · 2026-02-16T14:38:24Z

As this was a first, some advices:
keep this short, we don't need to know about each individual commits/adaptations/corrections that were done while reviewing, remove uneeded blank lines between items

Commodore64user · 2026-02-16T17:48:11Z

I feel like i disappointed granddad. ;)

poire-z · 2026-02-16T17:59:34Z

You did, but I can take it :)

SethMilliken · 2026-02-18T22:07:35Z

 function HotKeys:onHotkeyAction(hotkey)
+    -- Note: we could have started text selection and then trigger a reflow through hotkeys (e.g., increase
+    --       font-size) which will cause pandemonium to ensue (invalid coordinates).
+    if self.ui.highlight then self.ui.highlight:stopHighlightIndicator(true) end


Shouldn't this be invoking self.ui.highlight:onStopHighlightIndicator(true)?

As-is, I get:

02/18/26-17:00:49 ERROR An error occurred while executing a handler: plugins/hotkeys.koplugin/main.lua:92: attempt to call method 'stopHighlightIndicator' (a nil value) hotkeys:onHotkeyAction stack traceback: plugins/hotkeys.koplugin/main.lua:92: in function <plugins/hotkeys.koplugin/main.lua:89> [C]: in function 'xpcall' frontend/pluginloader.lua:97: in function 'handleEvent' frontend/ui/widget/container/widgetcontainer.lua:83: in function 'propagateEvent' frontend/ui/widget/container/widgetcontainer.lua:101: in function 'handleEvent' frontend/ui/uimanager.lua:910: in function 'sendEvent' frontend/ui/uimanager.lua:53: in function '__default__' frontend/ui/uimanager.lua:1437: in function 'handleInputEvent' frontend/ui/uimanager.lua:1537: in function 'handleInput' frontend/ui/uimanager.lua:1581: in function 'run' reader.lua:294: in main chunk

YEESSS, how did that happen?

fixed now, thank you. fix should be in the next nightly.

* refactor onStopHighlightIndicator so it returns early * stop text selection when opening menus or triggering hotkeys * use self.ui.highlight (naturally) * remove duplicated setDirty

Commodore64user added 2 commits February 7, 2026 16:12

refactor onStopHighlightIndicator so it returns early

f81a455

stop text selection when opening menus ot triggering hotkeys

a5c05cc

poire-z reviewed Feb 7, 2026

View reviewed changes

wrap menu functions during onReaderReady

7037b78

use self.ui.highlight (naturally)

667e566

Commodore64user commented Feb 8, 2026

View reviewed changes

poire-z reviewed Feb 8, 2026

View reviewed changes

Commodore64user added 3 commits February 8, 2026 15:26

hotkeys sullied

83a5a4e

remove Event requirement

ff86fb6

remove duplicated setDirty

400cca8

poire-z approved these changes Feb 10, 2026

View reviewed changes

Commodore64user added the NT Non Touch devices label Feb 13, 2026

Commodore64user merged commit 09982fe into koreader:master Feb 16, 2026
4 checks passed

Commodore64user added this to the 2026.02 milestone Feb 16, 2026

Commodore64user deleted the stop-select branch February 16, 2026 14:28

SethMilliken reviewed Feb 18, 2026

View reviewed changes

Uh oh!

Uh oh!

Conversation

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

Uh oh!

what's new

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hius07 commented Feb 7, 2026

Uh oh!

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

Uh oh!

Uh oh!

poire-z commented Feb 7, 2026

Uh oh!

Commodore64user commented Feb 7, 2026

Uh oh!

poire-z commented Feb 8, 2026

Uh oh!

Commodore64user commented Feb 8, 2026

Uh oh!

poire-z commented Feb 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Commodore64user commented Feb 16, 2026

Uh oh!

Uh oh!

poire-z commented Feb 16, 2026

Uh oh!

Commodore64user commented Feb 16, 2026

Uh oh!

poire-z commented Feb 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Commodore64user commented Feb 7, 2026 •

edited

Loading

Commodore64user commented Feb 7, 2026 •

edited

Loading

Commodore64user commented Feb 8, 2026 •

edited

Loading