Bookmarks: fix sort within one page by hius07 · Pull Request #8616 · koreader/koreader

hius07 · 2022-01-05T17:16:34Z

Bookmarks of one page will be sorted depending on their position in the text (now they are sorted in order of adding).
Page bookmark is the first (before highlights and notes).
If two highlights start at one position, the shorter one will be the first.

This change is

poire-z · 2022-01-05T17:58:05Z

-        local a_page = self.ui.document:getPageFromXPointer(a.page)
-        local b_page = self.ui.document:getPageFromXPointer(b.page)
-        if a_page == b_page then -- have bookmarks before highlights
-            return a.highlighted
+        local compare_xp = self.ui.document:compareXPointers(a.page, b.page)
+        if compare_xp == 0 then -- both bookmarks with the same start


I have a litlte concern in here, with the many calls to compareXPointers(a,b) being expensive, at least this being more expensive than before:

getPageFromXPointer() is cached by the cre call cache (as most of the other get*() methods), while compareXPointers() is not - and if we were to cache the results of compareXPointers(a,b), we would cache N*N results if we have N highlights. The cacheing of getPageFromXPointer() for N highlights caches just N results.

This would be ok if isBookmarkInPageOrder() was called only when adding or removing bookmark/highlights (I don't care that this spends time at the moment we do it), but it seems to be called in a few other "just reading" calls, like setDogearVisibility() (on each page, to know if we should show the dogear) or getPreviousBookmarkedPage(), and may be others.

(same feeling may apply to the paging section above, switching out of reflow mode and doing more expensive work, not sure)

So, may be it needs a dedicated duplicate that you would call only when you need the accurate ordering - and may be keep this one as it was for the calls like setDogearVisibility that may not need the ordering accuracy ?
I also see you didn't update the next function, isBookmarkInReversePageOrder(), so there's some unbalance, that might be restored with a 3rd function that does the ordering accuracy.

compareXPointers(a,b) being expensive

So maybe at first compare pages, and only when two bookmarks are within one page, compare xpointers.

I also see you didn't update the next function, isBookmarkInReversePageOrder()

This one is used only in one place for

koreader/frontend/apps/reader/modules/readerpaging.lua

Lines 318 to 320 in c11ea4f

elseif direction == "west" then

self.ui:handleEvent(Event:new("GotoNextBookmark", flipping_page))

end

in flipping mode, so no need of accuracy within one page.

This one is used only in one place for

But also by this it seems:
skimtowidget.lua: self:goToByEvent("GotoNextBookmarkFromPage")

All this ReaderBookmark reorderning/insertion/binary search feels a bit crappy, thought we could get it simpler when migrating to annotations.
But my idea is that any navigation should be smooth and cheap, and only at add/remove bookmark/highlight we are allowed to do expensive stuff (expensive so it orders everything so that the others can then be smooth and cheap).

So maybe at first compare pages, and only when two bookmarks are within one page, compare xpointers.

Yes, that would be good - if you're sure about page being ok (might not be when created in reflow mode I beleive from what you wrote in your code?)

any navigation should be smooth and cheap, and only at add/remove bookmark/highlight we are allowed to do expensive stuff

Let's decide. isBookmarkInPageOrder is used 6 times:
fixBookmarkSort
addBookmark
isBookmarkAdded
removeBookmark

getDogearBookmarkIndex
getPreviousBookmarkedPage

The first group needs accuracy. You propose to make my isBookmarkInPositionOrder for it only, right?

The first group needs accuracy. You propose to make my isBookmarkInPositionOrder for it only, right?

Yes, exactly, that's the idea: a new function, that you would use where you needed to have stuff fixed.

isBookmarkInPageOrder / isBookmarkInReversePageOrder for quick navigation cheap stuff where only page is enough (mainly, comparing page in binary search)

isBookmarkInPositionOrder for fixBookmarkSort and addBookmark (which can do reordering/insertion).

isBookmarkAdded / removeBookmark don't do any insert/ordering, so it feels they could use the cheaper one (isBookmarkAdded is used only by DictQuickLookup).

When you first talked about this PR some days ago, I though you might need a one step migration: fixBookmarkSort() did a migration (and may do it for books not opened since long), so you might want to either make it better and have it be called again (if config:hasNot("bookmarks_sorted2")), or have another fixBookmarkSort2() so your isBookmarkInPositionOrder() can work on something already perfectly sorted (or may be it doesn't need it ? and will do the sorting? but then, not until another bookmark is added?).

Okay. What about removeBookmark? It makes the full scan when there are several bookmarks in a page.
Using isBookmarkInPositionOrder we can remove full scan code, it is not used (I've checked).

koreader/frontend/apps/reader/modules/readerbookmark.lua

Lines 903 to 906 in ec280f8

-- If we haven't found item, it may be because there are multiple

-- bookmarks on the same page, and the above binary search decided to

-- not search on one side of one it found on page, where item could be.

-- Fallback to do a full scan.

EDIT: no, cannot remove full scan code, but it would be used much rarely.

That means you would need to have your bookmarks correctly sorted with your new way, after an initial migration ?
But for removeBookmark(), I wonder if doing binary search and calling your (expensive) compareXPointers() might be slower than a full scan :) (but may be that's no more an issue as you first check the are identical before using it).

(I think it's me that added the fallback full scan, may be because I was in a situation that shouldn't happen once you have precisely sorted the bookmark ? Dunno.)

hius07 · 2022-01-05T19:18:54Z

Fixed an extremely edge case existing crash.
To reproduce: open pdf, set reflow on, select text, choose Select, set reflow off, select ending fragment, crash.

hius07 · 2022-01-06T10:06:07Z

3 calls of quick isBookmarkInPageOrder, 3 calls of thorough isBookmarkInPositionOrder.

poire-z · 2022-01-06T11:35:18Z

+    local box1 = self:getWordFromPosition(doc, ppos1).pbox
+    local box2 = self:getWordFromPosition(doc, ppos2).pbox
+    if box1.y == box2.y then


Dunno what ppos1 looks like, it doesn't have any y ? Or does it have a y but it may not be "current" ?

comparePositions function is used in ReaderBookmark and in ReaderHighlight (extended selection).
In ReaderHighlight one of the positions is the hold_pos, so at first we need to get a word under the long-press and then compare.
EDIT: I've refreshed memory of my development of extended selection: pos is not a wordbox upper left corner, but just a position of long-press-start or long-press-release. So we always need to get words under both positions at first.

OK, so you would need this only when you invoke comparePositions() from extendSelection() ?
When you invoke comparePositions() from isBookmarkInPositionOrder(), you know you have bookmarks that are not random screen coordinates, but are proper word boxes ?
So, may be it could have another parameter to say we need to go thru this or not?

Sorry, missed your comment.
pos0 and pos1 stored in pdf bookmarks are not box positions, but touch positions.
We always need to get a word and its box's x and y.

OK, well, then OK.
(I suppose the correct word/box are available in the bookmark counterpart in the "highlight" list, which has "pboxes" = [ {x/y/w/h}, {x/y/w/h}... ] ? Not asking you to go fetch them here, just wondering if these are correct, and it this would be solved if we would move the pboxes from highlight into bookmarks if one day, we get rid of "highlight".)

That's a good idea, pboxes are ordered and it is easy to get coordinates from them and compare.
We already have pboxes in some bookmarks (when adding a note, to save it in the pdf file).

In the meantime, currently pboxes are stored in highlights, but used only when saving highlight to pdf. If this option is disabled, pboxes are not needed.
Readerview does not use them to draw the highlights, just positions

koreader/frontend/apps/reader/modules/readerview.lua

Lines 484 to 485 in 7163a39

local pos0, pos1 = item.pos0, item.pos1

local boxes = self.ui.document:getPageBoxesFromPositions(page, pos0, pos1)

poire-z · 2022-01-06T11:42:03Z

    if config:hasNot("bookmarks_sorted") then
        table.sort(self.bookmarks, function(a, b)
-            return self:isBookmarkInPageOrder(a, b)
+            return self:isBookmarkInPositionOrder(a, b)


Don't you think you need to have this re-done once for past books, so you're sure all bookmarks are precisely sorted?
If not, and if we have past highlights in a same page in that order (let's say these are y) [10, 30, 20, 40], I'm not sure where your addBookmark may insert a new highlight at y=25: before the 30 or after the 20 ? and the order will stay being not ordered.
if config:hasNot("bookmarks_sorted_20220106") then ?

I propose to add a button Sort bookmarks to the extra button menu, to allow a user to sort if he needs (maybe he doesn't want).
And to remove bookmarks_sorted from doc_settings.

I propose to add a button Sort bookmarks to the extra button menu, to allow a user to sort if he needs (maybe he doesn't want).

Mhhh, no, this doesn't feel it should be a user choice. It's a technical internal ordering of the data structure, for efficiency purpose, to have an accurate binary search.
If "he doesn't want", I guess he wants to see bookmarks ordered by date added - but then it would still be by page, and only bookmarks inside a same page would be by "date added". So, a user facing button should be: Sort by page (and position inside a same page) / Sort by date added (with some new work for you) - but I think sort by date added is a bit useless :)

And to remove bookmarks_sorted from doc_settings.

And to what aim ?:

still calling fixBookmarkSort and have table.sort(...isBookmarkInPositionOrder) done on each book opening ? (feels like an expensive no-op after you have done it once, and you have addBoookmark always insert correctly.)

or not doing any sorting fix at all ?

or not doing any sorting fix at all

I thought of that, and a button for manual sorting. OK, dropped.

config:hasNot("bookmarks_sorted_20220106")

Yes, I like the idea, re-sort all.

hius07 · 2022-01-06T12:25:41Z

What do you think about highlighting images?

koreader/frontend/apps/reader/modules/readerhighlight.lua

Lines 1588 to 1605 in ec280f8

    
           --[[ 
        
           function ReaderHighlight:exportToClippings(page, item) 
        
               logger.dbg("export highlight to clippings", item) 
        
               local clippings = io.open("/mnt/us/documents/My Clippings.txt", "a+") 
        
               if clippings and item.text then 
        
                   local current_locale = os.setlocale() 
        
                   os.setlocale("C") 
        
                   clippings:write(self.document.file:gsub("(.*/)(.*)", "%2").."\n") 
        
                   clippings:write("- KOReader Highlight Page "..page.." ") 
        
                   clippings:write("| Added on "..os.date("%A, %b %d, %Y %I:%M:%S %p\n\n")) 
        
                   -- My Clippings only holds one line of highlight 
        
                   clippings:write(item["text"]:gsub("\n", " ").."\n") 
        
                   clippings:write("==========\n") 
        
                   clippings:close() 
        
                   os.setlocale(current_locale) 
        
               end 
        
           end 
        
           --]]

It is connected with #579 and

koreader/plugins/exporter.koplugin/clip.lua

Lines 267 to 279 in ec280f8

    
           if item.text == "" and item.pos0 and item.pos1 and 
        
                   item.pos0.x and item.pos0.y and 
        
                   item.pos1.x and item.pos1.y then 
        
               -- highlights in reflowing mode don't have page in pos 
        
               if item.pos0.page == nil then item.pos0.page = page end 
        
               if item.pos1.page == nil then item.pos1.page = page end 
        
               local image = {} 
        
               image.file = book.file 
        
               image.pos0, image.pos1 = item.pos0, item.pos1 
        
               image.pboxes = item.pboxes 
        
               image.drawer = item.drawer 
        
               clipping.image = self:getImage(image) 
        
           end

but currently we cannot do this. Is it needed? Maybe, remove all this old stuff?

poire-z · 2022-01-06T12:38:28Z

What do you think about highlighting images?

Not much, I didn't even know that was possible with PDF :) @mustafa-001 seemed to care about clipping images in #5794.
As for crengine, the text selection and xpointer stuff don't work with images, or they skip images - and I don't feel like having to dig into crengine/cre.cpp to be able to highlight images :)

hius07 · 2022-01-06T12:42:16Z

I'm going to fix Exporter plugin sorting order. Maybe remove this export of "highlighted image" that doesn't work at all? And this commented code from readerhighlight?

poire-z · 2022-01-06T12:54:02Z

Dunno, I'm usually for not touching stuff that does not originate from me :)
For this image highlight stuff, may be some day someone will want to work on it, and having past vestiges commented out around will be of help to know where things were happening and should be worked on ?
But well, as you're adopting this poor forgotten Exporter plugin, it's just up to you :)

hius07 · 2022-01-06T16:46:15Z

I enabled one-time accurate sorting for the test book (von Mises, 282 bms), it took about 10 sec on my Kindle to re-sort them, all perfectly.

poire-z · 2022-01-06T18:27:11Z

-    self.ui.doc_settings:makeTrue("bookmarks_sorted")
+    self.ui.doc_settings:delSetting("bookmarks_sorted")
+    self.ui.doc_settings:makeTrue("bookmarks_sorted_20220106")


I would let self.ui.doc_settings:makeTrue("bookmarks_sorted") and don't mind it.
We don't really need to have docsettings that tidy :)
But more importantly, think about people trying an older KOReader version (for comparing behaviour, regressions...):

they have a document with bookmarks_sorted, upgrade, get bookmarks_sorted_20220106, and get bookmarks_sorted removed

they switch back to v2021.10: the old fixBookmark is run (because bookmarks_sorted is not there) which might re-order your nice ordering.
(I don't mind much how bookmark behave on v2021.10, we should just try to be sure it doesn't crash)

they switch back to v2022.02: you don't run your nice re-ordering because bookmarks_sorted_20220106 is there, but you have badly re-ordered bookmarks as done by v2021.10 (and you get reports for bugs and issues you have a hard time understanding :)

poire-z · 2022-01-09T08:44:19Z

Unrelated, just mentionning this in the latest bookmark PR:
Last evening, I was making some highlights: select text, add note, and I ended up in a situation where Add note did nothing. I ended up in a situation where I had in bookmarks:

        [3] = {
            ["notes"] = "ces meubles",
            ["pos0"] = "/body/DocFragment[9]/body/p[135]/text().2586",
            ["pos1"] = "/body/DocFragment[9]/body/p[135]/text().2597",
            ["highlighted"] = true,
            ["page"] = "/body/DocFragment[9]/body/p[135]/text().2586",
            ["chapter"] = "5",
            ["datetime"] = "2022-01-09 02:04:51",
        },

and no equivalent in highlight - and in (metadata.epub.lua.old) a step before I "tapped on highlight > Delete", I had an item in highlight with the same xpointers, but dated 02:07:22, so not the same datetime, which explained why Add note didn't work.
I don't know how they ended up desynced about the datetime, and I can't reproduce after I clean it all and try to highlight the same text.
This was with a KOReader without the commit from this PR - and on a recently opened book with only 2 other small highlights/bookmarks on other pages which had no issue.
Just mentionning it in case you can imagine what happened :)

hius07 · 2022-01-09T12:59:08Z

Cannot imagine. Datetime is saved once when creating a highlight/bookmark and is never changed.
os.date is requested separately for a highlight and for a corresponding bookmark (do not know why) in saveHighlight

koreader/frontend/apps/reader/modules/readerhighlight.lua

Line 1563 in 7117225

local datetime = os.date("%Y-%m-%d %H:%M:%S")

which then calls getHighlightBookmarkItem, and in it:

koreader/frontend/apps/reader/modules/readerhighlight.lua

Line 1539 in 7117225

local datetime = os.date("%Y-%m-%d %H:%M:%S")

This cannot give 3 sec difference (and time request for hl is first, earlier by microseconds).

poire-z · 2022-01-09T13:10:11Z

os.date is requested separately for a highlight and for a corresponding bookmark (do not know why) in saveHighlight

Sounds like an oversight and a bug - and may explain why it's rare and me doing this highlight at maybe 02:04:51.999 was bad luck :)

This cannot give 3 sec difference

Yes, but may be 3 minutes differences I observed was because there may have been a 1 second difference, and then when trying to play around, I deleted the highlight (which because of the 1 second difference, did not get the bookmark deleted), then remade it 3 mn later - and I may have ending up with 2 bookmarks and 1 highlight for the same thing, one bookmark among the 2 having no highlight buddy.

hius07 · 2022-01-09T13:19:23Z

Sounds like an oversight and a bug

Maybe the reason is

koreader/frontend/ui/widget/dictquicklookup.lua

Lines 917 to 920 in a1cbc3b

    
           function DictQuickLookup:getHighlightedItem() 
        
               if self:isDocless() then return end 
        
               return self.ui.highlight:getHighlightBookmarkItem() 
        
           end

poire-z · 2022-01-09T13:36:04Z

(I don't remember having used the dictquicklookup "Highlight" button, if I get that this is used by it - actually, I hack this button out in my own koreader patch :) so I'm sure I didn't hit it.)

poire-z · 2022-01-09T13:39:03Z

If by "the reason" you mean why one has to compute a datetime because it could be used standalone, when not used standalone, a datetime could be passed around so it is used instead, and computed only when nothing is passed. (seems kludgy, but well, that's our datastructure :)

poire-z · 2022-01-18T14:05:02Z

Got a crash when opening some old test document, so possibly edited and having bookmark/xpointers no longer found in the document (I think I mentionned that case in another issue):

[!] doShowReader coroutine crashed:
frontend/apps/reader/modules/readerbookmark.lua:241: attempt to compare nil with number
stack traceback:
 frontend/apps/reader/modules/readerbookmark.lua:298: in function 'fixBookmarkSort'
 frontend/apps/reader/modules/readerbookmark.lua:356: in function 'v'
 frontend/apps/reader/readerui.lua:449: in function 'init'
 frontend/ui/widget/widget.lua:48: in function 'new'
 frontend/apps/reader/readerui.lua:620: in function 'doShowReader'

logging what we got:

            local compare_xp = self.ui.document:compareXPointers(a.page, b.page)
+           logger.warn(a.page, b.page, compare_xp)

01/18/22-15:00:24 WARN  /html/body/p[1]/text().0 /html/body/p[6]/i/text().0 1
01/18/22-15:00:24 WARN  /html/body/p[6]/bdo/text().21 /html/body/p[6]/i/text().0
[!] doShowReader coroutine crashed:
frontend/apps/reader/modules/readerbookmark.lua:242: attempt to compare nil with number

And indeed, in the C function, we may return nil when a xpointer is not found:

static int compareXPointers(lua_State *L){
    CreDocument *doc = (CreDocument*) luaL_checkudata(L, 1, "credocument");
    const char* xp1 = luaL_checkstring(L, 2);
    const char* xp2 = luaL_checkstring(L, 3);
    ldomXPointerEx nodep1 = doc->dom_doc->createXPointer(lString32(xp1));
    ldomXPointerEx nodep2 = doc->dom_doc->createXPointer(lString32(xp2));
    if (nodep1.isNull() || nodep2.isNull())
        return 0;   -- returning nil when xpointer not found
    // Return 1 if pointers are ordered (if xp2 is after xp1), -1 if not, 0 if same
    lua_pushinteger(L, nodep2.compare(nodep1));
    return 1;
}

Dunno how we'd like to handle this case, sorting wise - but I think we should not crash, and if possible, not remove the wrong bookmark (as one might edit and restore the document to the previous good state, and would like to find again his bookmarks).

hius07 · 2022-01-18T16:30:21Z

Can we check if xpointer is correct?
If so, add a check for valid xpointer somewhere here

koreader/frontend/apps/reader/modules/readerbookmark.lua

Lines 231 to 234 in 87dbb98

    
           local a_page = self.ui.document:getPageFromXPointer(a.page) 
        
           local b_page = self.ui.document:getPageFromXPointer(b.page) 
        
           if a_page == b_page then -- both bookmarks in the same page 
        
               local compare_xp = self.ui.document:compareXPointers(a.page, b.page)

and if not, compare pages only?

poire-z · 2022-01-18T16:46:23Z

We can:

         if a_page == b_page then -- both bookmarks in the same page
             local compare_xp = self.ui.document:compareXPointers(a.page, b.page)
+            if compare_xp == nil then -- some xpointer not found (?)
+                logger.warn(a.page, b.page, a_page, b_page)
+                logger.warn(a.page, self.ui.document:isXPointerInDocument(a.page))
+                logger.warn(b.page, self.ui.document:isXPointerInDocument(b.page))
+            end
             if compare_xp == 0 then -- both bookmarks with the same start

01/18/22-17:41:07 WARN  /html/body/p[6]/bdo/text().21 /html/body/p[6]/i/text().0 1 1
01/18/22-17:41:07 WARN  /html/body/p[6]/bdo/text().21 false
01/18/22-17:41:07 WARN  /html/body/p[6]/i/text().0 true

But I'd rather do it later than line 231, only in the rare case we find out compare_xp didn't work and that one (or both) of the compared xpointer is invalid.

Note that for invalid xpointers, self.ui.document:getPageFromXPointer(a.page) returns page 1 ... (some old code, didn't ever need to have this fixed, hoping I don't have to :) or else there's risk for other things to break.)

So, given that these invalid xpointers (that most people will not have) get to be announced as being on page 1, may be sort them before all other valid ones (those really on page 1, but most of them on pages 1++).

hius07 · 2022-01-18T17:49:43Z

So it can be

        local a_page = self.ui.document:getPageFromXPointer(a.page)
        local b_page = self.ui.document:getPageFromXPointer(b.page)
        if a_page == b_page then -- both bookmarks in the same page
            local compare_xp = self.ui.document:compareXPointers(a.page, b.page)
            if compare_xp then
                if compare_xp == 0 then -- both bookmarks with the same start
                    if a.highlighted and b.highlighted then -- both are highlights, compare ends
                        return self.ui.document:compareXPointers(a.pos1, b.pos1) < 0
                    end
                    return a.highlighted -- have page bookmarks before highlights
                end
                return compare_xp < 0
            end
        end
        return a_page > b_page

Invalid bookmarks will be sorted to the page 1. After correcting the book, a user will need to resort bookmarks to restore their correct positions, so need to delete bookmarks_sorted_20220106 key from the doc settings manually.
Or, if not compare_xp we can set a new key in the doc settings bookmarks_invalid and resort every time when opening the book, until correcting?

poire-z · 2022-01-18T18:16:20Z

Oh, that simple :) Good!
(With a comment below the end
-- if compare_xp is nil, some xpointer is invalid and will be sorted first to page 1 )

Or, if not compare_xp we can set a new key in the doc settings bookmarks_invalid and resort every time when opening the book, until correcting?

Well, I guess we don't need to really bother with that. Again, it's rare, and people like me messing with content get what they deserve :) (except a crash was more than what they deserved :)
Remind me what will be the effect of having bookmarks_sorted_20220106 set but bookmarks actually not very well sorted (with the invalid ones at sort time being put first) ? Just future insertion being misordered, and deletion/edit sometimes not working as binary search may not find it? Anything else?
I'd say for such old edited books, what matters mostly is drawing past highlight (when they are not invalid), and showing them in bookmarks list (even when invalid, I think that's currently the case).

Unless you feel it's quickly doable and doesn't hurt (a logger.info() telling an invalid was found will help noticing if this happens when it should not).

hius07 · 2022-01-18T20:39:06Z

effect of having bookmarks_sorted_20220106 set but bookmarks actually not very well sorted

removeBookmark fallbacks to full scan, so we can always remove existing bookmark.
renameBookmark always makes a full scan.
addBookmark makes binary search only, so the worst case would be that bookmarks within one page are not well sorted.

poire-z · 2022-01-18T20:50:12Z

OK, so not much issue with doing nothing ? :)
Except your if compare_xp then wrapping, to avoid the crash. Go for that!

Accurate sorting of bookmarks located in one page depending on their positions in text.

hius07 added 5 commits January 5, 2022 19:06

koptinterface: compare positions

71d42fd

pdfdocument: compare positions

b2eee6d

djvudocument: compare positions

73173b4

readerhighlight: compare positions

b41ed35

readerbookmark: fix sort within one page

a35158e

poire-z mentioned this pull request Jan 5, 2022

[CI] Fix CircleCI context issue by separating out docs & translations #8614

Merged

poire-z reviewed Jan 5, 2022

View reviewed changes

hius07 added 2 commits January 5, 2022 21:06

compare xpointers only when needed

dce4545

readerhighlight: fix select crash in reflow mode

ece25ae

isBookmarkInPositionOrder

c3eacdc

poire-z reviewed Jan 6, 2022

View reviewed changes

hius07 added 2 commits January 6, 2022 18:36

koptinterface: comment

feed19a

readerbookmark: one-time accurate sorting

1d9eda5

poire-z reviewed Jan 6, 2022

View reviewed changes

readerbookmark: keep bookmarks_sorted

9069f8c

poire-z approved these changes Jan 6, 2022

View reviewed changes

Frenzie added this to the 2022.01 milestone Jan 6, 2022

hius07 merged commit 00b08d7 into koreader:master Jan 6, 2022

hius07 deleted the BM-sort-in-one-page branch January 6, 2022 19:54

hius07 mentioned this pull request Jan 19, 2022

Bookmarks: fix compare invalid xpointers #8690

Merged

This was referenced Jan 21, 2022

Incorrect order of extracted highlights #8601

Closed

Changing the font size breaks the order of bookmarks when exporting #7469

Closed

ichnilatis-gr mentioned this pull request Apr 11, 2022

Incorrect order of extracted highlights #8982

Closed

poire-z mentioned this pull request Apr 16, 2022

Maintain Highlight Order #8996

Closed

0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026

Bookmarks: fix sort within one page (koreader#8616)

ddad424

Accurate sorting of bookmarks located in one page depending on their positions in text.

	elseif direction == "west" then
	self.ui:handleEvent(Event:new("GotoNextBookmark", flipping_page))
	end

	-- If we haven't found item, it may be because there are multiple
	-- bookmarks on the same page, and the above binary search decided to
	-- not search on one side of one it found on page, where item could be.
	-- Fallback to do a full scan.

	local pos0, pos1 = item.pos0, item.pos1
	local boxes = self.ui.document:getPageBoxesFromPositions(page, pos0, pos1)

Uh oh!

Uh oh!

Conversation

hius07 commented Jan 5, 2022 • edited by Frenzie 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!

hius07 Jan 5, 2022 • 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!

Choose a reason for hiding this comment

Uh oh!

hius07 Jan 5, 2022 • 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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hius07 commented Jan 5, 2022

Uh oh!

hius07 commented Jan 6, 2022

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hius07 Jan 6, 2022 • 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!

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!

poire-z Jan 6, 2022 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hius07 commented Jan 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

poire-z commented Jan 6, 2022

Uh oh!

hius07 commented Jan 6, 2022

Uh oh!

poire-z commented Jan 6, 2022

Uh oh!

hius07 commented Jan 6, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

poire-z commented Jan 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hius07 commented Jan 9, 2022

Uh oh!

poire-z commented Jan 9, 2022

Uh oh!

hius07 commented Jan 9, 2022

hius07 commented Jan 5, 2022 •

edited by Frenzie

Loading

hius07 Jan 5, 2022 •

edited

Loading

hius07 Jan 5, 2022 •

edited

Loading

hius07 Jan 6, 2022 •

edited

Loading

poire-z Jan 6, 2022 •

edited

Loading

hius07 commented Jan 6, 2022 •

edited

Loading

poire-z commented Jan 9, 2022 •

edited

Loading