From 8d3aacbac3782eced71598fc1c18e5938f54e551 Mon Sep 17 00:00:00 2001
From: NiLuJe <ninuje@gmail.com>
Date: Mon, 17 May 2021 23:40:24 +0200
Subject: [PATCH] ReaderFooter: Fix the madness related to its hold footer

Namely, instead of making it lower priority than readerhighlight and
hoping for the best, make it higher priority, as it should (ReaderFooter
itself is *above* Reader, after all), with a sane set of fallthroughs:

* No footer
* No hold-to-skim
* Held outside the footer (but inside the footer tap zone)

This made the crap workaround from #7466 unnecessary, and actually fixes
the behavior in PDFs (because readerhighlight will match the *physical*
page, which may include off-screen content) and ePubs w/r eclaim bar
height (in which case, there's actual content behind the footer that
readerhighlight could have matched on).

Fix #7697
---
 frontend/apps/reader/modules/readerfooter.lua | 34 +++++++++++--------
 .../apps/reader/modules/readerhighlight.lua   |  3 --
 2 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/frontend/apps/reader/modules/readerfooter.lua b/frontend/apps/reader/modules/readerfooter.lua
index 1661bcca0..ad204ebac 100644
--- a/frontend/apps/reader/modules/readerfooter.lua
+++ b/frontend/apps/reader/modules/readerfooter.lua
@@ -718,10 +718,11 @@ function ReaderFooter:setupTouchZones()
             id = "readerfooter_hold",
             ges = "hold",
             screen_zone = footer_screen_zone,
-            handler = function() return self:onHoldFooter() end,
-            -- (Low priority: word lookup and text selection
-            -- have priority - SkimTo widget can be more easily
-            -- brought up via some other gestures)
+            handler = function(ges) return self:onHoldFooter(ges) end,
+            overrides = {
+                "readerhighlight_hold",
+            },
+            -- (High priority: it's a fallthrough if we held outside the footer)
         },
     })
 end
@@ -2163,17 +2164,22 @@ function ReaderFooter:onTapFooter(ges)
     return true
 end
 
-function ReaderFooter:onHoldFooter()
-    -- We're lower priority than readerhighlight_hold, which means it's already been tripped by the time we got here.
-    -- If it actually had something to do, ReaderHighlight would have stopped propagating the event,
-    -- so we can safely clear ReaderHighlight state here, otherwise it'll get confused on the next tap...
-    -- (c.f., #7464)
-    self.ui.highlight:clear()
-    if self.mode == self.mode_list.off then return end
-    if self.settings.skim_widget_on_hold then
-        self.ui:handleEvent(Event:new("ShowSkimtoDialog"))
-        return true
+function ReaderFooter:onHoldFooter(ges)
+    -- We're higher priority than readerhighlight_hold, so, make sure we fall through properly...
+    if not self.settings.skim_widget_on_hold then
+        return
     end
+    if not self.view.footer_visible then
+        return
+    end
+    if not self.footer_content.dimen or not self.footer_content.dimen:contains(ges.pos) then
+        -- We held outside the footer: meep!
+        return
+    end
+
+    -- We're good, make sure we stop the event from going to readerhighlight_hold
+    self.ui:handleEvent(Event:new("ShowSkimtoDialog"))
+    return true
 end
 
 function ReaderFooter:refreshFooter(refresh, signal)
diff --git a/frontend/apps/reader/modules/readerhighlight.lua b/frontend/apps/reader/modules/readerhighlight.lua
index e80ede0e8..9bebadd82 100644
--- a/frontend/apps/reader/modules/readerhighlight.lua
+++ b/frontend/apps/reader/modules/readerhighlight.lua
@@ -215,9 +215,6 @@ function ReaderHighlight:setupTouchZones()
             screen_zone = {
                 ratio_x = 0, ratio_y = 0, ratio_w = 1, ratio_h = 1,
             },
-            overrides = {
-                "readerfooter_hold",
-            },
             handler = function(ges) return self:onHold(nil, ges) end
         },
         {