Merge pull request #2160 from koreader/houqp-master

set scroll mode as default

base

@@ -1 +1 @@
-Subproject commit f52fbec914e54ac24be68161951788e9fb12253c
+Subproject commit a287e291724707eb515440436882450cc480c95e

defaults.lua

@@ -13,7 +13,7 @@ DHINTCOUNT = 1
 DFULL_SCREEN = 1
 
 -- scroll mode, 1 for true, 0 for false
-DSCROLL_MODE = 0
+DSCROLL_MODE = 1
 
 -- default gamma setting:
 DGLOBALGAMMA = 1.0

frontend/apps/filemanager/filemanagermenu.lua

@@ -40,7 +40,7 @@ function FileManagerMenu:init()
                     UIManager:show(ConfirmBox:new{
                         text = _("You have unsaved default settings. Save them now?"),
                         ok_callback = function()
-                            SetDefaults:SaveSettings()
+                            SetDefaults:saveSettings()
                         end,
                     })
                 else

frontend/apps/filemanager/filemanagersetdefaults.lua

@@ -64,9 +64,9 @@ function SetDefaults:init()
         dimen = Screen:getSize(),
     }
     -- FIXME:
-    -- in this use case (an input dialog is closed and the menu container is opened
-    -- immediately) we need to set the full screen dirty because otherwise only
-    -- the input dialog part of the screen is refreshed.
+    -- in this use case (an input dialog is closed and the menu container is
+    -- opened immediately) we need to set the full screen dirty because
+    -- otherwise only the input dialog part of the screen is refreshed.
     menu_container.onShow = function()
         UIManager:setDirty(nil, "partial")
     end
@@ -256,7 +256,7 @@ function SetDefaults:ConfirmSave()
     UIManager:show(ConfirmBox:new{
         text = _('Are you sure you want to save the settings to "defaults.persistent.lua"?'),
         ok_callback = function()
-            self:SaveSettings()
+            self:saveSettings()
         end,
     })
 end
@@ -275,7 +275,7 @@ function SetDefaults:build_setting(j)
     end
 end
 
-function SetDefaults:SaveSettings()
+function SetDefaults:saveSettings()
     self.results = {}
     local persisted_defaults = {}
     local file = io.open(persistent_filename, "r")

frontend/apps/reader/modules/readerpaging.lua

@@ -428,8 +428,7 @@ end
 function ReaderPaging:onRestoreBookLocation(saved_location)
     if self.view.page_scroll then
         self.view:restoreViewContext(saved_location)
