fix(reader): preserve third-party KOReader menu entries

Extend KOReader's cached menu order instead of persisting complete
override lists. Remove marker-owned legacy overrides during upgrade
while preserving hand-maintained entries.

Closes #1878

koreader-plugin/README.md

@@ -40,7 +40,7 @@ Interface size defaults to 1.05x when no preference is saved. Existing scale cho
 
 To add a companion, choose Customize > Add widget > Decor: Reading dog, Reading rabbit, Reading owl, Reading penguin, Reading robot, Reading vampire, Reading dragon or Reading ghost. The original Reading buddy is the cat. Each companion reacts to reading, idle time, streak milestones, finishing a book and late-night sessions, with six distinct pixel-art poses. All support small, medium and large tiles; each widget keeps its selected character when the dashboard is saved.
 
-The plugin manages `settings/reader_menu_order.lua` and `settings/filemanager_menu_order.lua` to keep its menu entry pinned below Calibre and its icon in the menu bar across updates; delete those files to reset your menu order.
+The plugin extends KOReader's live menu order to keep its Tools entry below Calibre and its icon just before the main menu without replacing tabs or tools added by other plugins. Older BookOrbit-generated menu-order files are removed automatically; hand-maintained menu-order files keep their other entries.
 
 Settings > About & updates checks for plugin updates; an available update also shows in the BookOrbit menu. Sync status shows whether sync is working, recent sync activity, and Technical details for bug reports.
 

koreader-plugin/bookorbit.koplugin/bookorbit_menu_pin.lua

@@ -1,14 +1,14 @@
 --[[--
 Pins the BookOrbit menu entry onto the first page of the Tools menu, right
 below calibre, and puts BookOrbit's icon in KOReader's menu bar just before
-the main menu, by maintaining user menu-order override files.
+the main menu.
 
-KOReader merges <settings_dir>/<prefix>_menu_order.lua over the bundled order
-tables per top-level key, so the generated files must carry the complete tools
-list. Files created here start with a marker comment and are regenerated from
-the bundled order on every startup, so KOReader updates never drift. A
-hand-maintained file only ever gets old BookOrbit ids replaced by the current
-one.
+KOReader caches its bundled menu-order tables and lets plugins extend them
+before the menu is built. Mutating those shared tables preserves entries added
+by every plugin regardless of load order. Older BookOrbit releases instead
+wrote complete user override lists, which hid entries added by later plugins;
+those marker-owned files are removed during migration. A hand-maintained file
+keeps its other entries and only has BookOrbit ids normalized.
 ]]
 
 local DataStorage = require("datastorage")
@@ -35,6 +35,8 @@ local PREFIXES = {
     { prefix = "filemanager", order_module = "ui/elements/filemanager_menu_order" },
 }
 
