Reset document font and page settings #8412

hius07 · 2021-11-03T15:57:42Z

Allows to reset document view (font and page) settings to default.
Available for reflowable documents via the Reset settings button.
Closes #7708.

Additionally: swaps two buttons in the History popup menu for consistency with the file browser popup menu.

This change is

Frenzie · 2021-11-03T19:14:32Z

Reset display?

poire-z · 2021-11-03T19:35:50Z

frontend/apps/filemanager/filemanager.lua

@@ -253,6 +253,18 @@ function FileManager:setupLayout()
                    text = _("Reset settings"),
                    enabled = is_file and DocSettings:hasSidecarFile(BaseUtil.realpath(file)),
                    callback = function()
+                        local other_buttons
+                        if DocumentRegistry:getProvider(file).provider == "crengine" then


Why limit this to crengine documents ? Some cre docs may be opened with MuPDF, and vice-versa.

poire-z · 2021-11-03T19:38:51Z

frontend/apps/filemanager/filemanagerutil.lua

+-- Purge reflowable doc font and page settings
+function filemanagerutil.purgeViewSettings(file)
+    local view_settings = {
+        "font_face",


Again, why only reflowable docs ?

poire-z · 2021-11-03T19:44:55Z

frontend/apps/filemanager/filemanagerutil.lua

+        -- crop tab
+        "copt_h_page_margins",
+        "copt_t_page_margin",
+        "copt_b_page_margin",
+        -- pageview tab
+        "copt_line_spacing",
+        "line_space_percent",
+        -- textsize tab


What is kept then ?
And why not remove visible_pages, page_overlap_style, toc_ticks_ignored_levels, book style tweak ?
I'm not saying these should be deleted, it just feels a bit arbitrary to remove your choice of settings, or mine - and being not really sure of what got reset.
Before looking at your code, from the screenshots and in spite of the wording, I thought this option would remove everything except bookmarks/highlights (ie. a safer Reset settings :)

Have you read the Issue related?
It's a real use case I understand well.
Only settings from the bottom menu are reset (plus the font face).

I've just read it, and I understand it.
But this use case is just a use case of changing fonts and wanting to reset some font related settings so the new defaults associated to your new preferred default fonts are used instead.
But Word spacing and page margins are not related to changing fonts - so you might do a bit more than some use case would wish.
Or a bit less than some other use case would wish (ie. I messed with style tweaks, Toc ticks, page overlap).

But may be your choice of settings is the most consistent and the one most wished for, so why not. But it's a bit low at the area of use cases it can solve, as it's limited to a single set of settings.

And it still feels a bit convoluted to have to close the book, open another book, go to History, long-press on that book to Reset page and font settings, and re-open it.
Ideally, you would have another menu invoked on the current opened book. a set of "sets" of settings as radio buttons, that you would check for what you want remove: it would close the book, clean up settings, and re-open the book.

poire-z · 2021-11-03T19:53:21Z

Additionally: swaps two buttons in the History popup menu for consistency with the file browser popup menu.

Not really convinced this is best. Yes, Reset settings were a bit distant when comparing these menu, but it's not the thing we (I ?) use the most when we get to this menu (so, the consistency argument feels light). Remove from history is for me, and it's less destructive than Reset settings, so I was fine with having it on the right easily accessible.

hius07 · 2021-11-03T19:59:38Z

Can we have Reset at left or at right side, but the same in both popup menus?

poire-z · 2021-11-03T20:12:13Z

Can we have Reset at left or at right side, but the same in both popup menus?

I guess I'd prefer this - if you manage to re-order the other ones in a consistent way.

hius07 · 2021-11-04T12:28:10Z

I'm so sad with disabled CircleCI tests for my commits.
I've tried everything in the settings of CircleCI and my GitHub repository, nothing helps.

poire-z · 2021-11-05T11:11:23Z

frontend/ui/elements/reader_menu_order.lua

@@ -38,12 +38,13 @@ local order = {
        "bookmarks_settings",
    },
    typeset = {
+        "reset_view_settings",


Can be renamed reset_document_settings as it is no longer just "view" settings.

poire-z · 2021-11-05T11:14:03Z

frontend/apps/filemanager/filemanagerutil.lua

 function filemanagerutil.purgeSettings(file)
    local file_abs_path = util.realpath(file)
    if file_abs_path then
        DocSettings:open(file_abs_path):purge()
    end
 end

+-- Purge doc settings except kept
+function filemanagerutil.purgeViewSettings(file)


Not sure it is now appropriate to have this defined in filenamanagerutil - as we don't invoke it at all from FileManager - and it is quite not different than the above function.
But I can't think of an obvious better place for this (docsettings.lua is a generic and currently doesn't know about any real setting).

I guess this function name can now be filemanagerutil.resetDocumentSettings(file)

poire-z · 2021-11-05T11:16:51Z

frontend/apps/filemanager/filemanagerutil.lua

+    if file_abs_path then
+        local doc_settings = DocSettings:open(file_abs_path)
+        for k in pairs(doc_settings.data) do
+            if not require("util").arrayContains(settings_kept, k) then


This is a bit ugly/expensive (doesn't matter much in this context, so, just ugly :)
I would build a hash table from the array, and just use the hash table:
local settings_to_keep = {}
for _, k in ipairs(settings_kept ) do settings_to_keep[k] = true end
and here just:
if not settings_to_keep[k] then
or just define this little bit uglier:

local settings_kept = { "bookmarks" = true "bookmarks_sorted" = true "bookmarks_version" = true

poire-z · 2021-11-06T17:18:48Z

frontend/apps/filemanager/filemanagerutil.lua

 function filemanagerutil.purgeSettings(file)
    local file_abs_path = util.realpath(file)
    if file_abs_path then
        DocSettings:open(file_abs_path):purge()
    end
 end

+-- Purge doc settings except kept
+function filemanagerutil.purgeViewSettings(file)


I guess this function name can now be filemanagerutil.resetDocumentSettings(file)

poire-z · 2021-11-06T17:20:21Z

frontend/apps/filemanager/filemanagerutil.lua

+        bookmarks = true,
+        bookmarks_sorted = true,
+        bookmarks_version = true,
+        cre_dom_version = true,
+        highlight = true,
+        highlights_imported = true,
+        last_page = true,
+        last_xpointer = true,


Have you had another look at some of your setting files ? Or did you just trust my quick look ? :)

It's my list.

poire-z · 2021-11-06T17:21:27Z

frontend/apps/filemanager/filemanagerutil.lua

 function filemanagerutil.purgeSettings(file)
    local file_abs_path = util.realpath(file)
    if file_abs_path then
        DocSettings:open(file_abs_path):purge()
    end
 end

+-- Purge doc settings except kept
+function filemanagerutil.purgeViewSettings(file)
+    local settings_kept = {


(settings_to_keep sounds better)

poire-z · 2021-11-06T17:24:37Z

frontend/apps/reader/modules/readermenu.lua

+        keep_menu_open = true,
+        callback = function()
+            UIManager:show(ConfirmBox:new{
+                text = _("Reset current document settings to default values?\n\nReading position, highlights and bookmarks will be kept.\nThe document will be reopened."),


to their default values ? to defaults ? to your default values @Frenzie ?

We have (The book will be reloaded.) in readertoc.lua - sounds a bit better than reopened to my french ears.

to their default values sounds good to me.

The book will be reloaded.

That was initially, but after this ConfirmBox we immediately have an infomessage "Opening file...", so I've changed to "reopened".

hius07 · 2021-11-07T06:24:15Z

Ready

poire-z · 2022-02-04T00:35:59Z

Minor issue noticed at #8770 - not sure we really need to find a solution.
Set default Render mode to "book". Then "Reset document settings": Render mode becomes "legacy".

Because of this bit of logic (depending on last_xpointer, which is preserved by "Reset doc settings"):

koreader/frontend/apps/reader/modules/readertypeset.lua

Lines 63 to 79 in cb7e670

    
           -- Block rendering mode: stay with legacy rendering for books 
        
           -- previously opened so bookmarks and highlights stay valid. 
        
           -- For new books, use 'web' mode below in BLOCK_RENDERING_FLAGS 
        
           if config:has("copt_block_rendering_mode") then 
        
               self.block_rendering_mode = config:readSetting("copt_block_rendering_mode") 
        
           else 
        
               if config:has("last_xpointer") then 
        
                   -- We have a last_xpointer: this book was previously opened 
        
                   self.block_rendering_mode = 0 
        
               else 
        
                   self.block_rendering_mode = G_reader_settings:readSetting("copt_block_rendering_mode") 
        
                                            or 3 -- default to 'web' mode 
        
               end 
        
               -- Let ConfigDialog know so it can update it on screen and have it saved on quit 
        
               self.ui.document.configurable.block_rendering_mode = self.block_rendering_mode 
        
           end 
        
           self:setBlockRenderingMode(self.block_rendering_mode)

hius07 · 2022-02-04T04:04:22Z

What's going on with saved highlights when changing rendering mode?

poire-z · 2022-02-04T06:52:37Z

Some old hell... detailed in #3940 and buggins/coolreader#125 (comment).

For old read books with highlights not opened since 2018, if the user is to re-open them now, we need to render them at least one last time with some old code (cre_dom_version and block_rendering_mode = legacy) to be able to get xpointers the same as before 2018. Only that way we are able to migrate xpointers and allow using new dom_version and changing block rendering.

hius07 · 2022-02-04T07:44:05Z

Set default Render mode to "book". Then "Reset document settings": Render mode becomes "legacy".

I think this must be fixed.
We can remove last_xpointer in filemanagerutil.resetDocumentSettings and add a warning to the ConfirmBox, something like "Reading position will be lost. Make a bookmark if you need to save it".

poire-z · 2022-02-04T07:56:13Z

That's a bit radical and not user friendly :)

We may be can do better if, on reset settings, you add docsettings_reset=true, check for this:
if config:has("last_xpointer") and not config:has("docsettings_reset") then
and get rid of that flag at end of ReaderUI:init() or in some onReaderReady().

(I don't like the word "reset" which is identical whether it's something done or something to do... if I saw "reseted", I would understand it is something done, but that's not correct English :)

hius07 added 3 commits November 3, 2021 17:52

filemanagerutil: reset view settings

0e2a416

filemanage: reset view settings

cde45fb

filemanagerhistory: reset view settings

b223315

Frenzie added this to the 2021.11 milestone Nov 3, 2021

poire-z reviewed Nov 3, 2021

View reviewed changes

hius07 mentioned this pull request Nov 4, 2021

FR: Apply current defaults to this document #7708

Closed

hius07 added 4 commits November 4, 2021 13:59

filemanager: revert

656dd99

filemanagerhistory: revert

51bc90c

readermenu: reset view settings

12cb48c

reader_menu_order: reset view settings

88c475a

hius07 added 5 commits November 4, 2021 14:58

readermenu: help text

02775cc

filemanagerutil: reset kopt settings

8065150

readermenu: help text for pdf

075e267

filemanagerutils: purge all except kept

c15b35e

readermenu: reset all settings except kept

ee077ce

poire-z reviewed Nov 5, 2021

View reviewed changes

hius07 added 3 commits November 5, 2021 13:46

reader_menu_order: item name

2b6742b

readermenu: item name

8221b2d

filemanagerutil: optimize check

6334fd9

poire-z reviewed Nov 6, 2021

View reviewed changes

hius07 added 2 commits November 7, 2021 08:23

Update filemanagerutil.lua

fab0ad4

Update readermenu.lua

e990444

poire-z approved these changes Nov 7, 2021

View reviewed changes

poire-z merged commit 3dabbd5 into koreader:master Nov 7, 2021

hius07 deleted the hius07-reset-view-settings branch November 7, 2021 19:38

hius07 mentioned this pull request Feb 9, 2022

Doc settings reset: fixes #8792

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reset document font and page settings #8412

Reset document font and page settings #8412

hius07 commented Nov 3, 2021 •

edited

Frenzie commented Nov 3, 2021

poire-z Nov 3, 2021

poire-z Nov 3, 2021

poire-z Nov 3, 2021

hius07 Nov 3, 2021 •

edited

poire-z Nov 3, 2021

poire-z commented Nov 3, 2021

hius07 commented Nov 3, 2021

poire-z commented Nov 3, 2021

hius07 commented Nov 4, 2021

poire-z Nov 5, 2021

poire-z Nov 5, 2021

poire-z Nov 6, 2021

poire-z Nov 5, 2021

hius07 Nov 5, 2021

poire-z Nov 6, 2021

poire-z Nov 6, 2021

hius07 Nov 6, 2021

poire-z Nov 6, 2021

poire-z Nov 6, 2021

Frenzie Nov 6, 2021

hius07 Nov 6, 2021

hius07 commented Nov 7, 2021

poire-z commented Feb 4, 2022

hius07 commented Feb 4, 2022

poire-z commented Feb 4, 2022

hius07 commented Feb 4, 2022

poire-z commented Feb 4, 2022

Reset document font and page settings #8412

Reset document font and page settings #8412

Conversation

hius07 commented Nov 3, 2021 • edited

Frenzie commented Nov 3, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hius07 Nov 3, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

poire-z commented Nov 3, 2021

hius07 commented Nov 3, 2021

poire-z commented Nov 3, 2021

hius07 commented Nov 4, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hius07 commented Nov 7, 2021

poire-z commented Feb 4, 2022

hius07 commented Feb 4, 2022

poire-z commented Feb 4, 2022

hius07 commented Feb 4, 2022

poire-z commented Feb 4, 2022

hius07 commented Nov 3, 2021 •

edited

hius07 Nov 3, 2021 •

edited