Bookmarks: fix sort within one page (#8616)

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

frontend/apps/reader/modules/readerbookmark.lua

@@ -195,6 +195,55 @@ function ReaderBookmark:isBookmarkInTimeOrder(a, b)
     return a.datetime > b.datetime
 end
 
+function ReaderBookmark:isBookmarkInPositionOrder(a, b)
+    if self.ui.paging then
+        if a.page == b.page then -- both bookmarks in the same page
+            if a.highlighted and b.highlighted then -- both are highlights, compare positions
+                local is_reflow = self.ui.document.configurable.text_wrap -- save reflow mode
+                -- reflow mode doesn't set page in positions
+                a.pos0.page = a.page
+                a.pos1.page = a.page
+                b.pos0.page = b.page
+                b.pos1.page = b.page
+                self.ui.document.configurable.text_wrap = 0 -- native positions
+                -- sort start and end positions of each highlight
+                local compare_pos, a_start, a_end, b_start, b_end, result
+                compare_pos = self.ui.document:comparePositions(a.pos0, a.pos1) > 0
+                a_start = compare_pos and a.pos0 or a.pos1
+                a_end = compare_pos and a.pos1 or a.pos0
+                compare_pos = self.ui.document:comparePositions(b.pos0, b.pos1) > 0
+                b_start = compare_pos and b.pos0 or b.pos1
+                b_end = compare_pos and b.pos1 or b.pos0
+                -- compare start positions
+                compare_pos = self.ui.document:comparePositions(a_start, b_start)
+                if compare_pos == 0 then -- both highlights with the same start, compare ends
+                    result = self.ui.document:comparePositions(a_end, b_end) < 0
+                else
+                    result = compare_pos < 0
+                end
+                self.ui.document.configurable.text_wrap = is_reflow -- restore reflow mode
+                return result
+            end
+            return a.highlighted -- have page bookmarks before highlights
+        end
+        return a.page > b.page
+    else
+        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 == 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
+        return a_page > b_page
+    end
+end
+
 function ReaderBookmark:isBookmarkInPageOrder(a, b)
     if self.ui.document.info.has_pages then
         if a.page == b.page then -- have bookmarks before highlights
@@ -244,9 +293,10 @@ end
 function ReaderBookmark:fixBookmarkSort(config)
     -- for backward compatibility, since previously bookmarks for credocuments
     -- are not well sorted. We need to do a whole sorting for at least once.
-    if config:hasNot("bookmarks_sorted") then
+    -- 20220106: accurate sorting with isBookmarkInPositionOrder
+    if config:hasNot("bookmarks_sorted_20220106") then
         table.sort(self.bookmarks, function(a, b)
-            return self:isBookmarkInPageOrder(a, b)
+            return self:isBookmarkInPositionOrder(a, b)
         end)
     end
 end
@@ -311,6 +361,7 @@ end
 function ReaderBookmark:onSaveSettings()
     self.ui.doc_settings:saveSetting("bookmarks", self.bookmarks)
     self.ui.doc_settings:makeTrue("bookmarks_sorted")
+    self.ui.doc_settings:makeTrue("bookmarks_sorted_20220106")
     self.ui.doc_settings:makeTrue("highlights_imported")
 end
 
@@ -833,7 +884,7 @@ function ReaderBookmark:addBookmark(item)
             logger.warn("skip adding duplicated bookmark")
             return
         end
-        if self:isBookmarkInPageOrder(item, self.bookmarks[_middle]) then
+        if self:isBookmarkInPositionOrder(item, self.bookmarks[_middle]) then
             _end, direction = _middle - 1, 0
         else
             _start, direction = _middle + 1, 1
@@ -894,7 +945,7 @@ function ReaderBookmark:removeBookmark(item, reset_auto_text_only)
                 self.view.footer:onUpdateFooter(self.view.footer_visible)
             end
             return
-        elseif self:isBookmarkInPageOrder(item, v) then
+        elseif self:isBookmarkInPositionOrder(item, v) then
             _end = _middle - 1
         else
             _start = _middle + 1

frontend/apps/reader/modules/readerhighlight.lua

@@ -1737,24 +1737,16 @@ function ReaderHighlight:extendSelection()
     -- getting starting and ending positions, text and pboxes of extended highlight
     local new_pos0, new_pos1, new_text, new_pboxes
     if self.ui.document.info.has_pages then
-        local is_reflow = self.ui.document.configurable.text_wrap == 1
+        local is_reflow = self.ui.document.configurable.text_wrap
         local new_page = self.hold_pos.page
         -- reflow mode doesn't set page in positions
-        if is_reflow then
-            item1.pos0.page = new_page
-            item1.pos1.page = new_page
-            item2_pos0.page = new_page
-            item2_pos1.page = new_page
-        end
+        item1.pos0.page = new_page
+        item1.pos1.page = new_page
+        item2_pos0.page = new_page
+        item2_pos1.page = new_page
         -- pos0 and pos1 are not in order within highlights, hence sorting all
         local function comparePositions (pos1, pos2)
-            local box1 = self.ui.document:getWordFromPosition(pos1).pbox
-            local box2 = self.ui.document:getWordFromPosition(pos2).pbox
-            if box1.y == box2.y then
-                return box1.x < box2.x
-            else
-                return box1.y < box2.y
-            end
+            return self.ui.document:comparePositions(pos1, pos2) == 1
         end
         local positions = {item1.pos0, item1.pos1, item2_pos0, item2_pos1}
         self.ui.document.configurable.text_wrap = 0 -- native positions
@@ -1764,7 +1756,7 @@ function ReaderHighlight:extendSelection()
         local text_boxes = self.ui.document:getTextFromPositions(new_pos0, new_pos1)
         new_text = text_boxes.text
         new_pboxes = text_boxes.pboxes
-        self.ui.document.configurable.text_wrap = is_reflow and 1 or 0 -- restore reflow
+        self.ui.document.configurable.text_wrap = is_reflow -- restore reflow
         -- draw
         self.view.highlight.temp[new_page] = self.ui.document:getPageBoxesFromPositions(new_page, new_pos0, new_pos1)
     else

frontend/document/djvudocument.lua

@@ -68,6 +68,10 @@ function DjvuDocument:getProps()
     return props
 end
 
+function DjvuDocument:comparePositions(pos1, pos2)
+    return self.koptinterface:comparePositions(self, pos1, pos2)
+end
+
 function DjvuDocument:getPageTextBoxes(pageno)
     return self._document:getPageText(pageno)
 end

frontend/document/koptinterface.lua

@@ -1212,7 +1212,6 @@ function KoptInterface:getTextFromNativePositions(doc, native_boxes, pos0, pos1)
     return text_boxes
 end
 
-
 --[[--
 Get text boxes from page positions.
 --]]
@@ -1239,6 +1238,26 @@ function KoptInterface:getPageBoxesFromPositions(doc, pageno, ppos0, ppos1)
     end
 end
 
+--[[--
+Compare positions within one page.
+Returns 1 if positions are ordered (if ppos2 is after ppos1), -1 if not, 0 if same.
+Positions of the word boxes containing ppos1 and ppos2 are compared.
+--]]
+function KoptInterface:comparePositions(doc, ppos1, ppos2)
+    local box1 = self:getWordFromPosition(doc, ppos1).pbox
+    local box2 = self:getWordFromPosition(doc, ppos2).pbox
+    if box1.y == box2.y then
+        if box1.x == box2.x then
+            return 0
+        elseif box1.x > box2.x then
+            return -1
+        end
+    elseif box1.y > box2.y then
+        return -1
+    end
+    return 1
+end
+
 --[[--
 Get page rect from native rect.
 --]]

frontend/document/pdfdocument.lua

@@ -98,6 +98,10 @@ function PdfDocument:unlock(password)
     return true
 end
 
+function PdfDocument:comparePositions(pos1, pos2)
+    return self.koptinterface:comparePositions(self, pos1, pos2)
+end
+
 function PdfDocument:getPageTextBoxes(pageno)
     local page = self._document:openPage(pageno)
     local text = page:getPageText()