The road to 3.5

[kjk]

Time to release 3.5 which means: finalize things, outside of finalization only small changes and bugfixes. Other stuff can be developed on a branch

Specifically:

• in dark themes, tree view is not colored properly and document colors are inverter. The notion of GetDocumentColors() is not great
• Black box around all new annotations #3732
• don't invert colors by default in dark themes. it's probably too confusing for users
• there's a crash in SaveAs (probably when saving a file that doesn't have an engine i.e. .chm
• fix crash from [bug] don't work RememberStatePerDocument #3729 (comment)
• look at IsDocLoaded() usage - with lazy loading in now returns false where it used to return true

Done:

• add theme selection to menu
• add a setting to remember themes
• fix v3.5+ erroneously complains about "unsaved annotations" #3448
• fix Saving annotations corrupts PDF file #3551
• fix: on lazy loading the files do not restore TabState. Not sure if will keep lazy loading but should fix it
• minimize logging that we've added to track misc stuff
• fix "Current Annotation" colour is not themed #3675
• disable / ignore auto-refresh during Ctrl + Shift + S. Not sure how best do it. Set a flag on WindowTab to tell it to ignore one auto-refresh?
• fix Cursor Keys don't work in Annotation "Contents" field #3685
• a notion of selected annotation
• list on Annotation* should be in EngineMupdf with lifetimes tied to it. Currently we have a confusing mix of lifetimes tied to EditAnnotationsWindow and new objects (returned by GetAnnotationAtPos()). It'll be faster and easier to manage lifetimes
• fix [Vulnerability] Denial Of Service CVE-2023-33802 in 32 bit 3.4.6 #3615
• Crashes after changing theme (or just settings?) multiple times #3691
• menu radio check is not painted in dark theme (custom draw), see Settings / Theme menu
• tab colors in dark themes are pretty random
• trim down what goes into a theme as much as possible
• after switching theme should redraw all tabs in all windows
• apply theme colors to LabelWithCloseWnd
• when edit annotations window is shown, creating circle annotation doesn't add it to listbox
• e or Edit Annotations command from palette doesn't show annot edit window unless mouse over an annotation. With context menu it always shows
• change CmdEditAnnotations keybinding from 'e' to Ctrl + E
• look into Assert in SumatraUIAutomationProvider::OnDocumentLoad #3683
• look into 3.5 pre-alpha does not maintain text highlight when changing documents #3473
• fix Possibly a memory corruption #3679
• fix Annotatoins crash #3692
• settled on: e to select annotation without opening edit annotations window, Ctrl + E to select with opening. This is to avoid conflicts with existing mouse functionality i.e. ctrl + click is for rectangular selection. was: a way to select annotation. double-click conflicts with "select word" so maybe ctrl + click? ctrl + double-click? alt + click?
• when mouse over an annotation, show a notification message e to select, ctrl + E to edit to inform about this functionality. Not sure if will keep it
• when mouse move over selected movable annotation, change the cursor to hand to indicate it can be moved
• in edit annotation window, Del should delete currently selected annotation
• change Del to delete currently selected annotation, not the under the curso
• can only move selected annotation (no more ctrl - drag for moving)
• when cursor over annotation, cannot start text selection
• creating 'a' annotation when edit window not open creates bogus bounds
• when Ctrl + E ensure listbox is synced with selectedAnnotation
• Moving annotations is broken? #3706
• annotation under cursor notification persists when switching tabs
• don't include translations that have > 100 untranslated strings
• look into Comments could not be saved. #3304
• translate annotation notification
• see if can do Ctrl + click to select an annotation and Ctrl + double click to edit
• see if invert colors should be decoupled from themes. maybe on switching to dark theme sets invertColors to true but allows changing it afterwards. for light theme sets to false and also allows changing afterwards
• don't require uninstaller to be run as admin if doesn't need to
• fix cppcheck issues
• remove hidden conversion to .txt or .pdf as part of Save As
• when doing Save As and file has unsaved annotations, do CmdSaveAnnotations

[kerryland]

How about adding this one about search highlighting being lost when switching between tabs. It seems like regression, and would be annoying if you are using multiple tabs.

Now, I really have to stop poking my nose into your project!

[kerryland]

@kjk let me know when you are happy with the reworked annotations -- I'm keen to give them a try.

[kerryland]

The last build I made locally was rather crash-tastic, but I don't want to complain about it if it's still a WIP.

[GitHubRulesOK]

@kerryland Latest run went well so pre-release should be available
BEWARE browser cache may attempt 15578,
DAILY should be 15601
Testing the reported CVE


No crash but need to reset preferences to not keep trying to load big files

[kerryland]

OK, just in case there aren't known problems with annotations, here's how it died for me. Again, just FYI, the sky isn't falling :-)

Start 15601
Open "Emma"
goto page 2
create an annotation via "a"
(jumps to first page)?
press ctrl-e to view annotations
goto page 2
create another annotation via "a"
wait for the crash

[kjk]

@kerryland please do try annotations and report all problems, especially crashes.

I've bee lightly testing it and I don't get crashes anymore so consistent crash repro steps are valuable.

[kjk]

Start 15601 Open "Emma" goto page 2 create an annotation via "a" (jumps to first page)? press ctrl-e to view annotations goto page 2 create another annotation via "a" wait for the crash

