UIManager: Fix handling of toast widgets in sendEvent (#9617)

The ultimate goal is for toast widgets (i.e., Notification when flagged as such) to:
  * Not stop event propagation
  * Close themselves when the event was emitted by user input.

Instead of doing event filtering in UIManager, we simply overload the onGesture & onKey* handlers in Notification to do just that, and just make sure UIManager will *send* those events to toasts, but without affecting the usual semantics of top widget selection and event propagation (which is as simple as just calling handleEvent on them unchecked ;p).

Thanks to @poire-z for the brainstorming in https://github.com/koreader/koreader/issues/9594 ;).

This also happens to fix a bug in which we might have looped on the top widget twice, because of an array vs. hash mishap ;).

frontend/ui/uimanager.lua

@@ -775,17 +775,29 @@ function UIManager:sendEvent(event)
         return
     end
 
-    -- The top widget gets to be the first to get the event
-    local top_widget = self._window_stack[#self._window_stack].widget
-
-    -- A toast widget gets closed by any event, and lets the event be handled by a lower widget.
-    -- (Notification is our only widget flagged as such).
-    while top_widget.toast do -- close them all
-        self:close(top_widget)
-        if not self._window_stack[1] then
-            return
+    local top_widget
+    local checked_widgets = {}
+    -- Toast widgets, which, by contract, must be at the top of the window stack, never stop event propagation.
+    for i = #self._window_stack, 1, -1 do
+        local widget = self._window_stack[i].widget
+        -- Whether it's a toast or not, we'll call handleEvent now,
+        -- so we'll want to skip it during the table walk later.
+        checked_widgets[widget] = true
+        if widget.toast then
+            -- We never stop event propagation on toasts, but we still want to send the event to them.
+            -- (In particular, because we want them to close on user input).
+            widget:handleEvent(event)
+        else
+            -- The first widget to consume events as designed is the topmost non-toast one
+            top_widget = widget
+            break
         end
-        top_widget = self._window_stack[#self._window_stack].widget
+    end
+
+    -- Extremely unlikely, but we can't exclude the possibility of *everything* being a toast ;).
+    -- In which case, the event has nowhere else to go, so, we're done.
+    if not top_widget then
+        return
     end
 
     if top_widget:handleEvent(event) then
@@ -793,7 +805,9 @@ function UIManager:sendEvent(event)
     end
     if top_widget.active_widgets then
         for _, active_widget in ipairs(top_widget.active_widgets) do
-            if active_widget:handleEvent(event) then return end
+            if active_widget:handleEvent(event) then
+                return
+            end
         end
     end
 
@@ -804,7 +818,6 @@ function UIManager:sendEvent(event)
     --       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}
     local i = #self._window_stack
     while i > 0 do
         local widget = self._window_stack[i].widget
@@ -826,6 +839,8 @@ function UIManager:sendEvent(event)
                     return
                 end
             end
+            -- As mentioned above, event handlers might have shown/closed widgets,
+            -- so all bets are off on our old window tally being accurate, so let's take it from the top again ;).
             i = #self._window_stack
         else
             i = i - 1

frontend/ui/widget/notification.lua

@@ -15,9 +15,10 @@ local Size = require("ui/size")
 local TextWidget = require("ui/widget/textwidget")
 local UIManager = require("ui/uimanager")
 local VerticalGroup = require("ui/widget/verticalgroup")
-local Input = Device.input
 local time = require("ui/time")
+local _ = require("gettext")
 local Screen = Device.screen
+local Input = Device.input
 
 local band = bit.band
 
@@ -41,13 +42,14 @@ local SOURCE_ALL = SOURCE_BOTTOM_MENU + SOURCE_DISPATCHER + SOURCE_OTHER
 
 local Notification = InputContainer:extend{
     face = Font:getFace("x_smallinfofont"),
-    text = "Null Message",
+    text = _("N/A"),
     margin = Size.margin.default,
     padding = Size.padding.default,
     timeout = 2, -- default to 2 seconds
     toast = true, -- closed on any event, and let the event propagate to next top widget
 
-    _nums_shown = {}, -- actual static class member, array of stacked notifications
+    _shown_list = {}, -- actual static class member, array of stacked notifications (value is show (well, init) time or false).
+    _shown_idx = nil, -- index of this instance in the class's _shown_list array (assumes each Notification object is only shown (well, init) once).
 
     SOURCE_BOTTOM_MENU_ICON = SOURCE_BOTTOM_MENU_ICON,
     SOURCE_BOTTOM_MENU_TOGGLE = SOURCE_BOTTOM_MENU_TOGGLE,
@@ -110,8 +112,8 @@ function Notification:init()
     local notif_height = self.frame:getSize().h
 
     self:_cleanShownStack()
-    table.insert(Notification._nums_shown, UIManager:getTime())
-    self.num = #Notification._nums_shown
+    table.insert(Notification._shown_list, UIManager:getTime())
+    self._shown_idx = #Notification._shown_list
 
     self[1] = VerticalGroup:new{
         align = "center",
@@ -156,30 +158,31 @@ function Notification:notify(arg, refresh_after)
     return false
 end
 
-function Notification:_cleanShownStack(num)
+function Notification:_cleanShownStack()
     -- Clean stack of shown notifications
-    if num then
+    if self._shown_idx then
+        -- If this field exists, this is the first time this instance was closed since its init.
         -- This notification is no longer displayed
-        Notification._nums_shown[num] = false
+        Notification._shown_list[self._shown_idx] = false
     end
-    -- We remove from the stack tail all slots no longer displayed.
+    -- We remove from the stack's tail all slots no longer displayed.
     -- Even if slots at top are available, we'll keep adding new
     -- notifications only at the tail/bottom (easier for the eyes
     -- to follow what is happening).
     -- As a sanity check, we also forget those shown for
     -- more than 30s in case no close event was received.
     local expire_time = UIManager:getTime() - time.s(30)
-    for i=#Notification._nums_shown, 1, -1 do
-        if Notification._nums_shown[i] and Notification._nums_shown[i] > expire_time then
+    for i = #Notification._shown_list, 1, -1 do
+        if Notification._shown_list[i] and Notification._shown_list[i] > expire_time then
             break -- still shown (or not yet expired)
         end
-        table.remove(Notification._nums_shown, i)
+        table.remove(Notification._shown_list, i)
     end
 end
 
 function Notification:onCloseWidget()
-    self:_cleanShownStack(self.num)
-    self.num = nil -- avoid mess in case onCloseWidget is called multiple times
+    self:_cleanShownStack()
+    self._shown_idx = nil -- Don't do something stupid if this same instance gets closed multiple times
     UIManager:setDirty(nil, function()
         return "ui", self.frame.dimen
     end)
@@ -208,4 +211,27 @@ function Notification:onTapClose()
     return true
 end
 
+-- Toasts should go bye-bye on user input, without stopping the event's propagation.
+function Notification:onKeyPress(key)
+    if self.toast then
+        UIManager:close(self)
+        return false
+    end
+    return InputContainer.onKeyPress(self, key)
+end
+function Notification:onKeyRepeat(key)
+    if self.toast then
+        UIManager:close(self)
+        return false
+    end
+    return InputContainer.onKeyRepeat(self, key)
+end
+function Notification:onGesture(ev)
+    if self.toast then
+        UIManager:close(self)
+        return false
+    end
+    return InputContainer.onGesture(self, ev)
+end
+
 return Notification