From a982c8b92f8f74f755653987d294009f5d18dfb8 Mon Sep 17 00:00:00 2001 From: Frans de Jonge Date: Sun, 31 Mar 2019 15:46:41 +0200 Subject: [PATCH 1/8] [feat, UX] Add external link to Wallabag Offers a choice when relevant, i.e., on platforms that support an external browser. --- frontend/apps/reader/modules/readerlink.lua | 142 ++++++++++++++------ 1 file changed, 99 insertions(+), 43 deletions(-) diff --git a/frontend/apps/reader/modules/readerlink.lua b/frontend/apps/reader/modules/readerlink.lua index ccd652c4d6f9..75c1b02adb87 100644 --- a/frontend/apps/reader/modules/readerlink.lua +++ b/frontend/apps/reader/modules/readerlink.lua @@ -2,6 +2,7 @@ ReaderLink is an abstraction for document-specific link interfaces. ]] +local ButtonDialogTitle = require("ui/widget/buttondialogtitle") local ConfirmBox = require("ui/widget/confirmbox") local Device = require("device") local Event = require("ui/event") @@ -517,6 +518,57 @@ function ReaderLink:onGotoLink(link, neglect_current_location, allow_footnote_po end logger.dbg("External link:", link_url) + local is_http_link = link_url:find("^https?://") ~= nil + if is_http_link and self:onGoToExternalLink(link_url) then + return true + end + + -- Check if it is a link to a local file + local linked_filename = link_url:gsub("^file:", "") -- remove local file protocol if any + local anchor + if linked_filename:find("?") then -- remove any query string (including any following anchor) + linked_filename, anchor = linked_filename:match("^(.-)(%?.*)$") + elseif linked_filename:find("#") then -- remove any anchor + linked_filename, anchor = linked_filename:match("^(.-)(#.*)$") + end + linked_filename = ffiutil.joinPath(self.document_dir, linked_filename) -- get full path + linked_filename = ffiutil.realpath(linked_filename) -- clean full path from ./ or ../ + if linked_filename and lfs.attributes(linked_filename, "mode") == "file" then + local DocumentRegistry = require("document/documentregistry") + local provider = DocumentRegistry:getProvider(linked_filename) + if provider then + -- Display filename with anchor or query string, so the user gets + -- this information and can manually go to the appropriate place + local display_filename = linked_filename + if anchor then + display_filename = display_filename .. anchor + end + UIManager:show(ConfirmBox:new{ + text = T(_("Would you like to read this local document?\n\n%1\n"), display_filename), + ok_callback = function() + UIManager:scheduleIn(0.1, function() + self.ui:switchDocument(linked_filename) + end) + end + }) + else + UIManager:show(InfoMessage:new{ + text = T(_("Link to unsupported local file:\n%1"), link_url), + }) + end + return true + end + + -- Not supported + UIManager:show(InfoMessage:new{ + text = T(_("Invalid or external link:\n%1"), link_url), + -- no timeout to allow user to type that link in his web browser + }) + -- don't propagate, user will notice and tap elsewhere if he wants to change page + return true +end + +function ReaderLink:onGoToExternalLink(link_url) -- Check if it is a wikipedia link local wiki_lang, wiki_page = link_url:match([[https?://([^%.]+).wikipedia.org/wiki/([^/]+)]]) if wiki_lang and wiki_page then @@ -576,56 +628,60 @@ function ReaderLink:onGotoLink(link, neglect_current_location, allow_footnote_po }) end return true - end + elseif self.ui.wallabag then + local settings = G_reader_settings:readSetting("external_link_action") + local choose_action + if settings == "pop-up" or settings == nil then + local buttons = { + { + { + text = _("Cancel"), + callback = function() + UIManager:close(choose_action) + end, + }, + { + text = _("Add to Wallabag"), + callback = function() + self.ui.wallabag:addArticle(link_url) + UIManager:close(choose_action) + end, + }, - -- Check if it is a link to a local file - local linked_filename = link_url:gsub("^file:", "") -- remove local file protocol if any - local anchor - if linked_filename:find("?") then -- remove any query string (including any following anchor) - linked_filename, anchor = linked_filename:match("^(.-)(%?.*)$") - elseif linked_filename:find("#") then -- remove any anchor - linked_filename, anchor = linked_filename:match("^(.-)(#.*)$") - end - linked_filename = ffiutil.joinPath(self.document_dir, linked_filename) -- get full path - linked_filename = ffiutil.realpath(linked_filename) -- clean full path from ./ or ../ - if linked_filename and lfs.attributes(linked_filename, "mode") == "file" then - local DocumentRegistry = require("document/documentregistry") - local provider = DocumentRegistry:getProvider(linked_filename) - if provider then - -- Display filename with anchor or query string, so the user gets - -- this information and can manually go to the appropriate place - local display_filename = linked_filename - if anchor then - display_filename = display_filename .. anchor + }, + } + if Device:openLink() == nil then + table.insert(buttons, { + { + text = "–", + enabled = false, + }, + { + text = _("Open in browser"), + callback = function() + Device:openLink(link_url) + UIManager:close(choose_action) + end, + }, + }) end - UIManager:show(ConfirmBox:new{ - text = T(_("Would you like to read this local document?\n\n%1\n"), display_filename), - ok_callback = function() - UIManager:scheduleIn(0.1, function() - self.ui:switchDocument(linked_filename) - end) - end - }) - else - UIManager:show(InfoMessage:new{ - text = T(_("Link to unsupported local file:\n%1"), link_url), - }) + + choose_action = ButtonDialogTitle:new{ + title = T(_("External link:\n\n'%1'"), link_url), + buttons = buttons, + } + + UIManager:show(choose_action) + elseif settings == "add_to_wallabag" then + self.ui.wallabag:addArticle(link_url) + elseif settings == "open_in_browser" then + Device:openLink(link_url) end return true - end - -- try opening link in native browser - if Device:openLink(link_url) then + elseif Device:openLink(link_url) then return true end - - -- Not supported - UIManager:show(InfoMessage:new{ - text = T(_("Invalid or external link:\n%1"), link_url), - -- no timeout to allow user to type that link in his web browser - }) - -- don't propagate, user will notice and tap elsewhere if he wants to change page - return true end --- Goes back to previous location. From fd5fd0ad81fb04481064d60f2b7127c96314f179 Mon Sep 17 00:00:00 2001 From: Frans de Jonge Date: Sun, 31 Mar 2019 16:42:17 +0200 Subject: [PATCH 2/8] address a few review comments --- frontend/apps/reader/modules/readerlink.lua | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/frontend/apps/reader/modules/readerlink.lua b/frontend/apps/reader/modules/readerlink.lua index 75c1b02adb87..405dcce0b804 100644 --- a/frontend/apps/reader/modules/readerlink.lua +++ b/frontend/apps/reader/modules/readerlink.lua @@ -629,9 +629,9 @@ function ReaderLink:onGoToExternalLink(link_url) end return true elseif self.ui.wallabag then - local settings = G_reader_settings:readSetting("external_link_action") + local external_link_action = G_reader_external_link_action:readSetting("external_link_action") local choose_action - if settings == "pop-up" or settings == nil then + if external_link_action == "pop-up" or external_link_action == nil then local buttons = { { { @@ -650,7 +650,7 @@ function ReaderLink:onGoToExternalLink(link_url) }, } - if Device:openLink() == nil then + if Device:openLink() ~= false then table.insert(buttons, { { text = "–", @@ -667,14 +667,14 @@ function ReaderLink:onGoToExternalLink(link_url) end choose_action = ButtonDialogTitle:new{ - title = T(_("External link:\n\n'%1'"), link_url), + title = T(_("External link:\n\n%1"), link_url), buttons = buttons, } UIManager:show(choose_action) - elseif settings == "add_to_wallabag" then + elseif external_link_action == "add_to_wallabag" then self.ui.wallabag:addArticle(link_url) - elseif settings == "open_in_browser" then + elseif external_link_action == "open_in_browser" then Device:openLink(link_url) end return true From 091aada22cf96aaf051b62b37ba7582d84b6c78e Mon Sep 17 00:00:00 2001 From: Frans de Jonge Date: Sun, 31 Mar 2019 16:48:13 +0200 Subject: [PATCH 3/8] whoops, one too many --- frontend/apps/reader/modules/readerlink.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/apps/reader/modules/readerlink.lua b/frontend/apps/reader/modules/readerlink.lua index 405dcce0b804..0795c26c05af 100644 --- a/frontend/apps/reader/modules/readerlink.lua +++ b/frontend/apps/reader/modules/readerlink.lua @@ -629,7 +629,7 @@ function ReaderLink:onGoToExternalLink(link_url) end return true elseif self.ui.wallabag then - local external_link_action = G_reader_external_link_action:readSetting("external_link_action") + local external_link_action = G_reader_settings:readSetting("external_link_action") local choose_action if external_link_action == "pop-up" or external_link_action == nil then local buttons = { From 75786257d9bd95285445f0ccd2c5892d1e0ee12a Mon Sep 17 00:00:00 2001 From: Frans de Jonge Date: Sun, 31 Mar 2019 17:24:18 +0200 Subject: [PATCH 4/8] Add external link settings menu and Device:canOpenLink() --- frontend/apps/reader/modules/readerlink.lua | 47 ++++++++++++++++++++- frontend/device/android/device.lua | 1 + frontend/device/generic/device.lua | 1 + frontend/device/sdl/device.lua | 1 + 4 files changed, 48 insertions(+), 2 deletions(-) diff --git a/frontend/apps/reader/modules/readerlink.lua b/frontend/apps/reader/modules/readerlink.lua index 0795c26c05af..49548c3c04f4 100644 --- a/frontend/apps/reader/modules/readerlink.lua +++ b/frontend/apps/reader/modules/readerlink.lua @@ -126,7 +126,50 @@ function ReaderLink:addToMainMenu(menu_items) separator = true, help_text = _([[Tap on links to follow them.]]), }, - + { + text = _("External link action"), + sub_item_table = { + { + text = _("Ask with pop-up dialog"), + checked_func = function() + local setting = G_reader_settings:readSetting("external_link_action") + return setting == "pop-up" or setting == nil + end, + callback = function() + G_reader_settings:saveSetting("external_link_action", "pop-up") + end, + }, + { + text = _("Do nothing"), + checked_func = function() + return G_reader_settings:readSetting("external_link_action") == "nothing" + end, + callback = function() + G_reader_settings:saveSetting("external_link_action", "nothing") + end, + }, + { + text = _("Add to Wallabag"), + checked_func = function() + return G_reader_settings:readSetting("external_link_action") == "add_to_wallabag" + end, + enabled_func = function() return self.ui.wallabag ~= nil end, + callback = function() + G_reader_settings:saveSetting("external_link_action", "add_to_wallabag") + end, + }, + { + text = _("Open in browser"), + checked_func = function() + return G_reader_settings:readSetting("external_link_action") == "open_in_browser" + end, + enabled_func = function() return Device:canOpenLink() end, + callback = function() + G_reader_settings:saveSetting("external_link_action", "open_in_browser") + end, + }, + }, + }, { text = _("Swipe to go back"), checked_func = isSwipeToGoBackEnabled, @@ -650,7 +693,7 @@ function ReaderLink:onGoToExternalLink(link_url) }, } - if Device:openLink() ~= false then + if Device:canOpenLink() then table.insert(buttons, { { text = "–", diff --git a/frontend/device/android/device.lua b/frontend/device/android/device.lua index 8c732fb50026..32dbe0426eae 100644 --- a/frontend/device/android/device.lua +++ b/frontend/device/android/device.lua @@ -27,6 +27,7 @@ local Device = Generic:new{ display_dpi = android.lib.AConfiguration_getDensity(android.app.config), hasClipboard = yes, hasOTAUpdates = canUpdateApk, + canOpenLink = yes, openLink = function(self, link) if not link or type(link) ~= "string" then return end return android.openLink(link) == 0 diff --git a/frontend/device/generic/device.lua b/frontend/device/generic/device.lua index e334826a9afa..654d240c07cd 100644 --- a/frontend/device/generic/device.lua +++ b/frontend/device/generic/device.lua @@ -65,6 +65,7 @@ local Device = { -- set to yes on devices that support over-the-air incremental updates. hasOTAUpdates = no, + canOpenLink = no, openLink = no, } diff --git a/frontend/device/sdl/device.lua b/frontend/device/sdl/device.lua index 81ef0ec53e86..3cf45bd54617 100644 --- a/frontend/device/sdl/device.lua +++ b/frontend/device/sdl/device.lua @@ -17,6 +17,7 @@ local Device = Generic:new{ needsScreenRefreshAfterResume = no, hasColorScreen = yes, hasEinkScreen = no, + canOpenLink = yes, openLink = function(self, link) if not link or type(link) ~= "string" then return end return os.execute("xdg-open '"..link.."'") == 0 From fd369461b274fe5701d9f24f0cbde36f4110c54c Mon Sep 17 00:00:00 2001 From: Frans de Jonge Date: Sun, 31 Mar 2019 17:42:44 +0200 Subject: [PATCH 5/8] Rework Wallabag stuff slightly to show feedback message --- frontend/apps/reader/modules/readerlink.lua | 5 ++-- plugins/wallabag.koplugin/main.lua | 26 ++++++++++++++++++++- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/frontend/apps/reader/modules/readerlink.lua b/frontend/apps/reader/modules/readerlink.lua index 49548c3c04f4..147ddfac2b5b 100644 --- a/frontend/apps/reader/modules/readerlink.lua +++ b/frontend/apps/reader/modules/readerlink.lua @@ -686,7 +686,7 @@ function ReaderLink:onGoToExternalLink(link_url) { text = _("Add to Wallabag"), callback = function() - self.ui.wallabag:addArticle(link_url) + self.ui:handleEvent(Event:new("AddWallabagArticle", link_url)) UIManager:close(choose_action) end, }, @@ -716,7 +716,8 @@ function ReaderLink:onGoToExternalLink(link_url) UIManager:show(choose_action) elseif external_link_action == "add_to_wallabag" then - self.ui.wallabag:addArticle(link_url) + + self.ui:handleEvent(Event:new("AddWallabagArticle", link_url)) elseif external_link_action == "open_in_browser" then Device:openLink(link_url) end diff --git a/plugins/wallabag.koplugin/main.lua b/plugins/wallabag.koplugin/main.lua index a40021629b83..a6aeee68b16b 100644 --- a/plugins/wallabag.koplugin/main.lua +++ b/plugins/wallabag.koplugin/main.lua @@ -546,7 +546,7 @@ function Wallabag:addArticle(article_url) ["Authorization"] = "Bearer " .. self.access_token, } - self:callAPI("POST", "/api/entries.json", headers, body_JSON, "") + return self:callAPI("POST", "/api/entries.json", headers, body_JSON, "") end function Wallabag:deleteArticle( path ) @@ -749,6 +749,27 @@ function Wallabag:saveWBSettings(setting) self.wb_settings:flush() end +function Wallabag:onAddWallabagArticle(article_url) + if not NetworkMgr:isOnline() then + NetworkMgr:promptWifiOn() + return + end + + local wallabag_result = self:addArticle(article_url) + if wallabag_result then + UIManager:show(InfoMessage:new{ + text = T(_("Article added to Wallabag:\n%1"), article_url), + }) + else + UIManager:show(InfoMessage:new{ + text = T(_("Error adding link to Wallabag:\n%1"), article_url), + }) + end + + -- stop propagation + return true +end + function Wallabag:onSynchronizeWallabag() if not NetworkMgr:isOnline() then NetworkMgr:promptWifiOn() @@ -756,6 +777,9 @@ function Wallabag:onSynchronizeWallabag() end self:synchronize() self:refreshCurrentDirIfNeeded() + + -- stop propagation + return true end return Wallabag From cf6479d458e3c71e82ddac07a532ca8808337c11 Mon Sep 17 00:00:00 2001 From: Frans de Jonge Date: Sun, 31 Mar 2019 17:53:27 +0200 Subject: [PATCH 6/8] change menu order --- frontend/apps/reader/modules/readerlink.lua | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/frontend/apps/reader/modules/readerlink.lua b/frontend/apps/reader/modules/readerlink.lua index 147ddfac2b5b..4ec954810369 100644 --- a/frontend/apps/reader/modules/readerlink.lua +++ b/frontend/apps/reader/modules/readerlink.lua @@ -123,7 +123,6 @@ function ReaderLink:addToMainMenu(menu_items) G_reader_settings:saveSetting("tap_to_follow_links", not isTapToFollowLinksOn()) end, - separator = true, help_text = _([[Tap on links to follow them.]]), }, { @@ -169,6 +168,7 @@ function ReaderLink:addToMainMenu(menu_items) end, }, }, + separator = true, }, { text = _("Swipe to go back"), @@ -225,7 +225,6 @@ If any of the other Swipe to follow link options is enabled, this will work only -- even if the user enabled these features for EPUB documents). if not self.ui.document.info.has_pages then -- Tap section - menu_items.follow_links.sub_item_table[1].separator = nil table.insert(menu_items.follow_links.sub_item_table, 2, { text = _("Allow larger tap area around links"), enabled_func = isTapToFollowLinksOn, @@ -247,9 +246,8 @@ If any of the other Swipe to follow link options is enabled, this will work only help_text = _([[ Ignore taps on external links. Useful with Wikipedia EPUBs to make page turning easier. You can still follow them from the dictionary window or the selection menu after holding on them.]]), - separator = true, }) - table.insert(menu_items.follow_links.sub_item_table, 4, { + table.insert(menu_items.follow_links.sub_item_table, 5, { text = _("Show footnotes in popup"), enabled_func = function() return isTapToFollowLinksOn() or isSwipeToFollowNearestLinkEnabled() @@ -267,7 +265,7 @@ The footnote content may be empty, truncated, or include other footnotes. From the footnote popup, you can jump to the footnote location in the book by swiping to the left.]]), }) - table.insert(menu_items.follow_links.sub_item_table, 5, { + table.insert(menu_items.follow_links.sub_item_table, 6, { text = _("Show more links as footnotes"), enabled_func = function() return isFootnoteLinkInPopupEnabled() and @@ -280,7 +278,7 @@ From the footnote popup, you can jump to the footnote location in the book by sw end, help_text = _([[Loosen footnote detection rules to show more links as footnotes.]]), }) - table.insert(menu_items.follow_links.sub_item_table, 6, { + table.insert(menu_items.follow_links.sub_item_table, 7, { text = _("Set footnote popup font size"), enabled_func = function() return isFootnoteLinkInPopupEnabled() and @@ -716,7 +714,6 @@ function ReaderLink:onGoToExternalLink(link_url) UIManager:show(choose_action) elseif external_link_action == "add_to_wallabag" then - self.ui:handleEvent(Event:new("AddWallabagArticle", link_url)) elseif external_link_action == "open_in_browser" then Device:openLink(link_url) From 7bc6627c062b1bbb8fcc7f5c93adc53c0ae6f6de Mon Sep 17 00:00:00 2001 From: Frans de Jonge Date: Sun, 31 Mar 2019 19:06:00 +0200 Subject: [PATCH 7/8] enabled_func --- frontend/apps/reader/modules/readerlink.lua | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frontend/apps/reader/modules/readerlink.lua b/frontend/apps/reader/modules/readerlink.lua index 4ec954810369..ebfacf3b88e4 100644 --- a/frontend/apps/reader/modules/readerlink.lua +++ b/frontend/apps/reader/modules/readerlink.lua @@ -127,6 +127,9 @@ function ReaderLink:addToMainMenu(menu_items) }, { text = _("External link action"), + enabled_func = function() + return self.ui.document.info.has_pages or not isTapIgnoreExternalLinksEnabled + end, sub_item_table = { { text = _("Ask with pop-up dialog"), From a2054ce8fd33e9d1bac9cffc3ce81e3e819251ee Mon Sep 17 00:00:00 2001 From: Frans de Jonge Date: Sun, 31 Mar 2019 19:07:53 +0200 Subject: [PATCH 8/8] and now correctly --- frontend/apps/reader/modules/readerlink.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/apps/reader/modules/readerlink.lua b/frontend/apps/reader/modules/readerlink.lua index ebfacf3b88e4..7e366f33be71 100644 --- a/frontend/apps/reader/modules/readerlink.lua +++ b/frontend/apps/reader/modules/readerlink.lua @@ -128,7 +128,7 @@ function ReaderLink:addToMainMenu(menu_items) { text = _("External link action"), enabled_func = function() - return self.ui.document.info.has_pages or not isTapIgnoreExternalLinksEnabled + return self.ui.document.info.has_pages or not isTapIgnoreExternalLinksEnabled() end, sub_item_table = { {