-        self:_gotoPage(self.view.page_states[#self.view.page_states].page,
-                       "scrolling")
+        self:_gotoPage(self.view.page_states[1].page, "scrolling")
     else
         -- gotoPage will emit PageUpdate event, which will trigger recalculate
         -- in ReaderView and resets the view context. So we need to call

spec/unit/defaults_spec.lua

@@ -22,7 +22,7 @@ describe("defaults module", function()
         Defaults.changed[28] = true
         Defaults.changed[63] = true
         Defaults.changed[77] = true
-        Defaults:SaveSettings()
+        Defaults:saveSettings()
         assert.is_same(#Defaults.defaults_name, 77)
         assert.is_same(Defaults.defaults_name[28], 'DHINTCOUNT')
         assert.is_same(Defaults.defaults_name[77], 'SEARCH_TITLE')
@@ -63,7 +63,7 @@ DTAP_ZONE_MENU = {
             h = 20.25,
             w = 20.75
         }
-        Defaults:SaveSettings()
+        Defaults:saveSettings()
         fd = io.open(persistent_filename)
         assert.Equals(
 [[-- For configuration changes that persists between updates
@@ -109,7 +109,7 @@ DHINTCOUNT = 2
         Defaults:init()
         Defaults.changed[28] = true
         Defaults.defaults_value[28] = 1
-        Defaults:SaveSettings()
+        Defaults:saveSettings()
         fd = io.open(persistent_filename)
         assert.Equals(
 [[-- For configuration changes that persists between updates

spec/unit/readerhighlight_spec.lua

@@ -1,5 +1,5 @@
 describe("Readerhighlight module", function()
-    local DocumentRegistry, ReaderUI, UIManager, Screen, Geom, dbg
+    local DocumentRegistry, ReaderUI, UIManager, Screen, Geom, dbg, Event
     setup(function()
         require("commonrequire")
         DocumentRegistry = require("document/documentregistry")
@@ -8,6 +8,7 @@ describe("Readerhighlight module", function()
         Screen = require("device").screen
         Geom = require("ui/geometry")
         dbg = require("dbg")
+        Event = require("ui/event")
     end)
 
     local function highlight_single_word(readerui, pos0)
@@ -23,12 +24,24 @@ describe("Readerhighlight module", function()
     local function highlight_text(readerui, pos0, pos1)
         readerui.highlight:onHold(nil, { pos = pos0 })
         readerui.highlight:onHoldPan(nil, { pos = pos1 })
+        local next_slot
+        for i = #UIManager._window_stack, 0, -1 do
+            local top_window = UIManager._window_stack[i]
+            -- skip modal window
+            if not top_window or not top_window.widget.modal then
+                next_slot = i + 1
+                break
+            end
+        end
         readerui.highlight:onHoldRelease()
         assert.truthy(readerui.highlight.highlight_dialog)
+        assert.truthy(UIManager._window_stack[next_slot].widget
+                      == readerui.highlight.highlight_dialog)
         readerui.highlight:onHighlight()
         UIManager:scheduleIn(1, function()
             UIManager:close(readerui.highlight.highlight_dialog)
             UIManager:close(readerui)
+            UIManager:quit()
         end)
         UIManager:run()
     end
@@ -90,13 +103,14 @@ describe("Readerhighlight module", function()
         end)
     end)
 
-    describe("highlight for PDF documents", function()
+    describe("highlight for PDF documents in page mode", function()
         local readerui
         setup(function()
             local sample_pdf = "spec/front/unit/data/sample.pdf"
             readerui = ReaderUI:new{
                 document = DocumentRegistry:openDocument(sample_pdf),
             }
+            readerui:handleEvent(Event:new("SetScrollMode", false))
         end)
         describe("for scanned page with text layer", function()
             before_each(function()
@@ -170,4 +184,89 @@ describe("Readerhighlight module", function()
             end)
         end)
     end)
+
+    describe("highlight for PDF documents in scroll mode", function()
+        local readerui
+        setup(function()
+            local sample_pdf = "spec/front/unit/data/sample.pdf"
+            readerui = ReaderUI:new{
+                document = DocumentRegistry:openDocument(sample_pdf),
+            }
+            readerui:handleEvent(Event:new("SetScrollMode", true))
+        end)
+        describe("for scanned page with text layer", function()
+            before_each(function()
+                UIManager:quit()
+                UIManager:show(readerui)
+                readerui.paging:onGotoPage(10)
+                readerui.zooming:setZoomMode("contentwidth")
+            end)
+            after_each(function()
+                readerui.highlight:clear()
+            end)
+            it("should highlight single word", function()
+                highlight_single_word(readerui, Geom:new{ x = 260, y = 70 })
+                Screen:shot("screenshots/reader_highlight_single_word_pdf_scroll.png")
+            end)
+            it("should highlight text", function()
+                highlight_text(readerui, Geom:new{ x = 260, y = 70 }, Geom:new{ x = 260, y = 150 })
+                Screen:shot("screenshots/reader_highlight_text_pdf_scroll.png")
+            end)
+            it("should response on tap gesture", function()
+                tap_highlight_text(readerui,
+                                   Geom:new{ x = 260, y = 70 },
+                                   Geom:new{ x = 260, y = 150 },
+                                   Geom:new{ x = 280, y = 110 })
+                Screen:shot("screenshots/reader_tap_highlight_text_pdf_scroll.png")
+            end)
+        end)
+        describe("for scanned page without text layer", function()
+            before_each(function()
+                UIManager:quit()
+                UIManager:show(readerui)
+                readerui.paging:onGotoPage(28)
+                readerui.zooming:setZoomMode("contentwidth")
+            end)
+            after_each(function()
+                readerui.highlight:clear()
+            end)
+            it("should highlight single word", function()
+                highlight_single_word(readerui, Geom:new{ x = 260, y = 70 })
+                Screen:shot("screenshots/reader_highlight_single_word_pdf_scanned_scroll.png")
+            end)
+            it("should highlight text", function()
+                highlight_text(readerui, Geom:new{x = 192, y = 186}, Geom:new{x = 280, y = 186})
+                Screen:shot("screenshots/reader_highlight_text_pdf_scanned_scroll.png")
+            end)
+            it("should response on tap gesture", function()
+                tap_highlight_text(readerui, Geom:new{ x = 260, y = 70 }, Geom:new{ x = 260, y = 150 }, Geom:new{ x = 280, y = 110 })
+                Screen:shot("screenshots/reader_tap_highlight_text_pdf_scanned_scroll.png")
+            end)
+        end)
+        describe("for reflowed page", function()
+            before_each(function()
+                UIManager:quit()
+                readerui.document.configurable.text_wrap = 1
+                UIManager:show(readerui)
+                readerui.paging:onGotoPage(31)
+            end)
+            after_each(function()
+                readerui.highlight:clear()
+                readerui.document.configurable.text_wrap = 0
+            end)
+            it("should highlight single word", function()
+                highlight_single_word(readerui, Geom:new{ x = 260, y = 70 })
+                Screen:shot("screenshots/reader_highlight_single_word_pdf_reflowed_scroll.png")
+            end)
+            it("should highlight text", function()
+                highlight_text(readerui, Geom:new{ x = 260, y = 70 }, Geom:new{ x = 260, y = 150 })
+                Screen:shot("screenshots/reader_highlight_text_pdf_reflowed_scroll.png")
+            end)
+            it("should response on tap gesture", function()
+                tap_highlight_text(readerui, Geom:new{ x = 260, y = 70 }, Geom:new{ x = 260, y = 150 }, Geom:new{ x = 280, y = 110 })
+                Screen:shot("screenshots/reader_tap_highlight_text_pdf_reflowed_scroll.png")
+            end)
+        end)
+    end)
+
 end)

spec/unit/readerlink_spec.lua

@@ -1,9 +1,10 @@
 describe("ReaderLink module", function()
-    local DocumentRegistry, ReaderUI, UIManager, sample_epub, sample_pdf
+    local DocumentRegistry, ReaderUI, UIManager, sample_epub, sample_pdf, Event
 
     setup(function()
         require("commonrequire")
         DocumentRegistry = require("document/documentregistry")
+        Event = require("ui/event")
         ReaderUI = require("apps/reader/readerui")
         UIManager = require("ui/uimanager")
         sample_epub = "spec/front/unit/data/leaves.epub"
@@ -19,17 +20,30 @@ describe("ReaderLink module", function()
         assert.is.same(36, readerui.rolling.current_page)
     end)
 
-    it("should jump to links in pdf", function()
+    it("should jump to links in pdf page mode", function()
         UIManager:quit()
         local readerui = ReaderUI:new{
             document = DocumentRegistry:openDocument(sample_pdf),
         }
+        readerui:handleEvent(Event:new("SetScrollMode", false))
         readerui.paging:onGotoPage(1)
         readerui.link:onTap(nil, {pos = {x = 363, y = 585}})
         UIManager:run()
         assert.is.same(22, readerui.paging.current_page)
     end)
 
+    it("should jump to links in pdf scroll mode", function()
+        UIManager:quit()
+        local readerui = ReaderUI:new{
+            document = DocumentRegistry:openDocument(sample_pdf),
+        }
+        readerui:handleEvent(Event:new("SetScrollMode", true))
+        readerui.paging:onGotoPage(1)
+        readerui.link:onTap(nil, {pos = {x = 250, y = 534}})
+        UIManager:run()
+        assert.is.same(21, readerui.paging.current_page)
+    end)
+
     it("should be able to go back after link jump in epub", function()
         local readerui = ReaderUI:new{
             document = DocumentRegistry:openDocument(sample_epub),
@@ -41,11 +55,12 @@ describe("ReaderLink module", function()
         assert.is.same(4, readerui.rolling.current_page)
     end)
 
-    it("should be able to go back after link jump in pdf", function()
+    it("should be able to go back after link jump in pdf page mode", function()
         UIManager:quit()
         local readerui = ReaderUI:new{
             document = DocumentRegistry:openDocument(sample_pdf),
         }
+        readerui:handleEvent(Event:new("SetScrollMode", false))
         readerui.paging:onGotoPage(1)
         readerui.link:onTap(nil, {pos = {x = 363, y = 585}})
         UIManager:run()
@@ -54,6 +69,20 @@ describe("ReaderLink module", function()
         assert.is.same(1, readerui.paging.current_page)
     end)
 
+    it("should be able to go back after link jump in pdf scroll mode", function()
+        UIManager:quit()
+        local readerui = ReaderUI:new{
+            document = DocumentRegistry:openDocument(sample_pdf),
+        }
+        readerui:handleEvent(Event:new("SetScrollMode", true))
+        readerui.paging:onGotoPage(1)
+        readerui.link:onTap(nil, {pos = {x = 250, y = 534}})
+        UIManager:run()
+        assert.is.same(21, readerui.paging.current_page)
+        readerui.link:onGoBackLink()
+        assert.is.same(1, readerui.paging.current_page)
+    end)
+
     it("should be able to go back after link jump in pdf in scroll mode", function()
         UIManager:quit()
         local expected_page_states = {
@@ -106,7 +135,7 @@ describe("ReaderLink module", function()
         UIManager:run()
         assert.is.same(22, readerui.paging.current_page)
         readerui.link:onGoBackLink()
-        assert.is.same(4, readerui.paging.current_page)
+        assert.is.same(3, readerui.paging.current_page)
         assert.are.same(expected_page_states, readerui.view.page_states)
     end)
 end)

spec/unit/readerpaging_spec.lua

@@ -6,14 +6,18 @@ describe("Readerpaging module", function()
     setup(function() require("commonrequire") end)
 
     describe("Page mode", function()
+        local Event
+
         setup(function()
+            Event = require("ui/event")
             readerui = require("apps/reader/readerui"):new{
                 document = require("document/documentregistry"):openDocument(sample_pdf),
             }
             paging = readerui.paging
         end)
 
-        it("should emit EndOfBook event at the end", function()
+        it("should emit EndOfBook event at the end in page mode", function()
+            readerui:handleEvent(Event:new("SetScrollMode", false))
             readerui.zooming:setZoomMode("pageheight")
             paging:onGotoPage(readerui.document:getPageCount())
             local called = false
@@ -24,6 +28,19 @@ describe("Readerpaging module", function()
             assert.is.truthy(called)
             readerui.onEndOfBook = nil
         end)
+
+        it("should emit EndOfBook event at the end in scroll mode", function()
+            readerui:handleEvent(Event:new("SetScrollMode", true))
+            paging:onGotoPage(readerui.document:getPageCount())
+            readerui.zooming:setZoomMode("pageheight")
+            local called = false
+            readerui.onEndOfBook = function()
+                called = true
+            end
+            paging:onPagingRel(1)
+            assert.is.truthy(called)
+            readerui.onEndOfBook = nil
+        end)
     end)
 
     describe("Scroll mode", function()

spec/unit/readerview_spec.lua

@@ -1,5 +1,5 @@
 describe("Readerview module", function()
-    local DocumentRegistry, Blitbuffer, ReaderUI, UIManager
+    local DocumentRegistry, Blitbuffer, ReaderUI, UIManager, Event
 
     setup(function()
         require("commonrequire")
@@ -7,6 +7,7 @@ describe("Readerview module", function()
         Blitbuffer = require("ffi/blitbuffer")
         ReaderUI = require("apps/reader/readerui")
         UIManager = require("ui/uimanager")
+        Event = require("ui/event")
     end)
 
     it("should stop hinting on document close event", function()
@@ -44,11 +45,12 @@ describe("Readerview module", function()
         end
     end)
 
-    it("should return and restore view context", function()
+    it("should return and restore view context in page mode", function()
         local sample_pdf = "spec/front/unit/data/2col.pdf"
         local readerui = ReaderUI:new{
             document = DocumentRegistry:openDocument(sample_pdf),
         }
+        readerui:handleEvent(Event:new("SetScrollMode", false))
         local view = readerui.view
         local ctx = view:getViewContext()
         local zoom = ctx[1].zoom
@@ -75,7 +77,7 @@ describe("Readerview module", function()
                 h = 800, w = 566,
             },
         }
-        assert.are.same(ctx, saved_ctx)
+        assert.are.same(saved_ctx, ctx)
         assertAlmostEquals(zoom, 0.95011876484561, 0.0001)
 
         assert.is.same(view.state.page, 1)
@@ -89,4 +91,51 @@ describe("Readerview module", function()
         assert.is.same(view.visible_area.x, 0)
         assert.is.same(view.visible_area.y, 10)
     end)
+
+    it("should return and restore view context in scroll mode", function()
+        local sample_pdf = "spec/front/unit/data/2col.pdf"
+        local readerui = ReaderUI:new{
+            document = DocumentRegistry:openDocument(sample_pdf),
+        }
+        readerui:handleEvent(Event:new("SetScrollMode", true))
+        local view = readerui.view
+        local ctx = view:getViewContext()
+        local zoom = ctx[1].zoom
+        ctx[1].zoom = nil
+        local saved_ctx = {
+            {
+                 gamma = 1,
+                 offset = {x = 17, y = 0},
+                 page = 1,
+                 page_area = {
+                     h = 800,
+                     w = 566,
+                     x = 0,
+                     y = 0,
+                 },
+                 rotation = 0,
+                 visible_area = {
+                     h = 800,
+                     w = 566,
+                     x = 0,
+                     y = 0,
+                 },
+            },
+        }
+
+        assert.are.same(saved_ctx, ctx)
+        assertAlmostEquals(zoom, 0.95011876484561, 0.0001)
+
+        assert.is.same(view.state.page, 1)
+        assert.is.same(view.visible_area.x, 0)
+        assert.is.same(view.visible_area.y, 0)
+        saved_ctx[1].page = 2
+        saved_ctx[1].zoom = zoom
+        saved_ctx[1].visible_area.y = 10
+        view:restoreViewContext(saved_ctx)
+        assert.is.same(#view.page_states, 1)
+        assert.is.same(view.page_states[1].page, 2)
+        assert.is.same(view.page_states[1].visible_area.x, 0)
+        assert.is.same(view.page_states[1].visible_area.y, 10)
+    end)
 end)