From e7acec15261927eb5380813d85786b7b3108fc12 Mon Sep 17 00:00:00 2001 From: NiLuJe Date: Sat, 1 May 2021 18:53:04 +0200 Subject: [PATCH] ReaderUI: Saner FM/RD lifecycle * Ensure that going from one to the other tears down the former and its plugins before instantiating the latter and its plugins. UIManager: Unify Event sending & broadcasting * Make the two behave the same way (walk the widget stack from top to bottom), and properly handle the window stack shrinking shrinking *and* growing. Previously, broadcasting happened bottom-to-top and didn't really handle the list shrinking/growing, while sending only handled the list shrinking by a single element, and hopefully that element being the one the event was just sent to. These two items combined allowed us to optimize suboptimal refresh behavior with Menu and other Menu classes when opening/closing a document. e.g., the "opening document" Notification is now properly regional, and the "open last doc" option no longer flashes like a crazy person anymore. Plugins: Allow optimizing Menu refresh with custom menus, too. Requires moving Menu's close_callback *after* onMenuSelect, which, eh, probably makes sense, and is probably harmless in the grand scheme of things. --- frontend/apps/cloudstorage/dropbox.lua | 10 +++- frontend/apps/cloudstorage/ftp.lua | 10 +++- frontend/apps/cloudstorage/webdav.lua | 10 +++- frontend/apps/filemanager/filemanager.lua | 23 +++++++- .../filemanager/filemanagercollection.lua | 7 +-- .../filemanager/filemanagerfilesearcher.lua | 18 +++++- .../apps/filemanager/filemanagerhistory.lua | 7 +-- frontend/apps/filemanager/filemanagermenu.lua | 12 ++-- frontend/apps/reader/modules/readerfont.lua | 2 +- frontend/apps/reader/modules/readermenu.lua | 2 +- frontend/apps/reader/readerui.lua | 42 +++++++++++++- frontend/readhistory.lua | 11 ++-- frontend/ui/uimanager.lua | 51 ++++++++++------- frontend/ui/widget/dictquicklookup.lua | 3 + frontend/ui/widget/filechooser.lua | 3 - frontend/ui/widget/menu.lua | 17 +++++- frontend/ui/widget/touchmenu.lua | 11 +++- plugins/autosuspend.koplugin/main.lua | 4 +- plugins/calibre.koplugin/search.lua | 6 +- plugins/movetoarchive.koplugin/main.lua | 7 ++- plugins/opds.koplugin/opdscatalog.lua | 4 ++ reader.lua | 6 +- spec/unit/uimanager_spec.lua | 55 +++++++++++++++++-- 23 files changed, 245 insertions(+), 76 deletions(-) diff --git a/frontend/apps/cloudstorage/dropbox.lua b/frontend/apps/cloudstorage/dropbox.lua index 5c28ff1a3..bb8b6dbeb 100644 --- a/frontend/apps/cloudstorage/dropbox.lua +++ b/frontend/apps/cloudstorage/dropbox.lua @@ -21,7 +21,7 @@ function DropBox:showFiles(url, password) return DropBoxApi:showFiles(url, password) end -function DropBox:downloadFile(item, password, path, close) +function DropBox:downloadFile(item, password, path, callback_close) local code_response = DropBoxApi:downloadFile(item.url, password, path) if code_response == 200 then local __, filename = util.splitFilePathName(path) @@ -34,7 +34,13 @@ function DropBox:downloadFile(item, password, path, close) text = T(_("File saved to:\n%1\nWould you like to read the downloaded book now?"), BD.filepath(path)), ok_callback = function() - close() + local Event = require("ui/event") + UIManager:broadcastEvent(Event:new("SetupShowReader")) + + if callback_close then + callback_close() + end + ReaderUI:showReader(path) end }) diff --git a/frontend/apps/cloudstorage/ftp.lua b/frontend/apps/cloudstorage/ftp.lua index 398c09578..95c0bde6d 100644 --- a/frontend/apps/cloudstorage/ftp.lua +++ b/frontend/apps/cloudstorage/ftp.lua @@ -20,7 +20,7 @@ function Ftp:run(address, user, pass, path) return FtpApi:listFolder(url, path) end -function Ftp:downloadFile(item, address, user, pass, path, close) +function Ftp:downloadFile(item, address, user, pass, path, callback_close) local url = FtpApi:generateUrl(address, util.urlEncode(user), util.urlEncode(pass)) .. item.url logger.dbg("downloadFile url", url) path = util.fixUtf8(path, "_") @@ -43,7 +43,13 @@ function Ftp:downloadFile(item, address, user, pass, path, close) text = T(_("File saved to:\n%1\nWould you like to read the downloaded book now?"), BD.filepath(path)), ok_callback = function() - close() + local Event = require("ui/event") + UIManager:broadcastEvent(Event:new("SetupShowReader")) + + if callback_close then + callback_close() + end + ReaderUI:showReader(path) end }) diff --git a/frontend/apps/cloudstorage/webdav.lua b/frontend/apps/cloudstorage/webdav.lua index 61102406f..044b52b3f 100644 --- a/frontend/apps/cloudstorage/webdav.lua +++ b/frontend/apps/cloudstorage/webdav.lua @@ -17,7 +17,7 @@ function WebDav:run(address, user, pass, path) return WebDavApi:listFolder(address, user, pass, path) end -function WebDav:downloadFile(item, address, username, password, local_path, close) +function WebDav:downloadFile(item, address, username, password, local_path, callback_close) local code_response = WebDavApi:downloadFile(address .. WebDavApi:urlEncode( item.url ), username, password, local_path) if code_response == 200 then local __, filename = util.splitFilePathName(local_path) @@ -30,7 +30,13 @@ function WebDav:downloadFile(item, address, username, password, local_path, clos text = T(_("File saved to:\n%1\nWould you like to read the downloaded book now?"), BD.filepath(local_path)), ok_callback = function() - close() + local Event = require("ui/event") + UIManager:broadcastEvent(Event:new("SetupShowReader")) + + if callback_close then + callback_close() + end + ReaderUI:showReader(local_path) end }) diff --git a/frontend/apps/filemanager/filemanager.lua b/frontend/apps/filemanager/filemanager.lua index 241ea56a0..a95076dd7 100644 --- a/frontend/apps/filemanager/filemanager.lua +++ b/frontend/apps/filemanager/filemanager.lua @@ -222,7 +222,6 @@ function FileManager:setupLayout() end function file_chooser:onFileSelect(file) -- luacheck: ignore - FileManager.instance:onClose() ReaderUI:showReader(file) return true end @@ -507,6 +506,7 @@ function FileManager:setupLayout() end end +-- NOTE: The only thing that will *ever* instantiate a new FileManager object is our very own showFiles below! function FileManager:init() self:setupLayout() @@ -781,6 +781,21 @@ function FileManager:onClose() return true end +function FileManager:onShowingReader() + -- Allows us to optimize out a few useless refreshes in various CloseWidgets handlers... + self.tearing_down = true + -- Clear the dither flag to prevent it from infecting the queue and re-inserting a full-screen refresh... + self.dithered = nil + + self:onClose() +end + +-- Same as above, except we don't close it yet. Useful for plugins that need to close custom Menus before calling showReader. +function FileManager:onSetupShowReader() + self.tearing_down = true + self.dithered = nil +end + function FileManager:onRefresh() self.file_chooser:refreshPath() return true @@ -826,9 +841,7 @@ function FileManager:openRandomFile(dir) text = T(_("Do you want to open %1?"), BD.filename(BaseUtil.basename(random_file))), choice1_text = _("Open"), choice1_callback = function() - FileManager.instance:onClose() ReaderUI:showReader(random_file) - end, -- @translators Another file. This is a button on the open random file dialog. It presents a file with the choices Open/Another. choice2_text = _("Another"), @@ -1152,6 +1165,10 @@ function FileManager:showFiles(path, focused_file) end } UIManager:show(file_manager) + + -- NOTE: This is a bit clunky. This ought to be private and accessed via a getCurrentInstance method, àla ReaderUI. + -- But, it points to the *current* FM instance, and is nil'ed on exit. + -- As such, code outside of FileManager can just check/use FileManager.instance (which they do. extensively). self.instance = file_manager end diff --git a/frontend/apps/filemanager/filemanagercollection.lua b/frontend/apps/filemanager/filemanagercollection.lua index 6301fa433..235bc4552 100644 --- a/frontend/apps/filemanager/filemanagercollection.lua +++ b/frontend/apps/filemanager/filemanagercollection.lua @@ -166,12 +166,7 @@ function FileManagerCollection:onShowColl(collection) self:updateItemTable() self.coll_menu.close_callback = function() - -- Close it at next tick so it stays displayed - -- while a book is opening (avoids a transient - -- display of the underlying File Browser) - UIManager:nextTick(function() - UIManager:close(self.coll_menu) - end) + UIManager:close(self.coll_menu) end UIManager:show(self.coll_menu) return true diff --git a/frontend/apps/filemanager/filemanagerfilesearcher.lua b/frontend/apps/filemanager/filemanagerfilesearcher.lua index 2bc8f9ad5..f8a6962a4 100644 --- a/frontend/apps/filemanager/filemanagerfilesearcher.lua +++ b/frontend/apps/filemanager/filemanagerfilesearcher.lua @@ -42,10 +42,24 @@ function FileSearcher:readDir() -- Don't traverse hidden folders if we're not showing them if attributes.mode == "directory" and f ~= "." and f ~= ".." and (G_reader_settings:isTrue("show_hidden") or not util.stringStartsWith(f, ".")) then table.insert(new_dirs, fullpath) - table.insert(self.files, {name = f, text = f.."/", attr = attributes, callback = function() FileManager:showFiles(fullpath) end}) + table.insert(self.files, { + name = f, + text = f.."/", + attr = attributes, + callback = function() + FileManager:showFiles(fullpath) + end, + }) -- Always ignore macOS resource forks, too. elseif attributes.mode == "file" and not util.stringStartsWith(f, "._") and (show_unsupported or DocumentRegistry:hasProvider(fullpath)) then - table.insert(self.files, {name = f, text = f, attr = attributes, callback = function() ReaderUI:showReader(fullpath) end}) + table.insert(self.files, { + name = f, + text = f, + attr = attributes, + callback = function() + ReaderUI:showReader(fullpath) + end, + }) end end end diff --git a/frontend/apps/filemanager/filemanagerhistory.lua b/frontend/apps/filemanager/filemanagerhistory.lua index fb1a34a2b..f71a11f6d 100644 --- a/frontend/apps/filemanager/filemanagerhistory.lua +++ b/frontend/apps/filemanager/filemanagerhistory.lua @@ -155,12 +155,7 @@ function FileManagerHistory:onShowHist() self:updateItemTable() self.hist_menu.close_callback = function() - -- Close it at next tick so it stays displayed - -- while a book is opening (avoids a transient - -- display of the underlying File Browser) - UIManager:nextTick(function() - UIManager:close(self.hist_menu) - end) + UIManager:close(self.hist_menu) end UIManager:show(self.hist_menu) return true diff --git a/frontend/apps/filemanager/filemanagermenu.lua b/frontend/apps/filemanager/filemanagermenu.lua index 09223d3c3..12fac05cd 100644 --- a/frontend/apps/filemanager/filemanagermenu.lua +++ b/frontend/apps/filemanager/filemanagermenu.lua @@ -122,13 +122,17 @@ function FileManagerMenu:onOpenLastDoc() }) return end - local ReaderUI = require("apps/reader/readerui") - ReaderUI:showReader(last_file) - -- only close menu if we were called from the menu + -- Only close menu if we were called from the menu if self.menu_container then + -- Mimic's FileManager's onShowingReader refresh optimizations + self.ui.tearing_down = true + self.ui.dithered = nil self:onCloseFileManagerMenu() end + + local ReaderUI = require("apps/reader/readerui") + ReaderUI:showReader(last_file) end function FileManagerMenu:setUpdateItemTable() @@ -784,7 +788,7 @@ function FileManagerMenu:onShowMenu(tab_index) } end - main_menu.close_callback = function () + main_menu.close_callback = function() self:onCloseFileManagerMenu() end diff --git a/frontend/apps/reader/modules/readerfont.lua b/frontend/apps/reader/modules/readerfont.lua index 412a3a0e2..42e36e1cf 100644 --- a/frontend/apps/reader/modules/readerfont.lua +++ b/frontend/apps/reader/modules/readerfont.lua @@ -188,7 +188,7 @@ function ReaderFont:onShowFontMenu() dimen = Screen:getSize(), main_menu, } - main_menu.close_callback = function () + main_menu.close_callback = function() UIManager:close(menu_container) end -- show menu diff --git a/frontend/apps/reader/modules/readermenu.lua b/frontend/apps/reader/modules/readermenu.lua index f69e24c32..8a3de60e8 100644 --- a/frontend/apps/reader/modules/readermenu.lua +++ b/frontend/apps/reader/modules/readermenu.lua @@ -376,7 +376,7 @@ function ReaderMenu:onShowMenu(tab_index) } end - main_menu.close_callback = function () + main_menu.close_callback = function() self.ui:handleEvent(Event:new("CloseReaderMenu")) end diff --git a/frontend/apps/reader/readerui.lua b/frontend/apps/reader/readerui.lua index ee4c59de8..4217054f2 100644 --- a/frontend/apps/reader/readerui.lua +++ b/frontend/apps/reader/readerui.lua @@ -480,6 +480,21 @@ function ReaderUI:showFileManager(file) end end +function ReaderUI:onShowingReader() + -- Allows us to optimize out a few useless refreshes in various CloseWidgets handlers... + self.tearing_down = true + self.dithered = nil + + self:onClose() +end + +-- Same as above, except we don't close it yet. Useful for plugins that need to close custom Menus before calling showReader. +function ReaderUI:onSetupShowReader() + self.tearing_down = true + self.dithered = nil +end + +--- @note: Will sanely close existing FileManager/ReaderUI instance for you! function ReaderUI:showReader(file, provider) logger.dbg("show reader ui") @@ -497,6 +512,10 @@ function ReaderUI:showReader(file, provider) self:showFileManager(file) return end + + -- We can now signal the existing ReaderUI/FileManager instances that it's time to go bye-bye... + UIManager:broadcastEvent(Event:new("ShowingReader")) + -- prevent crash due to incompatible bookmarks --- @todo Split bookmarks from metadata and do per-engine in conversion. provider = provider or DocumentRegistry:getProvider(file) @@ -550,7 +569,7 @@ end local _running_instance = nil function ReaderUI:doShowReader(file, provider) logger.info("opening file", file) - -- keep only one instance running + -- Keep only one instance running if _running_instance then _running_instance:onClose() end @@ -591,12 +610,17 @@ function ReaderUI:doShowReader(file, provider) end Device:notifyBookState(title, document) - UIManager:show(reader) - _running_instance = reader + -- This is mostly for the few callers that bypass the coroutine shenanigans and call doShowReader directly, + -- instead of showReader... + -- Otherwise, showReader will have taken care of that *before* instantiating a new RD, + -- in order to ensure a sane ordering of plugins teardown -> instantiation. local FileManager = require("apps/filemanager/filemanager") if FileManager.instance then FileManager.instance:onClose() end + + UIManager:show(reader) + _running_instance = reader end function ReaderUI:_getRunningInstance() @@ -756,6 +780,11 @@ end function ReaderUI:reloadDocument(after_close_callback) local file = self.document.file local provider = getmetatable(self.document).__index + + -- Mimic onShowingReader's refresh optimizations + self.tearing_down = true + self.dithered = nil + self:handleEvent(Event:new("CloseReaderMenu")) self:handleEvent(Event:new("CloseConfigMenu")) self.highlight:onClose() -- close highlight dialog if any @@ -764,15 +793,22 @@ function ReaderUI:reloadDocument(after_close_callback) -- allow caller to do stuff between close an re-open after_close_callback(file, provider) end + self:showReader(file, provider) end function ReaderUI:switchDocument(new_file) if not new_file then return end + + -- Mimic onShowingReader's refresh optimizations + self.tearing_down = true + self.dithered = nil + self:handleEvent(Event:new("CloseReaderMenu")) self:handleEvent(Event:new("CloseConfigMenu")) self.highlight:onClose() -- close highlight dialog if any self:onClose(false) + self:showReader(new_file) end diff --git a/frontend/readhistory.lua b/frontend/readhistory.lua index 5fe43fafe..9cb0f326f 100644 --- a/frontend/readhistory.lua +++ b/frontend/readhistory.lua @@ -14,6 +14,11 @@ local ReadHistory = { last_read_time = 0, } +local function selectCallback(path) + local ReaderUI = require("apps/reader/readerui") + ReaderUI:showReader(path) +end + local function buildEntry(input_time, input_file) local file_path = realpath(input_file) or input_file -- keep orig file path of deleted files local file_exists = lfs.attributes(file_path, "mode") == "file" @@ -43,8 +48,7 @@ local function buildEntry(input_time, input_file) return lfs.attributes(file_path, "mode") == "file" end, callback = function() - local ReaderUI = require("apps/reader/readerui") - ReaderUI:showReader(input_file) + selectCallback(input_file) end } end @@ -245,8 +249,7 @@ function ReadHistory:updateItemByPath(old_path, new_path) self.hist[i].text = new_path:gsub(".*/", "") self:_flush() self.hist[i].callback = function() - local ReaderUI = require("apps/reader/readerui") - ReaderUI:showReader(new_path) + selectCallback(new_path) end break end diff --git a/frontend/ui/uimanager.lua b/frontend/ui/uimanager.lua index 968253007..103535c29 100644 --- a/frontend/ui/uimanager.lua +++ b/frontend/ui/uimanager.lua @@ -1076,7 +1076,10 @@ function UIManager:discardEvents(set_or_seconds) end --[[-- -Transmits an @{ui.event.Event|Event} to active widgets. +Transmits an @{ui.event.Event|Event} to active widgets, top to bottom. +Stops at the first handler that returns `true`. +Note that most complex widgets are based on @{ui.widget.container.WidgetContainer|WidgetContainer}, +which itself will take care of propagating an event to its members. @param event an @{ui.event.Event|Event} object ]] @@ -1113,26 +1116,34 @@ function UIManager:sendEvent(event) end end - -- if the event is not consumed, active widgets (from top to bottom) can access it. - -- NOTE: _window_stack can shrink when widgets are closed (CloseWidget & Close events). + -- If the event was not consumed (no handler returned true), active widgets (from top to bottom) can access it. + -- NOTE: _window_stack can shrink/grow when widgets are closed (CloseWidget & Close events) or opened. + -- Simply looping in reverse would only cover the list shrinking, and that only by a *single* element, + -- something we can't really guarantee, hence the more dogged iterator below, + -- which relies on a hash check of already processed widgets (LuaJIT actually hashes the table's GC reference), + -- rather than a simple loop counter, and will in fact iterate *at least* #items ^ 2 times. + -- Thankfully, that list should be very small, so the overhead should be minimal. local checked_widgets = {top_widget} - for i = #self._window_stack, 1, -1 do + local i = #self._window_stack + while i > 0 do local widget = self._window_stack[i] if checked_widgets[widget] == nil then - -- active widgets has precedence to handle this event + checked_widgets[widget] = true + -- Widget's active widgets have precedence to handle this event -- NOTE: While FileManager only has a single (screenshotter), ReaderUI has many active_widgets (each ReaderUI module gets added to the list). if widget.widget.active_widgets then - checked_widgets[widget] = true for _, active_widget in ipairs(widget.widget.active_widgets) do if active_widget:handleEvent(event) then return end end end if widget.widget.is_always_active then - -- active widgets will handle this event + -- Widget itself is flagged always active, let it handle the event -- NOTE: is_always_active widgets currently are widgets that want to show a VirtualKeyboard or listen to Dispatcher events - checked_widgets[widget] = true if widget.widget:handleEvent(event) then return end end + i = #self._window_stack + else + i = i - 1 end end end @@ -1143,18 +1154,18 @@ Transmits an @{ui.event.Event|Event} to all registered widgets. @param event an @{ui.event.Event|Event} object ]] function UIManager:broadcastEvent(event) - -- the widget's event handler might close widgets in which case - -- a simple iterator like ipairs would skip over some entries - local i = 1 - while i <= #self._window_stack do - local prev_widget = self._window_stack[i].widget - self._window_stack[i].widget:handleEvent(event) - local top_widget = self._window_stack[i] - if top_widget == nil then - -- top widget closed itself - break - elseif top_widget.widget == prev_widget then - i = i + 1 + -- Unlike sendEvent, we send the event to *all* (window-level) widgets (i.e., we don't stop, even if a handler returns true). + -- NOTE: Same defensive approach to _window_stack changing from under our feet as above. + local checked_widgets = {} + local i = #self._window_stack + while i > 0 do + local widget = self._window_stack[i] + if checked_widgets[widget] == nil then + checked_widgets[widget] = true + widget.widget:handleEvent(event) + i = #self._window_stack + else + i = i - 1 end end end diff --git a/frontend/ui/widget/dictquicklookup.lua b/frontend/ui/widget/dictquicklookup.lua index 9712c45d6..bc314bd44 100644 --- a/frontend/ui/widget/dictquicklookup.lua +++ b/frontend/ui/widget/dictquicklookup.lua @@ -392,6 +392,8 @@ function DictQuickLookup:init() self:onHoldClose(true) -- close current ReaderUI in 1 sec, and create a new one UIManager:scheduleIn(1.0, function() + UIManager:broadcastEvent(Event:new("SetupShowReader")) + if self.ui then -- close Highlight menu if any still shown if self.ui.highlight and self.ui.highlight.highlight_dialog then @@ -399,6 +401,7 @@ function DictQuickLookup:init() end self.ui:onClose() end + self.ui:showReader(epub_path) end) end, diff --git a/frontend/ui/widget/filechooser.lua b/frontend/ui/widget/filechooser.lua index 5dea49edb..56636acc1 100644 --- a/frontend/ui/widget/filechooser.lua +++ b/frontend/ui/widget/filechooser.lua @@ -540,7 +540,6 @@ function FileChooser:showSetProviderButtons(file, filemanager_instance, reader_u ok_callback = function() DocumentRegistry:setProvider(file, provider, false) - filemanager_instance:onClose() reader_ui:showReader(file, provider) UIManager:close(self.set_provider_dialog) end, @@ -554,14 +553,12 @@ function FileChooser:showSetProviderButtons(file, filemanager_instance, reader_u ok_callback = function() DocumentRegistry:setProvider(file, provider, true) - filemanager_instance:onClose() reader_ui:showReader(file, provider) UIManager:close(self.set_provider_dialog) end, }) else -- just once - filemanager_instance:onClose() reader_ui:showReader(file, provider) UIManager:close(self.set_provider_dialog) end diff --git a/frontend/ui/widget/menu.lua b/frontend/ui/widget/menu.lua index 112c41be0..dc303c6d4 100644 --- a/frontend/ui/widget/menu.lua +++ b/frontend/ui/widget/menu.lua @@ -955,6 +955,12 @@ function Menu:init() end end +function Menu:onShowingReader() + -- Clear the dither flag to prevent it from infecting the queue and re-inserting a full-screen refresh... + self.dithered = nil +end +Menu.onSetupShowReader = Menu.onShowingReader + function Menu:onCloseWidget() --- @fixme -- we cannot refresh regionally using the dimen field @@ -965,7 +971,14 @@ function Menu:onCloseWidget() -- the filemanager menu. -- NOTE: For the same reason, don't make it flash, -- because that'll trigger when we close the FM and open a book... - UIManager:setDirty(nil, "partial") + + -- Don't do anything if we're in the process of tearing down FM or RD, or if we don't actually have a live instance of 'em... + local FileManager = require("apps/filemanager/filemanager") + local ReaderUI = require("apps/reader/readerui") + local reader_ui = ReaderUI:_getRunningInstance() + if (FileManager.instance and not FileManager.instance.tearing_down) or (reader_ui and not reader_ui.tearing_down) then + UIManager:setDirty(nil, "ui") + end end function Menu:updatePageInfo(select_number) @@ -1192,10 +1205,10 @@ function Menu:onMenuSelect(item) return true end end + self:onMenuChoice(item) if self.close_callback then self.close_callback() end - self:onMenuChoice(item) else -- save menu title for later resume self.item_table.title = self.title diff --git a/frontend/ui/widget/touchmenu.lua b/frontend/ui/widget/touchmenu.lua index 01186215d..60a7caa5a 100644 --- a/frontend/ui/widget/touchmenu.lua +++ b/frontend/ui/widget/touchmenu.lua @@ -592,8 +592,15 @@ function TouchMenu:init() end function TouchMenu:onCloseWidget() - -- NOTE: We don't pass a region in order to ensure a full-screen flash to avoid ghosting - UIManager:setDirty(nil, "flashui") + -- NOTE: We don't pass a region in order to ensure a full-screen flash to avoid ghosting, + -- but we only need to do that if we actually have a FM or RD below us. + -- Don't do anything when we're switching between the two, or if we don't actually have a live instance of 'em... + local FileManager = require("apps/filemanager/filemanager") + local ReaderUI = require("apps/reader/readerui") + local reader_ui = ReaderUI:_getRunningInstance() + if (FileManager.instance and not FileManager.instance.tearing_down) or (reader_ui and not reader_ui.tearing_down) then + UIManager:setDirty(nil, "flashui") + end end function TouchMenu:_recalculatePageLayout() diff --git a/plugins/autosuspend.koplugin/main.lua b/plugins/autosuspend.koplugin/main.lua index 0369bfabb..958da96ba 100644 --- a/plugins/autosuspend.koplugin/main.lua +++ b/plugins/autosuspend.koplugin/main.lua @@ -110,8 +110,8 @@ function AutoSuspend:init() self.auto_suspend_timeout_seconds = G_reader_settings:readSetting("auto_suspend_timeout_seconds") or default_auto_suspend_timeout_seconds UIManager.event_hook:registerWidget("InputEvent", self) - -- We need an instance-specific function reference to schedule, because when going from FM to RD, - -- we instantiate the RD instance *before* tearing down the old FM instance. + -- We need an instance-specific function reference to schedule, because in some rare cases, + -- we may instantiate a new plugin instance *before* tearing down the old one. self.task = function(shutdown_only) self:_schedule(shutdown_only) end diff --git a/plugins/calibre.koplugin/search.lua b/plugins/calibre.koplugin/search.lua index bd0c9c7cf..6f3e30909 100644 --- a/plugins/calibre.koplugin/search.lua +++ b/plugins/calibre.koplugin/search.lua @@ -283,9 +283,13 @@ function CalibreSearch:bookCatalog(t, option) entry.text = string.format("%s - %s", book.title, book.authors[1]) end entry.callback = function() + local Event = require("ui/event") + UIManager:broadcastEvent(Event:new("SetupShowReader")) + + self.search_menu:onClose() + local ReaderUI = require("apps/reader/readerui") ReaderUI:showReader(book.rootpath .. "/" .. book.lpath) - self.search_menu:onClose() end table.insert(catalog, entry) end diff --git a/plugins/movetoarchive.koplugin/main.lua b/plugins/movetoarchive.koplugin/main.lua index 9d59603fb..74c702222 100644 --- a/plugins/movetoarchive.koplugin/main.lua +++ b/plugins/movetoarchive.koplugin/main.lua @@ -97,6 +97,9 @@ function MoveToArchive:commonProcess(is_move_process, moved_done_text) self.settings:saveSetting("last_copied_from_dir", self.last_copied_from_dir) self.settings:flush() + local Event = require("ui/event") + UIManager:broadcastEvent(Event:new("SetupShowReader")) + self.ui:onClose() if is_move_process then FileManager:moveFile(document_full_path, self.archive_dir_path) @@ -110,10 +113,10 @@ function MoveToArchive:commonProcess(is_move_process, moved_done_text) ReadCollection:updateItemByPath(document_full_path, dest_file) UIManager:show(ConfirmBox:new{ text = moved_done_text, - ok_callback = function () + ok_callback = function() ReaderUI:showReader(dest_file) end, - cancel_callback = function () + cancel_callback = function() self:openFileBrowser(self.last_copied_from_dir) end, }) diff --git a/plugins/opds.koplugin/opdscatalog.lua b/plugins/opds.koplugin/opdscatalog.lua index b2f4d1dfc..56021d5b1 100644 --- a/plugins/opds.koplugin/opdscatalog.lua +++ b/plugins/opds.koplugin/opdscatalog.lua @@ -31,7 +31,11 @@ function OPDSCatalog:init() ok_text = _("Read now"), cancel_text = _("Read later"), ok_callback = function() + local Event = require("ui/event") + UIManager:broadcastEvent(Event:new("SetupShowReader")) + self:onClose() + ReaderUI:showReader(downloaded_file) end }) diff --git a/reader.lua b/reader.lua index f317f14c3..797449bef 100755 --- a/reader.lua +++ b/reader.lua @@ -284,6 +284,7 @@ else if start_with == "last" and last_file then local ReaderUI = require("apps/reader/readerui") UIManager:nextTick(function() + -- Instantiate RD ReaderUI:showReader(last_file) end) exit_code = UIManager:run() @@ -292,10 +293,11 @@ else local home_dir = G_reader_settings:readSetting("home_dir") or Device.home_dir or lfs.currentdir() UIManager:nextTick(function() + -- Instantiate FM FileManager:showFiles(home_dir) end) - -- Always open history on top of filemanager so closing history - -- doesn't result in exit. + -- Always open FM modules on top of filemanager, so closing 'em doesn't result in an exit + -- because of an empty widget stack, and so they can interact with the FM instance as expected. if start_with == "history" then local FileManagerHistory = require("apps/filemanager/filemanagerhistory") UIManager:nextTick(function() diff --git a/spec/unit/uimanager_spec.lua b/spec/unit/uimanager_spec.lua index 14b3a6b02..3e7eb194c 100644 --- a/spec/unit/uimanager_spec.lua +++ b/spec/unit/uimanager_spec.lua @@ -326,33 +326,76 @@ describe("UIManager spec", function() UIManager:broadcastEvent("foo") assert.is.same(#UIManager._window_stack, 0) + -- Remember that the stack is processed top to bottom! + -- Test making a hole in the middle of the stack. UIManager._window_stack = { { widget = { handleEvent = function() - UIManager._window_stack[1] = nil - UIManager._window_stack[2] = nil - UIManager._window_stack[3] = nil + assert.truthy(true) + end + } + }, + { + widget = { + handleEvent = function() + assert.falsy(true) + end + } + }, + { + widget = { + handleEvent = function() + assert.falsy(true) + end + } + }, + { + widget = { + handleEvent = function() + table.remove(UIManager._window_stack, #UIManager._window_stack - 2) + table.remove(UIManager._window_stack, #UIManager._window_stack - 2) + table.remove(UIManager._window_stack, #UIManager._window_stack - 1) + end + } + }, + { + widget = { + handleEvent = function() + assert.truthy(true) end } }, + } + UIManager:broadcastEvent("foo") + assert.is.same(2, #UIManager._window_stack) + + -- Test inserting a new widget in the stack + local new_widget = { + widget = { + handleEvent = function() + assert.truthy(true) + end + } + } + UIManager._window_stack = { { widget = { handleEvent = function() - assert.falsy(true); + table.insert(UIManager._window_stack, new_widget) end } }, { widget = { handleEvent = function() - assert.falsy(true); + assert.truthy(true) end } }, } UIManager:broadcastEvent("foo") - assert.is.same(0, #UIManager._window_stack) + assert.is.same(3, #UIManager._window_stack) end) it("should handle stack change when closing widgets", function()