From 3d191490d00ccf9725e30ddaf2d95eecf23ecb15 Mon Sep 17 00:00:00 2001
From: poire-z <poire-z@users.noreply.github.com>
Date: Sat, 30 Nov 2019 17:38:28 +0100
Subject: [PATCH] Touch zones: fix loss of overrides when re-registering a zone
 (#5658)

Happens with "Inverse reading order", which re-registers
the Reader tap forward/backward zones and would prevent
tap menu and bookmark from working.
Before, when removing them, we would lose all the override=
set on them by other touch zones (tap menu, bookmarks...),
and so they were no more ensured after re-adding the zone.
So, make sure we don't lose that info.
---
 .../apps/reader/modules/readergesture.lua     |  5 ---
 frontend/depgraph.lua                         | 13 ++++++-
 spec/unit/depgraph_spec.lua                   | 39 +++++++++++++++++++
 3 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/frontend/apps/reader/modules/readergesture.lua b/frontend/apps/reader/modules/readergesture.lua
index 9aa0fd75f..8fffabe7f 100644
--- a/frontend/apps/reader/modules/readergesture.lua
+++ b/frontend/apps/reader/modules/readergesture.lua
@@ -1577,11 +1577,6 @@ function ReaderGesture:onToggleReadingOrder()
     local document_module = self.ui.document.info.has_pages and self.ui.paging or self.ui.rolling
     document_module.inverse_reading_order = not document_module.inverse_reading_order
     document_module:setupTouchZones()
-    -- Needed to reset the touch zone overrides
-    local gesture_manager = G_reader_settings:readSetting(self.ges_mode)
-    for gesture, action in pairs(gesture_manager) do
-        self:setupGesture(gesture, action)
-    end
     UIManager:show(Notification:new{
         text = document_module.inverse_reading_order and _("RTL page turning.") or _("LTR page turning."),
         timeout = 2.5,
diff --git a/frontend/depgraph.lua b/frontend/depgraph.lua
index d15d69da3..f7a061f76 100644
--- a/frontend/depgraph.lua
+++ b/frontend/depgraph.lua
@@ -40,7 +40,18 @@ function DepGraph:addNode(node_key, deps)
 end
 
 function DepGraph:removeNode(node_key)
-    self.nodes[node_key] = nil
+    -- We should not remove it from self.nodes if it has
+    -- a .deps array (it is the other nodes, that had this
+    -- one in their override=, that have added themselves in
+    -- this node's .deps). We don't want to lose these
+    -- dependencies if we later re-addNode this node.
+    local node = self.nodes[node_key]
+    if node then
+        if not node.deps or #node.deps == 0 then
+            self.nodes[node_key] = nil
+        end
+    end
+    -- But we should remove it from the .deps of other nodes.
     for curr_node_key, curr_node in pairs(self.nodes) do
         if curr_node.deps then
             local remove_idx
diff --git a/spec/unit/depgraph_spec.lua b/spec/unit/depgraph_spec.lua
index 66241c592..9c9a1ac9c 100644
--- a/spec/unit/depgraph_spec.lua
+++ b/spec/unit/depgraph_spec.lua
@@ -98,4 +98,43 @@ describe("DepGraph module", function()
             'readerhighlight_hold_release',
         }, dg:serialize())
     end)
+
+    it("should serialize complex graph and keep dependencies after removing and re-adding", function()
+        local dg = DepGraph:new{}
+        dg:addNode("tap_backward")
+        dg:addNode("tap_forward")
+        -- The next 3 steps are what is done when registering:
+        --    { id = "readermenu_tap", overrides = { "tap_forward", "tap_backward" } }
+        dg:addNode("readermenu_tap")
+        dg:addNodeDep("tap_backward", "readermenu_tap")
+        dg:addNodeDep("tap_forward", "readermenu_tap")
+        -- print(require("dump")(dg))
+        --     ["nodes"] = {
+        --         ["readermenu_tap"] = {},
+        --         ["tap_backward"] = {
+        --             ["deps"] = {
+        --                 [1] = "readermenu_tap"
+        --             }
+        --         },
+        --         ["tap_forward"] = {
+        --             ["deps"] = {
+        --                 [1] = "readermenu_tap"
+        --             }
+        --         }
+        --     }
+        dg:removeNode("tap_forward")
+        dg:removeNode("tap_backward")
+        dg:addNode("tap_forward")
+        dg:addNode("tap_backward")
+        -- print(require("dump")(dg))
+        assert.are.same({
+            "readermenu_tap",
+            "tap_backward",
+            "tap_forward",
+        }, dg:serialize())
+        assert.is_true(type(dg.nodes["tap_forward"].deps) == "table")
+        assert.is_true(#dg.nodes["tap_forward"].deps > 0)
+        assert.is_true(type(dg.nodes["tap_backward"].deps) == "table")
+        assert.is_true(#dg.nodes["tap_backward"].deps > 0)
+    end)
 end)