diff --git a/frontend/apps/reader/modules/readerlink.lua b/frontend/apps/reader/modules/readerlink.lua index f6eb355f5..04d8b26ff 100644 --- a/frontend/apps/reader/modules/readerlink.lua +++ b/frontend/apps/reader/modules/readerlink.lua @@ -2,12 +2,14 @@ ReaderLink is an abstraction for document-specific link interfaces. ]] +local ConfirmBox = require("ui/widget/confirmbox") local Device = require("device") local Event = require("ui/event") local Geom = require("ui/geometry") local GestureRange = require("ui/gesturerange") local InputContainer = require("ui/widget/container/inputcontainer") local LinkBox = require("ui/widget/linkbox") +local Notification = require("ui/widget/notification") local UIManager = require("ui/uimanager") local logger = require("logger") local _ = require("gettext") @@ -139,9 +141,37 @@ function ReaderLink:addToMainMenu(menu_items) text = _("Go back to previous location"), enabled_func = function() return #self.location_stack > 0 end, callback = function() self:onGoBackLink() end, + hold_callback = function() UIManager:show(ConfirmBox:new{ + text = _("Clear location history?"), + ok_text = _("Clear"), + ok_callback = function() + self.location_stack = {} + end, + }) + end, } end +--- Check if a xpointer to node really points to itself +function ReaderLink:isXpointerCoherent(a_xpointer) + -- Get screen coordinates of xpointer + local doc_margins = self.ui.document._document:getPageMargins() + local doc_y, doc_x = self.ui.document:getPosFromXPointer(a_xpointer) + local top_y = self.ui.document:getCurrentPos() + -- (strange, but using doc_margins.top is accurate even in scroll mode) + local screen_y = doc_y - top_y + doc_margins["top"] + local screen_x = doc_x + doc_margins["left"] + -- Get again link and a_xpointer from this position + local re_link_xpointer, re_a_xpointer = self.ui.document:getLinkFromPosition({x = screen_x, y = screen_y}) -- luacheck: no unused + -- We should get the same a_xpointer. If not, crengine has messed up + -- and we should not trust this xpointer to get back to this link. + if re_a_xpointer ~= a_xpointer then + logger.info("not coherent a_xpointer:", a_xpointer) + return false + end + return true +end + --- Gets link from gesture. -- `Document:getLinkFromPosition()` behaves differently depending on -- document type, so this function provides a wrapper. @@ -152,35 +182,56 @@ function ReaderLink:getLinkFromGes(ges) -- link box in native page local link, lbox = self.ui.document:getLinkFromPosition(pos.page, pos) if link and lbox then - return link, lbox, pos + return { + link = link, + lbox = lbox, + pos = pos, + } end end else - local link = self.ui.document:getLinkFromPosition(ges.pos) - if link ~= "" then - return link + local link_xpointer, a_xpointer = self.ui.document:getLinkFromPosition(ges.pos) + logger.dbg("getLinkFromPosition link_xpointer:", link_xpointer) + logger.dbg("getLinkFromPosition a_xpointer:", a_xpointer) + + -- On some documents, crengine may sometimes give a wrong a_xpointer + -- (in some Wikipedia saved as EPUB, it would point to some other + -- element in the same paragraph). If followed then back, we could get + -- to a different page. So, we check here how valid it is, and if not, + -- we just discard it so that addCurrentLocationToStack() is used. + if a_xpointer and not self:isXpointerCoherent(a_xpointer) then + a_xpointer = nil + end + + if link_xpointer ~= "" then + -- This link's target xpointer is more precise than a classic + -- xpointer to top of a page: we can show a marker at its + -- y-position in target page + return { + xpointer = link_xpointer, + marker_xpointer = link_xpointer, + from_xpointer = a_xpointer, + } end end end --- Highlights a linkbox if available and goes to it. -function ReaderLink:showLinkBox(link, lbox, pos) - if link and lbox then +function ReaderLink:showLinkBox(link) + if link and link.lbox then -- pdfdocument -- screen box that holds the link - local sbox = self.view:pageToScreenTransform(pos.page, - self.ui.document:nativeToPageRectTransform(pos.page, lbox)) + local sbox = self.view:pageToScreenTransform(link.pos.page, + self.ui.document:nativeToPageRectTransform(link.pos.page, link.lbox)) if sbox then UIManager:show(LinkBox:new{ box = sbox, timeout = FOLLOW_LINK_TIMEOUT, - callback = function() self:onGotoLink(link) end + callback = function() self:onGotoLink(link.link) end }) return true end - else - if link ~= "" then - return self:onGotoLink(link) - end + elseif link and link.xpointer ~= "" then -- credocument + return self:onGotoLink(link) end end @@ -193,9 +244,9 @@ end function ReaderLink:onTap(_, ges) if not isTapToFollowLinksOn() then return end - local link, lbox, pos = self:getLinkFromGes(ges) + local link = self:getLinkFromGes(ges) if link then - return self:showLinkBox(link, lbox, pos) + return self:showLinkBox(link) end end @@ -204,7 +255,9 @@ function ReaderLink:addCurrentLocationToStack() if self.ui.document.info.has_pages then table.insert(self.location_stack, self.ui.paging:getBookLocation()) else - table.insert(self.location_stack, self.ui.rolling:getBookLocation()) + table.insert(self.location_stack, { + xpointer = self.ui.rolling:getBookLocation(), + }) end end @@ -229,24 +282,47 @@ function ReaderLink:onGotoLink(link, neglect_current_location) -- If the XPointer does not exist (or is a full url), we will jump to page 1 -- Best to check that this link exists in document with the following, -- which accepts both of the above legitimate xpointer as input. - if self.ui.document:isXPointerInDocument(link) then + if self.ui.document:isXPointerInDocument(link.xpointer) then logger.dbg("Internal link:", link) if not neglect_current_location then - self:addCurrentLocationToStack() + if link.from_xpointer then + -- We have a more precise xpointer than the xpointer to top of + -- current page that addCurrentLocationToStack() would give, and + -- we may be able to show a marker when back + local saved_location + if self.view.view_mode == "scroll" then + -- In scroll mode, we still use the top of page as the + -- xpointer to go back to, so we get back to the same view. + -- We can still show the marker at the link position + saved_location = { + xpointer = self.ui.rolling:getBookLocation(), + marker_xpointer = link.from_xpointer, + } + else + -- In page mode, we use the same for go to and for marker, + -- as 'page mode' ensures we get back to the same view. + saved_location = { + xpointer = link.from_xpointer, + marker_xpointer = link.from_xpointer, + } + end + table.insert(self.location_stack, saved_location) + else + self:addCurrentLocationToStack() + end end - self.ui:handleEvent(Event:new("GotoXPointer", link)) + self.ui:handleEvent(Event:new("GotoXPointer", link.xpointer, link.marker_xpointer)) return true end end logger.dbg("External link:", link) -- Check if it is a wikipedia link - local wiki_lang, wiki_page = link:match([[https?://([^%.]+).wikipedia.org/wiki/([^/]+)]]) + local wiki_lang, wiki_page = link.xpointer:match([[https?://([^%.]+).wikipedia.org/wiki/([^/]+)]]) if wiki_lang and wiki_page then logger.dbg("Wikipedia link:", wiki_lang, wiki_page) -- Ask for user confirmation before launching lookup (on a -- wikipedia page saved as epub, full of wikipedia links, it's -- too easy to click on links when wanting to change page...) - local ConfirmBox = require("ui/widget/confirmbox") UIManager:show(ConfirmBox:new{ text = T(_("Would you like to read this Wikipedia %1 article?\n\n%2\n"), wiki_lang:upper(), wiki_page:gsub("_", " ")), ok_callback = function() @@ -256,11 +332,10 @@ function ReaderLink:onGotoLink(link, neglect_current_location) end }) else - -- local Notification = require("ui/widget/notification") local InfoMessage = require("ui/widget/infomessage") UIManager:show(InfoMessage:new{ - text = T(_("Invalid or external link:\n%1"), link), - timeout = 1.0, + text = T(_("Invalid or external link:\n%1"), link.xpointer), + -- no timeout to allow user to type that link in his web browser }) end -- don't propagate, user will notice and tap elsewhere if he wants to change page @@ -271,15 +346,32 @@ end function ReaderLink:onGoBackLink() local saved_location = table.remove(self.location_stack) if saved_location then + logger.dbg("GoBack: restoring:", saved_location) self.ui:handleEvent(Event:new('RestoreBookLocation', saved_location)) return true end end -function ReaderLink:onSwipe(_, ges) +function ReaderLink:onSwipe(arg, ges) if ges.direction == "east" then if isSwipeToGoBackEnabled() then - return self:onGoBackLink() + if #self.location_stack > 0 then + -- Remember if location stack is going to be empty, so we + -- can stop the propagation of next swipe back: so the user + -- knows it is empty and that next swipe back will get him + -- to previous page (and not to previous location) + self.swipe_back_resist = #self.location_stack == 1 + return self:onGoBackLink() + elseif self.swipe_back_resist then + self.swipe_back_resist = false + -- Make that gesture don't do anything, and show a Notification + -- so the user knows why + UIManager:show(Notification:new{ + text = _("Previous locations stack is empty"), + timeout = 1.0, + }) + return true + end end elseif ges.direction == "west" then local ret = false @@ -320,7 +412,7 @@ function ReaderLink:onGoToPageLink(ges, use_page_first_link) -- ["x0"] = 97, -- ["page"] = 347 -- }, - local pos_x, pos_y = ges.pos.x, ges.pos.y + local pos_x, pos_y = pos.x, pos.y local shortest_dist = nil local first_y0 = nil for _, link in ipairs(links) do @@ -367,7 +459,11 @@ function ReaderLink:onGoToPageLink(ges, use_page_first_link) -- ["end_y"] = 1201, -- ["start_x"] = 352, -- ["start_y"] = 1201 + -- ["a_xpointer"] = "/body/DocFragment/body/div/p[12]/sup[3]/a[3].0", -- }, + -- Note: with some documents and some links, crengine may give wrong + -- coordinates, and our code below may miss or give the wrong first + -- or nearest link... local pos_x, pos_y = ges.pos.x, ges.pos.y local shortest_dist = nil local first_start_y = nil @@ -377,7 +473,7 @@ function ReaderLink:onGoToPageLink(ges, use_page_first_link) -- links may not be in the order they are in the page, so let's -- find the one with the smallest start_y. if first_start_y == nil or link["start_y"] < first_start_y then - selected_link = link["section"] + selected_link = link first_start_y = link["start_y"] end else @@ -385,9 +481,7 @@ function ReaderLink:onGoToPageLink(ges, use_page_first_link) local end_dist = math.pow(link.end_x - pos_x, 2) + math.pow(link.end_y - pos_y, 2) local min_dist = math.min(start_dist, end_dist) if shortest_dist == nil or min_dist < shortest_dist then - -- onGotoLink()'s GotoXPointer event needs - -- the "section" value - selected_link = link["section"] + selected_link = link shortest_dist = min_dist end end @@ -398,6 +492,24 @@ function ReaderLink:onGoToPageLink(ges, use_page_first_link) -- and we'll find them highlighted when back from link. -- So let's clear them now. self.ui.document:clearSelection() + -- (Comment out previous line to visually see which links on the + -- page are not coherent: those not highlighted) + + if selected_link then + logger.dbg("original selected_link", selected_link) + -- Make it a link as expected by onGotoLink + selected_link = { + xpointer = selected_link["section"], + marker_xpointer = selected_link["section"], + from_xpointer = selected_link["a_xpointer"], + } + logger.dbg("selected_link", selected_link) + -- Check from_xpointer is coherent, and unset it if not + if selected_link.from_xpointer and not self:isXpointerCoherent(selected_link.from_xpointer) then + selected_link.from_xpointer = nil + end + + end end if selected_link then return self:onGotoLink(selected_link) @@ -416,9 +528,19 @@ function ReaderLink:onGoToLatestBookmark(ges) fake_link.page = latest_bookmark.page - 1 return self:onGotoLink(fake_link) else - -- self:onGotoLink() needs a xpointer, that we find - -- as the bookmark page attribute - return self:onGotoLink(latest_bookmark.page) + -- Make it a link as expected by onGotoLink + local link + if latest_bookmark.pos0 then -- text highlighted, precise xpointer + link = { + xpointer = latest_bookmark.pos0, + marker_xpointer = latest_bookmark.pos0, + } + else -- page bookmarked, 'page' is a xpointer to top of page + link = { + xpointer = latest_bookmark.page, + } + end + return self:onGotoLink(link) end end end diff --git a/frontend/apps/reader/modules/readerrolling.lua b/frontend/apps/reader/modules/readerrolling.lua index 7a76d7e1d..0625e1e47 100644 --- a/frontend/apps/reader/modules/readerrolling.lua +++ b/frontend/apps/reader/modules/readerrolling.lua @@ -1,6 +1,5 @@ local Blitbuffer = require("ffi/blitbuffer") local Device = require("device") -local Geom = require("ui/geometry") local InputContainer = require("ui/widget/container/inputcontainer") local Event = require("ui/event") local ReaderPanning = require("apps/reader/modules/readerpanning") @@ -394,25 +393,63 @@ function ReaderRolling:onGotoRelativePage(number) return true end -function ReaderRolling:onGotoXPointer(xp) +function ReaderRolling:onGotoXPointer(xp, marker_xp) + if self.mark_func then + -- unschedule previous marker as it's no more accurate + UIManager:unschedule(self.mark_func) + self.mark_func = nil + end + if self.unmark_func then + -- execute scheduled unmark now to clean previous marker + self.unmark_func() + self.unmark_func = nil + end self:_gotoXPointer(xp) self.xpointer = xp - -- Show a mark on left side of screen to give a visual feedback - -- of where xpointer target is (removed after 1 second) - if string.sub(xp, 1, 1) == "#" then -- only for links, not page top fragment identifier - local doc_y = self.ui.document:getPosFromXPointer(xp) + + -- Allow tweaking this marker behaviour with a manual setting: + -- followed_link_marker = false: no marker shown + -- followed_link_marker = true: maker shown and not auto removed + -- followed_link_marker = : removed after seconds + -- (no real need for a menu item, the default is the finest) + local marker_setting = G_reader_settings:readSetting("followed_link_marker") + if marker_setting == nil then + marker_setting = 1 -- default is: shown and removed after 1 second + end + + if marker_xp and marker_setting then + -- Show a mark on left side of screen to give a visual feedback of + -- where xpointer target is (and remove if after 1s) + local doc_y = self.ui.document:getPosFromXPointer(marker_xp) local top_y = self.ui.document:getCurrentPos() + local screen_y = doc_y - top_y local doc_margins = self.ui.document._document:getPageMargins() - local screen_y = doc_y - top_y + doc_margins["top"] - local marker_w = math.max(doc_margins["left"] - Screen:scaleBySize(5), Screen:scaleBySize(5)) + if self.view.view_mode == "page" then + screen_y = screen_y + doc_margins["top"] + end local marker_h = Screen:scaleBySize(self.ui.font.font_size * 1.1 * self.ui.font.line_space_percent/100.0) - UIManager:scheduleIn(0.5, function() + -- Make it 4/5 of left margin wide (and bigger when huge margin) + local marker_w = math.floor(math.max(doc_margins["left"] - Screen:scaleBySize(5), doc_margins["left"] * 4/5)) + + self.mark_func = function() + self.mark_func = nil Screen.bb:paintRect(0, screen_y, marker_w, marker_h, Blitbuffer.COLOR_BLACK) - Screen["refreshPartial"](Screen, 0, screen_y, marker_w, marker_h) - UIManager:scheduleIn(1, function() - UIManager:setDirty(self.view.dialog, "partial", Geom:new({x=0, y=screen_y, w=marker_w, h=marker_h})) - end) - end) + Screen["refreshUI"](Screen, 0, screen_y, marker_w, marker_h) + if type(marker_setting) == "number" then -- hide it + self.unmark_func = function() + self.unmark_func = nil + -- UIManager:setDirty(self.view.dialog, "ui", Geom:new({x=0, y=screen_y, w=marker_w, h=marker_h})) + -- No need to use setDirty (which would ask crengine to + -- re-render the page, which may take a few seconds on big + -- documents): we drew our black marker in the margin, we + -- can just draw a white one to make it disappear + Screen.bb:paintRect(0, screen_y, marker_w, marker_h, Blitbuffer.COLOR_WHITE) + Screen["refreshUI"](Screen, 0, screen_y, marker_w, marker_h) + end + UIManager:scheduleIn(marker_setting, self.unmark_func) + end + end + UIManager:scheduleIn(0.5, self.mark_func) end return true end @@ -422,7 +459,7 @@ function ReaderRolling:getBookLocation() end function ReaderRolling:onRestoreBookLocation(saved_location) - return self:onGotoXPointer(saved_location) + return self:onGotoXPointer(saved_location.xpointer, saved_location.marker_xpointer) end function ReaderRolling:onGotoViewRel(diff) diff --git a/frontend/apps/reader/modules/readersearch.lua b/frontend/apps/reader/modules/readersearch.lua index 28578d535..9e7323039 100644 --- a/frontend/apps/reader/modules/readersearch.lua +++ b/frontend/apps/reader/modules/readersearch.lua @@ -103,7 +103,7 @@ function ReaderSearch:onShowSearchDialog(text) end end if valid_link then - self.ui.link:onGotoLink(valid_link, neglect_current_location) + self.ui.link:onGotoLink({xpointer=valid_link}, neglect_current_location) end end -- Don't add result pages to location ("Go back") stack