@kerryland for what it's worth, I can't repro that in latest build (I'm talking in github, the pre-release build only happens once a day so it might be delayed).

I fixed a bunch of annotation crashes today.

[GitHubRulesOK]

@kjk Clean run instant crash as per above instructions?
sumatrapdfcrash.zip
Emma - Jane Austen - Copy.pdf

[kjk]

Yeah, I believe I've fixed it, it just didn't propagate yet to pre-release build.

[kjk]

So the final (?) design for annotations:

• when cursor over annotation we show notification with type of annotation and help text
• e selects annotation under cursor; Ctrl + E selects and opens annotations editor
• selected (and moveable) annotation can be moved
• Delete deletes selected annotation when focus is in main window or annotations editor

[GitHubRulesOK]

sounds OK ish :-)
so side issues you say "Maybe" with respect to UI highlight and both sidebar and users preference is ALL black OR whitish plus contrast
Thus a UI sidebar etc. highlight of #7f7f7f should with luck suit both camps ? also for top bar icons the prime problem is the scale when grey so a colour setting of forground all black all white and mid grey should help keep the number of colours to 3

That last point about highlighted text is a problem (some other editors do and some dont!) I was happy it was auto copied from selection to comment but then there are others complain so it needs an optional switch.

[kerryland]

15623 feedback:

Isn't putting focus in 'Contents' field

• ctrl-e
• a to add
• put focus in text entry field. (also, I vote against defaulting it to the selection text!)

done

e should open annotation editor if it's closed

In an earlier version an e would open the annotation editor. I don't see why it shouldn't now -- it's not like it's doing anything else. This does bring into question why ctrl-e would need to exist. I'd argue that it doesn't.

see comment for longer explanation

"Highlight Annotation: Tooltip timeout is too short

• Personally, I wouldn't have a timeout at all. Remove it when the mouse is moved.
• It's also in a funny location. It should be near the annotation I think Perhaps it should be where "You have unsaved annotations" appears; that would make it a little zone for all kinds of notifications.
• I'm not sure about the wording. How about "e to edit" (especially after revert to logic where open Automation Editor automatically). I would also want to remove 'Ctrl + e to start edit'

open issue for more discussion
I worry about obscuring some content hence the timeout but I'm fine without the timeout
I agree location could be better but it's easy to do with notifications and doing more is post-3.5

Can't save annotations from within annotation editor

• This is a mild irritation. ctrl-shift-s should work from inside the annotation editor too

fixed

Saving annotations shouldn't close the annotation editor

I'm saving because I'm paranoid, not because I'm finished editing annotations :-)
I do understand why you're closing it, but that's a technical problem, not mine as an end user :-)

fixed

Crashes if ctrl-e after saving

• 'a' add an annotation
• ctrl-shift-s to save
• ctrl-e
• crash

fixed

Annotations are lost if you decide not to save as a new file

• 'a' to add an annotation
• 'q' to quit (I hit this accidentally, btw. I'm not sure a single keypress should close the document, but anyway...)
• Choose 'Save to new PDF', but then click 'Cancel' because you have changed your mind
• Cry out in pain because document has closed, and your hard work lost.

fixed

Crashes if delete repeatedly

• Open a file with annotations
• crtl-e to open AE
• Hit 'Delete Annotations' button repeatedly
• Crash

fixed

Note that 'Del' shortcut is fine.

Annotation deletion should not navigate to another annotation

• Create annotation on page 1
• Create annotation on page 100
• Navigate to page 100
• Delete, via AE, the annotation on page 100
• User should still be on page 100, not ripped away to page 1.

This works OK when delete via "Delete Annotation" button, but not via Del shortcut

fixed

Adding an Annotation shouldn't adjust the PDF view

Here's an extreme example:

• Scroll the PDF view so that a page split is in the middle of the screen (you can see the bottom half of page 1 and the top half of page 2)
• Highlight a word near the top of page 2, and press 'a'
• The PDF view should not rearrange itself, placing the view at the top of page 2. (this is new behaviour)

makes sense, open issue, probably related to go to page

PS: The latest commit (305f203, when opening edit annotations window, sync with selectedAnnotation) crashes the moment you press a :-(

hmm, can't repro this

[kerryland]

Let me know if you agree with the above and I'll happily move each of them into individual tickets. A massive comment is not the best place to track bug reports -- sorry about that!

[kjk]

I've added comments inline, mostly agree, please open issues.

As to e vs. Ctrl + E this is kind of cascading logic i.e. one thing leads to another:

• I don't want an easy to press by accident key (i.e. e) to invoke a potentially confusing annotations window hence Ctrl + E which is still easy to press but not accidentally
• but I still want an easy way to select annotation so that it can be moved with mouse. Initially I planned Ctrl + click but that also starts rectangular selection so I went with e as kind-of-related to Ctr-E
• I might have been wrong about Ctrl + click in that it might be possible to do both select annotation on Ctrl + click and still support rectangular selection. In which case I would remove e key binding (but leave CmdSelectAnnotation command so that users can bind it themselves)

[kerryland]

@kjk
I just noticed this in the "maybe" list:

move selectedAnnotation from WindowTab to MainWindow?

What benefit would that bring? Wouldn't it mean that you would lose the "selected annotation" when you flip between tabs?
Isn't that also the problem behind #3473, because findThread is on MainWindow rather than WindowTab?

[kjk]

Yeah, good points, hence "maybe".