Fix a few things after #7166 (#7212)

* Switching between HTML/text dicts (Fix #7209)
* Updating the scrollbar and scroll state properly when switching dicts
* Highlights in SortWidget
* Highlights in the Dictionary Download page
* Minor simplification of the tail end of the update process in ImageViewer

frontend/ui/widget/dictquicklookup.lua

@@ -234,7 +234,7 @@ function DictQuickLookup:init()
     -- below the title:  lookup word and definition
     local content_padding_h = Size.padding.large
     local content_padding_v = Size.padding.large -- added via VerticalSpan
-    local content_width = inner_width - 2*content_padding_h
+    self.content_width = inner_width - 2*content_padding_h
 
     -- Spans between components
     local top_to_word_span = VerticalSpan:new{ width = content_padding_v }
@@ -305,13 +305,13 @@ function DictQuickLookup:init()
         text = self.displayword,
         face = Font:getFace(word_font_face, word_font_size),
         bold = true,
-        max_width = content_width - math.max(lookup_edit_button_w, lookup_word_nb_w),
+        max_width = self.content_width - math.max(lookup_edit_button_w, lookup_word_nb_w),
         padding = 0, -- to be aligned with lookup_word_nb
     }
     -- Group these 3 widgets
     local lookup_word = OverlapGroup:new{
         dimen = {
-            w = content_width,
+            w = self.content_width,
             h = lookup_height,
         },
         self.lookup_word_text,
@@ -570,7 +570,7 @@ function DictQuickLookup:init()
         local test_widget = ScrollTextWidget:new{
             text = "z",
             face = self.content_face,
-            width = content_width,
+            width = self.content_width,
             height = self.definition_height,
         }
         self.definition_line_height = test_widget:getLineHeight()
@@ -623,33 +623,8 @@ function DictQuickLookup:init()
         end
     end
 
-    if self.is_html then
+    -- Instantiate self.text_widget
-        self.text_widget = ScrollHtmlWidget:new{
+    self:_instantiateScrollWidget()
-            html_body = self.definition,
-            css = self:getHtmlDictionaryCss(),
-            default_font_size = Screen:scaleBySize(self.dict_font_size),
-            width = content_width,
-            height = self.definition_height,
-            dialog = self,
-            html_link_tapped_callback = function(link)
-                self.html_dictionary_link_tapped_callback(self.dictionary, link)
-            end,
-         }
-    else
-        self.text_widget = ScrollTextWidget:new{
-            text = self.definition,
-            face = self.content_face,
-            width = content_width,
-            height = self.definition_height,
-            dialog = self,
-            justified = G_reader_settings:nilOrTrue("dict_justify"), -- allow for disabling justification
-            lang = self.lang and self.lang:lower(), -- only available on wikipedia results
-            para_direction_rtl = self.rtl_lang,     -- only available on wikipedia results
-            auto_para_direction = not self.is_wiki, -- only for dict results (we don't know their lang)
-            image_alt_face = self.image_alt_face,
-            images = self.images,
-        }
-    end
 
     -- word definition
     self.definition_widget = FrameContainer:new{
@@ -768,6 +743,39 @@ function DictQuickLookup:getHtmlDictionaryCss()
     return css
 end
 
+-- Used in init & update to instantiate the Scroll*Widget that self.text_widget points to
+function DictQuickLookup:_instantiateScrollWidget()
+    if self.is_html then
+        self.shw_widget = ScrollHtmlWidget:new{
+            html_body = self.definition,
+            css = self:getHtmlDictionaryCss(),
+            default_font_size = Screen:scaleBySize(self.dict_font_size),
+            width = self.content_width,
+            height = self.definition_height,
+            dialog = self,
+            html_link_tapped_callback = function(link)
+                self.html_dictionary_link_tapped_callback(self.dictionary, link)
+            end,
+        }
+        self.text_widget = self.shw_widget
+    else
+        self.stw_widget = ScrollTextWidget:new{
+            text = self.definition,
+            face = self.content_face,
+            width = self.content_width,
+            height = self.definition_height,
+            dialog = self,
+            justified = G_reader_settings:nilOrTrue("dict_justify"), -- allow for disabling justification
+            lang = self.lang and self.lang:lower(), -- only available on wikipedia results
+            para_direction_rtl = self.rtl_lang,     -- only available on wikipedia results
+            auto_para_direction = not self.is_wiki, -- only for dict results (we don't know their lang)
+            image_alt_face = self.image_alt_face,
+            images = self.images,
+        }
+        self.text_widget = self.stw_widget
+    end
+end
+
 function DictQuickLookup:update()
     -- self[1] is a WidgetContainer, its free method will call free on each of its child widget with a free method.
     -- Here, that's the definitions' TextBoxWidget & HtmlBoxWidget,
@@ -794,17 +802,35 @@ function DictQuickLookup:update()
     end
 
     -- Update main text widgets
-    if self.is_html then
+    if self.is_html and self.shw_widget then
+        -- Re-use our ScrollHtmlWidget (self.shw_widget)
+        -- NOTE: The recursive free via our WidgetContainer (self[1]) above already released the previous MµPDF document instance ;)
         self.text_widget.htmlbox_widget:setContent(self.definition, self:getHtmlDictionaryCss(), Screen:scaleBySize(self.dict_font_size))
-    else
+        -- Scroll back to top
+        self.text_widget:resetScroll()
+    elseif not self.is_html and self.stw_widget then
+        -- Re-use our ScrollTextWidget (self.stw_widget)
         self.text_widget.text_widget.text = self.definition
         -- NOTE: The recursive free via our WidgetContainer (self[1]) above already free'd us ;)
         self.text_widget.text_widget:init()
+        -- Scroll back to top
+        self.text_widget:resetScroll()
+    else
+        -- We jumped from HTML to Text (or vice-versa), we need a new widget instance
+        self:_instantiateScrollWidget()
+        -- Update *all* the references to self.text_widget
+        self.definition_widget[1] = self.text_widget
+        -- Destroy the previous "opposite type" widget
+        if self.is_html then
+            self.stw_widget = nil
+        else
+            self.shw_widget = nil
+        end
     end
 
     -- Reset alpha to avoid stacking transparency on top of the previous content.
     -- NOTE: This doesn't take care of the Scroll*Widget, which will preserve alpha on scroll,
-    --       leading to increasingly opaque and muddy text as half-tarnsparent stuff gets stacked on top of each other...
+    --       leading to increasingly opaque and muddy text as half-transparent stuff gets stacked on top of each other...
     self.movable.alpha = nil
 
     UIManager:setDirty(self, function()

frontend/ui/widget/imageviewer.lua

@@ -347,48 +347,8 @@ function ImageViewer:init()
         self.img_container_h = self.img_container_h - self.progress_container:getSize().h
     end
 
-    -- If no buttons and no title are shown, use the full screen
+    -- Instantiate self._image_wg & self.image_container
-    local max_image_h = self.img_container_h
+    self:_new_image_wg()
-    local max_image_w = self.width
-    -- Otherwise, add paddings around image
-    if self.buttons_visible or self.with_title_bar then
-        max_image_h = self.img_container_h - self.image_padding*2
-        max_image_w = self.width - self.image_padding*2
-    end
-
-    local rotation_angle = 0
-    if self.rotated then
-        -- in portrait mode, rotate according to this global setting so we are
-        -- like in landscape mode
-        local rotate_clockwise = DLANDSCAPE_CLOCKWISE_ROTATION
-        if Screen:getWidth() > Screen:getHeight() then
-            -- in landscape mode, counter-rotate landscape rotation so we are
-            -- back like in portrait mode
-            rotate_clockwise = not rotate_clockwise
-        end
-        rotation_angle = rotate_clockwise and 90 or 270
-    end
-
-    self._image_wg = ImageWidget:new{
-        file = self.file,
-        image = self.image,
-        image_disposable = false, -- we may re-use self.image
-        alpha = true, -- we might be showing images with an alpha channel (e.g., from Wikipedia)
-        width = max_image_w,
-        height = max_image_h,
-        rotation_angle = rotation_angle,
-        scale_factor = self.scale_factor,
-        center_x_ratio = self._center_x_ratio,
-        center_y_ratio = self._center_y_ratio,
-    }
-
-    self.image_container = CenterContainer:new{
-        dimen = Geom:new{
-            w = self.width,
-            h = self.img_container_h,
-        },
-        self._image_wg,
-    }
 
     local frame_elements = VerticalGroup:new{ align = "left" }
     if self.with_title_bar then
@@ -440,6 +400,53 @@ function ImageViewer:_clean_image_wg()
     end
 end
 
+-- Used in init & update to instantiate a new ImageWidget & its container
+function ImageViewer:_new_image_wg()
+    -- If no buttons and no title are shown, use the full screen
+    local max_image_h = self.img_container_h
+    local max_image_w = self.width
+    -- Otherwise, add paddings around image
+    if self.buttons_visible or self.with_title_bar then
+        max_image_h = self.img_container_h - self.image_padding*2
+        max_image_w = self.width - self.image_padding*2
+    end
+
+    local rotation_angle = 0
+    if self.rotated then
+        -- in portrait mode, rotate according to this global setting so we are
+        -- like in landscape mode
+        -- NOTE: This is the sole user of this legacy global left!
+        local rotate_clockwise = DLANDSCAPE_CLOCKWISE_ROTATION
+        if Screen:getWidth() > Screen:getHeight() then
+            -- in landscape mode, counter-rotate landscape rotation so we are
+            -- back like in portrait mode
+            rotate_clockwise = not rotate_clockwise
+        end
+        rotation_angle = rotate_clockwise and 90 or 270
+    end
+
+    self._image_wg = ImageWidget:new{
+        file = self.file,
+        image = self.image,
+        image_disposable = false, -- we may re-use self.image
+        alpha = true, -- we might be showing images with an alpha channel (e.g., from Wikipedia)
+        width = max_image_w,
+        height = max_image_h,
+        rotation_angle = rotation_angle,
+        scale_factor = self.scale_factor,
+        center_x_ratio = self._center_x_ratio,
+        center_y_ratio = self._center_y_ratio,
+    }
+
+    self.image_container = CenterContainer:new{
+        dimen = Geom:new{
+            w = self.width,
+            h = self.img_container_h,
+        },
+        self._image_wg,
+    }
+end
+
 function ImageViewer:update()
     -- Free our ImageWidget, which is the only thing we'll replace (e.g., leave the TextBoxWidgets alone).
     self:_clean_image_wg()
@@ -503,48 +510,7 @@ function ImageViewer:update()
     end
 
     -- Update the image widget itself
-    -- If no buttons and no title are shown, use the full screen
+    self:_new_image_wg()
-    local max_image_h = self.img_container_h
-    local max_image_w = self.width
-    -- Otherwise, add paddings around image
-    if self.buttons_visible or self.with_title_bar then
-        max_image_h = self.img_container_h - self.image_padding*2
-        max_image_w = self.width - self.image_padding*2
-    end
-
-    local rotation_angle = 0
-    if self.rotated then
-        -- in portrait mode, rotate according to this global setting so we are
-        -- like in landscape mode
-        local rotate_clockwise = DLANDSCAPE_CLOCKWISE_ROTATION
-        if Screen:getWidth() > Screen:getHeight() then
-            -- in landscape mode, counter-rotate landscape rotation so we are
-            -- back like in portrait mode
-            rotate_clockwise = not rotate_clockwise
-        end
-        rotation_angle = rotate_clockwise and 90 or 270
-    end
-
-    self._image_wg = ImageWidget:new{
-        file = self.file,
-        image = self.image,
-        image_disposable = false, -- we may re-use self.image
-        alpha = true, -- we might be showing images with an alpha channel (e.g., from Wikipedia)
-        width = max_image_w,
-        height = max_image_h,
-        rotation_angle = rotation_angle,
-        scale_factor = self.scale_factor,
-        center_x_ratio = self._center_x_ratio,
-        center_y_ratio = self._center_y_ratio,
-    }
-
-    self.image_container = CenterContainer:new{
-        dimen = Geom:new{
-            w = self.width,
-            h = self.img_container_h,
-        },
-        self._image_wg,
-    }
 
     -- Update the final layout
     local frame_elements = VerticalGroup:new{ align = "left" }
@@ -560,22 +526,8 @@ function ImageViewer:update()
         table.insert(frame_elements, self.button_container)
     end
 
-    self.main_frame = FrameContainer:new{
+    self.main_frame.radius = not self.fullscreen and 8 or nil
-        radius = not self.fullscreen and 8 or nil,
+    self.main_frame[1] = frame_elements
-        padding = 0,
-        margin = 0,
-        background = Blitbuffer.COLOR_WHITE,
-        frame_elements,
-    }
-    self[1] = WidgetContainer:new{
-        align = self.align,
-        dimen = self.region,
-        FrameContainer:new{
-            bordersize = 0,
-            padding = Size.padding.default,
-            self.main_frame,
-        }
-    }
 
     self.dithered = true
     UIManager:setDirty(self, function()

frontend/ui/widget/keyvaluepage.lua

@@ -296,12 +296,22 @@ function KeyValueItem:onTap()
             -- Has to be scheduled *after* the dict delays for the lookup history pages...
             UIManager:scheduleIn(0.75, function()
                 self[1].invert = false
-                -- Skip the repaint if we've ended up below something, which is likely.
+                -- If we've ended up below something, things get trickier.
-                if UIManager:getTopWidget() ~= self.show_parent then
+                local top_widget = UIManager:getTopWidget()
+                if top_widget ~= self.show_parent then
                     -- It's generally tricky to get accurate dimensions out of whatever was painted above us,
                     -- so cheat by comparing against the previous refresh region...
                     if self[1].dimen:intersectWith(UIManager:getPreviousRefreshRegion()) then
+                        -- If that something is a modal (e.g., dictionary D/L), repaint the whole stack
+                        if top_widget.modal then
+                            UIManager:setDirty(self.show_parent, function()
+                                return "ui", self[1].dimen
+                            end)
                             return true
+                        else
+                            -- Otherwise, skip the repaint
+                            return true
+                        end
                     end
                 end
                 UIManager:widgetInvert(self[1], self[1].dimen.x, self[1].dimen.y)

frontend/ui/widget/scrollhtmlwidget.lua

@@ -50,7 +50,7 @@ function ScrollHtmlWidget:init()
         end
     }
 
-    self.v_scroll_bar:set((self.htmlbox_widget.page_number-1) / self.htmlbox_widget.page_count, self.htmlbox_widget.page_number / self.htmlbox_widget.page_count)
+    self:_updateScrollBar()
 
     local horizontal_group = HorizontalGroup:new{}
     table.insert(horizontal_group, self.htmlbox_widget)
@@ -85,10 +85,25 @@ function ScrollHtmlWidget:init()
     end
 end
 
+-- Not to be confused with ScrollTextWidget's updateScrollBar, which has user-visible effects.
+-- This simply updates the scroll bar's internal state according to the current page & page count.
+function ScrollHtmlWidget:_updateScrollBar()
+    self.v_scroll_bar:set((self.htmlbox_widget.page_number-1) / self.htmlbox_widget.page_count, self.htmlbox_widget.page_number / self.htmlbox_widget.page_count)
+end
+
 function ScrollHtmlWidget:getSinglePageHeight()
     return self.htmlbox_widget:getSinglePageHeight()
 end
 
+-- Reset the scrolling *state* to the top of the document, but don't actually re-render/refresh anything.
+-- (Useful when replacing a Scroll*Widget during an update call, c.f., DictQuickLookup).
+function ScrollHtmlWidget:resetScroll()
+    self.htmlbox_widget.page_number = 1
+    self:_updateScrollBar()
+
+    self.v_scroll_bar.enable = self.htmlbox_widget.page_count > 1
+end
+
 function ScrollHtmlWidget:scrollToRatio(ratio)
     ratio = math.max(0, math.min(1, ratio)) -- ensure ratio is between 0 and 1 (100%)
     local page_num = 1 + math.floor((self.htmlbox_widget.page_count) * ratio)
@@ -99,9 +114,11 @@ function ScrollHtmlWidget:scrollToRatio(ratio)
         return
     end
     self.htmlbox_widget.page_number = page_num
-    self.v_scroll_bar:set((page_num-1) / self.htmlbox_widget.page_count, page_num / self.htmlbox_widget.page_count)
+    self:_updateScrollBar()
+
     self.htmlbox_widget:freeBb()
     self.htmlbox_widget:_render()
+
     UIManager:setDirty(self.dialog, function()
         return "partial", self.dimen
     end)
@@ -125,8 +142,7 @@ function ScrollHtmlWidget:scrollText(direction)
 
         self.htmlbox_widget.page_number = self.htmlbox_widget.page_number - 1
     end
-
+    self:_updateScrollBar()
-    self.v_scroll_bar:set((self.htmlbox_widget.page_number-1) / self.htmlbox_widget.page_count, self.htmlbox_widget.page_number / self.htmlbox_widget.page_count)
 
     self.htmlbox_widget:freeBb()
     self.htmlbox_widget:_render()

frontend/ui/widget/scrolltextwidget.lua

@@ -159,6 +159,17 @@ function ScrollTextWidget:updateScrollBar(is_partial)
     end
 end
 
+-- Reset the scrolling *state* to the top of the document, but don't actually re-render/refresh anything.
+-- (Useful when replacing a Scroll*Widget during an update call, c.f., DictQuickLookup).
+function ScrollTextWidget:resetScroll()
+    local low, high = self.text_widget:getVisibleHeightRatios()
+    self.v_scroll_bar:set(low, high)
+
+    local visible_line_count = self.text_widget:getVisLineCount()
+    local total_line_count = self.text_widget:getAllLineCount()
+    self.v_scroll_bar.enable = visible_line_count < total_line_count
+end
+
 function ScrollTextWidget:moveCursorToCharPos(charpos)
     self.text_widget:moveCursorToCharPos(charpos)
     self:updateScrollBar()

frontend/ui/widget/sortwidget.lua

@@ -228,6 +228,7 @@ function SortWidget:init()
         bordersize = 0,
         padding = 0,
         radius = 0,
+        show_parent = self,
     }
     self.footer_right = Button:new{
         text = footer_right_text,
@@ -237,6 +238,7 @@ function SortWidget:init()
         bordersize = 0,
         padding = 0,
         radius = 0,
+        show_parent = self,
     }
     self.footer_first_up = Button:new{
         text = footer_first_up_text,
@@ -252,6 +254,7 @@ function SortWidget:init()
         bordersize = 0,
         padding = 0,
         radius = 0,
+        show_parent = self,
     }
     self.footer_last_down = Button:new{
         text = footer_last_down_text,
@@ -267,6 +270,7 @@ function SortWidget:init()
         bordersize = 0,
         padding = 0,
         radius = 0,
+        show_parent = self,
     }
     self.footer_cancel = Button:new{
         text = "✘",
@@ -276,6 +280,7 @@ function SortWidget:init()
         text_font_size = 28,
         padding = 0,
         radius = 0,
+        show_parent = self,
     }
 
     self.footer_ok = Button:new{
@@ -286,6 +291,7 @@ function SortWidget:init()
         padding = 0,
         radius = 0,
         text_font_size = 28,
+        show_parent = self,
     }
 
     self.footer_page = Button:new{
@@ -308,6 +314,7 @@ function SortWidget:init()
         text_font_face = "pgfont",
         text_font_bold = false,
         width = self.width_widget * 22 / 100,
+        show_parent = self,
     }
     local button_vertical_line = LineWidget:new{
         dimen = Geom:new{ w = Size.line.thick, h = math.floor(self.item_height * 1.25) },