From 5357fdde7c80120696b78b2b59afd00753ffab3e Mon Sep 17 00:00:00 2001 From: poire-z Date: Sun, 4 Feb 2018 20:57:13 +0100 Subject: [PATCH] cre full text search: fix inconsistencies (#3656) Fix various inconsitencies: some occurences not highlighted on displayed page; different occurences highlighted on same page wheter we went there searching backward or forward; pages with occurences simply just skipped when searching in one direction, and not in the other... To avoid edge cases, crengine will now give back results on up to 3 pages, that we need to filter. Bump base and crengine --- base | 2 +- frontend/apps/reader/modules/readersearch.lua | 73 ++++++++++++++++++- spec/unit/readersearch_spec.lua | 20 ++++- 3 files changed, 91 insertions(+), 4 deletions(-) diff --git a/base b/base index 027cc1c64..81cd9b5b9 160000 --- a/base +++ b/base @@ -1 +1 @@ -Subproject commit 027cc1c648fa54ad637aee083527a671fad65df9 +Subproject commit 81cd9b5b9324de5f6039a1ccaf49f189ccce59ad diff --git a/frontend/apps/reader/modules/readersearch.lua b/frontend/apps/reader/modules/readersearch.lua index 48b6d8576..28578d535 100644 --- a/frontend/apps/reader/modules/readersearch.lua +++ b/frontend/apps/reader/modules/readersearch.lua @@ -7,6 +7,11 @@ local _ = require("gettext") local ReaderSearch = InputContainer:new{ direction = 0, -- 0 for search forward, 1 for search backward case_insensitive = true, -- default to case insensitive + + -- internal: whether we expect results on previous pages + -- (can be different from self.direction, if, from a page in the + -- middle of a book, we search forward from start of book) + _expect_back_results = false, } function ReaderSearch:init() @@ -29,6 +34,7 @@ end function ReaderSearch:onShowSearchDialog(text) local neglect_current_location = false + local current_page local do_search = function(search_func, _text, param) return function() local res = search_func(self, _text, param) @@ -37,7 +43,68 @@ function ReaderSearch:onShowSearchDialog(text) self.ui.link:onGotoLink({page = res.page - 1}, neglect_current_location) self.view.highlight.temp[res.page] = res else - self.ui.link:onGotoLink(res[1].start, neglect_current_location) + -- Was previously just: + -- self.ui.link:onGotoLink(res[1].start, neglect_current_location) + + -- To avoid problems with edge cases, crengine may now give us links + -- that are on previous/next page of the page we should show. And + -- sometimes even xpointers that resolve to no page. + -- We need to loop thru all the results until we find one suitable, + -- to follow its link and go to the next/prev page with occurences. + local valid_link + -- If backward search, results are already in a reversed order, so we'll + -- start from the nearest to current page one. + for _, r in ipairs(res) do + -- result's start and end may be on different pages, we must + -- consider both + local r_start = r["start"] + local r_end = r["end"] + local r_start_page = self.ui.document:getPageFromXPointer(r_start) + local r_end_page = self.ui.document:getPageFromXPointer(r_end) + logger.dbg("res.start page & xpointer:", r_start_page, r_start) + logger.dbg("res.end page & xpointer:", r_end_page, r_end) + local bounds = {} + if self._expect_back_results then + -- Process end of occurence first, which is nearest to current page + table.insert(bounds, {r_end, r_end_page}) + table.insert(bounds, {r_start, r_start_page}) + else + table.insert(bounds, {r_start, r_start_page}) + table.insert(bounds, {r_end, r_end_page}) + end + for _, b in ipairs(bounds) do + local xpointer = b[1] + local page = b[2] + -- Look if it is valid for us + if page then -- it should resolve to a page + if not current_page then -- initial search + -- We can (and should if there are) display results on current page + current_page = self.ui.document:getCurrentPage() + if (self._expect_back_results and page <= current_page) or + (not self._expect_back_results and page >= current_page) then + valid_link = xpointer + end + else -- subsequent searches + -- We must change page, so only consider results from + -- another page, in the adequate search direction + current_page = self.ui.document:getCurrentPage() + if (self._expect_back_results and page < current_page) or + (not self._expect_back_results and page > current_page) then + valid_link = xpointer + end + end + end + if valid_link then + break + end + end + if valid_link then + break + end + end + if valid_link then + self.ui.link:onGotoLink(valid_link, neglect_current_location) + end end -- Don't add result pages to location ("Go back") stack neglect_current_location = true @@ -89,22 +156,26 @@ end function ReaderSearch:searchFromStart(pattern) self.direction = 0 + self._expect_back_results = true return self:search(pattern, -1) end function ReaderSearch:searchFromEnd(pattern) self.direction = 1 + self._expect_back_results = false return self:search(pattern, -1) end function ReaderSearch:searchFromCurrent(pattern, direction) self.direction = direction + self._expect_back_results = direction == 1 return self:search(pattern, 0) end -- ignore current page and search next occurrence function ReaderSearch:searchNext(pattern, direction) self.direction = direction + self._expect_back_results = direction == 1 return self:search(pattern, 1) end diff --git a/spec/unit/readersearch_spec.lua b/spec/unit/readersearch_spec.lua index 42832df4f..b3ab00702 100644 --- a/spec/unit/readersearch_spec.lua +++ b/spec/unit/readersearch_spec.lua @@ -81,13 +81,29 @@ describe("Readersearch module", function() it("should find all occurrences", function() local count = 0 rolling:onGotoPage(1) + local cur_page = doc:getCurrentPage() local words = search:searchFromCurrent("Verona", 0) while words do - count = count + #words + local new_page = nil for _, word in ipairs(words) do --dbg("found word", word.start) + local word_page = doc:getPageFromXPointer(word.start) + if word_page ~= cur_page then -- ignore words on current page + if not new_page then -- first word on a new page + new_page = word_page + count = count + 1 + doc:gotoXPointer(word.start) -- go to this new page + else -- new page seen + if word_page == new_page then -- only count words on this new page + count = count + 1 + end + end + end + end + if not new_page then -- no word seen on any new page + break end - doc:gotoXPointer(words[#words].start) + cur_page = doc:getCurrentPage() words = search:searchNext("Verona", 0) end assert.are.equal(13, count)