+local EMPTY_OVERRIDE = MARKER .. "\nreturn {}\n"
+
 local BookOrbitMenuPin = {
     done = false,
     -- Whether BookOrbit's icon belongs in the menu bar; set from the
@@ -92,30 +94,14 @@ local function normalizeButtons(buttons, with_tab)
 end
 BookOrbitMenuPin.normalizeButtons = normalizeButtons
 
-local function copyList(list)
-    local out = {}
-    for _, id in ipairs(list) do table.insert(out, id) end
-    return out
-end
-
-local function desiredOrder(order_module)
-    local bundled = require(order_module)
-    if type(bundled) ~= "table" or type(bundled.tools) ~= "table" then
-        return nil
-    end
-    local order = { tools = copyList(bundled.tools) }
+local function normalizeRuntimeOrder(order)
+    if type(order) ~= "table" or type(order.tools) ~= "table" then return end
     normalizeBookOrbitIds(order.tools)
-    if type(bundled[BUTTONS]) == "table" then
-        order[BUTTONS] = copyList(bundled[BUTTONS])
+    if type(order[BUTTONS]) == "table" then
         normalizeButtons(order[BUTTONS], BookOrbitMenuPin.tab)
         -- MenuSorter treats a button as a tab only when it has an order list.
-        if BookOrbitMenuPin.tab then order[TAB_ID] = {} end
+        order[TAB_ID] = BookOrbitMenuPin.tab and {} or nil
     end
-    return order
-end
-
-local function serializeOurs(order)
-    return MARKER .. "\nreturn " .. dump(order, nil, true) .. "\n"
 end
 
 local function isGeneratedByUs(existing)
@@ -130,19 +116,9 @@ local function isGeneratedByUs(existing)
     return false
 end
 
-local function ensureOne(prefix, order_module)
-    local order = desiredOrder(order_module)
-    if not order then return end
-
+local function migrateOverride(prefix)
     local path = DataStorage:getSettingsDir() .. "/" .. prefix .. "_menu_order.lua"
-
-    if not lfs.attributes(path) then
-        local ok, err = util.writeToFile(serializeOurs(order), path, true)
-        if not ok then
-            logger.warn("BookOrbit: cannot write menu order file", path, err)
-        end
-        return
-    end
+    if not lfs.attributes(path) then return end
 
     local existing = util.readFromFile(path, "r")
     if not existing then
@@ -151,11 +127,11 @@ local function ensureOne(prefix, order_module)
     end
 
     if isGeneratedByUs(existing) then
-        local content = serializeOurs(order)
-        if content ~= existing then
-            local ok, err = util.writeToFile(content, path, true)
-            if not ok then
-                logger.warn("BookOrbit: cannot update menu order file", path, err)
+        local ok, err = os.remove(path)
+        if not ok then
+            local cleared, clear_err = util.writeToFile(EMPTY_OVERRIDE, path, true)
+            if not cleared then
+                logger.warn("BookOrbit: cannot clear generated menu order file", path, err, clear_err)
             end
         end
         return
@@ -169,22 +145,29 @@ local function ensureOne(prefix, order_module)
     end
     if type(user.tools) == "table" then
         normalizeBookOrbitIds(user.tools)
-    else
-        user.tools = order.tools
     end
-    -- A hand-kept menu bar keeps its own order; only BookOrbit's tab moves.
     if type(user[BUTTONS]) == "table" then
         normalizeButtons(user[BUTTONS], BookOrbitMenuPin.tab)
-    elseif order[BUTTONS] then
-        user[BUTTONS] = order[BUTTONS]
+        user[TAB_ID] = BookOrbitMenuPin.tab and {} or nil
+    else
+        user[TAB_ID] = nil
     end
-    user[TAB_ID] = BookOrbitMenuPin.tab and {} or nil
     local ok, err = util.writeToFile(dump(user, nil, true), path, true, true)
     if not ok then
         logger.warn("BookOrbit: cannot update user menu order file", path, err)
     end
 end
 
+local function ensureOne(prefix, order_module)
+    local ok, order = pcall(require, order_module)
+    if not ok then
+        logger.warn("BookOrbit: cannot load menu order", order_module, order)
+        return
+    end
+    normalizeRuntimeOrder(order)
+    migrateOverride(prefix)
+end
+
 local function ensureAll()
     for _, entry in ipairs(PREFIXES) do
         local ok, err = pcall(ensureOne, entry.prefix, entry.order_module)
@@ -201,7 +184,7 @@ function BookOrbitMenuPin.ensure(with_tab)
     ensureAll()
 end
 
--- Rewrites both order files after the menu bar setting changes.
+-- Updates both cached orders after the menu bar setting changes.
 function BookOrbitMenuPin.refresh(with_tab)
     BookOrbitMenuPin.tab = with_tab ~= false
     BookOrbitMenuPin.done = true

koreader-plugin/spec/bookorbit_menu_pin_test.lua

@@ -1,5 +1,5 @@
--- Proves where BookOrbit's icon goes in KOReader's menu bar: just before the
--- main menu, once, and nowhere when it is turned off.
+-- Proves where BookOrbit's icon goes in KOReader's menu bar, then proves
+-- later plugin entries survive and old complete-list files are migrated.
 
 package.loaded["datastorage"] = { getSettingsDir = function() return "/tmp" end }
 package.loaded["dump"] = function() return "" end
@@ -38,4 +38,97 @@ local custom = { "tools", "search" }
 MenuPin.normalizeButtons(custom, true)
 assertEqual(custom[3], "bookorbit_tab", "no main button: the icon goes last")
 
+package.loaded["bookorbit_menu_pin"] = nil
+package.path = "koreader-plugin/spec/?.lua;koreader-plugin/spec/helpers/?.lua;" .. package.path
+local TempFilesystem = require("temp_filesystem")
+local fs = TempFilesystem.new()
+
+local function readFile(path)
+    local file = io.open(path, "rb")
+    if not file then return nil end
+    local content = file:read("*a")
+    file:close()
+    return content
+end
+
+local filemanager_order = {
+    ["KOMenu:menu_buttons"] = {
+        "filemanager_settings", "setting", "tools", "search", "plus_menu", "main",
+    },
+    tools = { "read_timer", "calibre", "exporter" },
+}
+local reader_order = {
+    ["KOMenu:menu_buttons"] = { "navi", "typeset", "setting", "tools", "search", "filemanager", "main" },
+    tools = { "read_timer", "calibre", "exporter" },
+}
+
+package.loaded["datastorage"] = { getSettingsDir = function() return fs.path end }
+package.loaded["dump"] = function(value) return fs:serialize(value):gsub("\n$", ""):gsub("^return ", "") end
+package.loaded["libs/libkoreader-lfs"] = {
+    attributes = function(path, attribute) return fs:attributes(path, attribute) end,
+}
+package.loaded["logger"] = { warn = function(...) error(table.concat({ ... }, " ")) end }
+package.loaded["util"] = {
+    readFromFile = readFile,
+    writeToFile = function(content, path, _force_flush, lua_dofile_ready)
+        if lua_dofile_ready then
+            content = "-- " .. path .. "\nreturn " .. content .. "\n"
+        end
+        return fs:write(content, path)
+    end,
+}
+package.loaded["ui/elements/filemanager_menu_order"] = filemanager_order
+package.loaded["ui/elements/reader_menu_order"] = reader_order
+
+local IntegrationMenuPin = require("bookorbit_menu_pin")
+
+local generated_path = fs.path .. "/filemanager_menu_order.lua"
+assert(fs:write(
+    "-- bookorbit-menu-pin v3 (generated; delete this file to reset menu order)\n"
+        .. "return { tools = { 'calibre', 'bookorbit' }, "
+        .. "['KOMenu:menu_buttons'] = { 'tools', 'bookorbit_tab', 'main' } }\n",
+    generated_path))
+
+local hand_path = fs.path .. "/reader_menu_order.lua"
+assert(fs:write(fs:serialize({
+    ["KOMenu:menu_buttons"] = { "navi", "custom_tab", "tools", "main" },
+    custom_tab = { "custom_action" },
+    tools = { "calibre", "custom_tool" },
+}), hand_path))
+
+IntegrationMenuPin.ensure(true)
+assertEqual(fs:attributes(generated_path), nil, "generated overrides are removed")
+assertEqual(table.concat(filemanager_order["KOMenu:menu_buttons"], ","),
+    "filemanager_settings,setting,tools,search,plus_menu,bookorbit_tab,main",
+    "BookOrbit extends the shared file-manager button order")
+assertEqual(table.concat(filemanager_order.tools, ","), "read_timer,calibre,bookorbit,exporter",
+    "BookOrbit extends the shared tools order")
+
+-- Bookshelf and another plugin load after BookOrbit.
+table.insert(filemanager_order["KOMenu:menu_buttons"], 2, "bookshelf_tab")
+filemanager_order.bookshelf_tab = { "bookshelf_toggle" }
+table.insert(filemanager_order.tools, 2, "later_plugin_tool")
+assertEqual(table.concat(filemanager_order["KOMenu:menu_buttons"], ","),
+    "filemanager_settings,bookshelf_tab,setting,tools,search,plus_menu,bookorbit_tab,main",
+    "later plugin tabs remain in the shared order")
+assertEqual(table.concat(filemanager_order.tools, ","),
+    "read_timer,later_plugin_tool,calibre,bookorbit,exporter",
+    "later plugin tools remain in the shared order")
+
+local hand_order = assert(loadfile(hand_path))()
+assertEqual(table.concat(hand_order["KOMenu:menu_buttons"], ","),
+    "navi,custom_tab,tools,bookorbit_tab,main", "hand-maintained tabs are preserved")
+assertEqual(table.concat(hand_order.tools, ","), "calibre,bookorbit,custom_tool",
+    "hand-maintained tools are preserved")
+
+IntegrationMenuPin.refresh(false)
+assertEqual(table.concat(filemanager_order["KOMenu:menu_buttons"], ","),
+    "filemanager_settings,bookshelf_tab,setting,tools,search,plus_menu,main",
+    "disabling the icon preserves later plugin tabs")
+assertEqual(table.concat(filemanager_order.tools, ","),
+    "read_timer,later_plugin_tool,calibre,bookorbit,exporter",
+    "disabling the icon preserves later plugin tools")
+
+fs:cleanup()
+
 print("bookorbit_menu_pin_test.lua: ok")