sumatrapdf icon indicating copy to clipboard operation
sumatrapdf copied to clipboard

SumatraPDF looses its window size when opens a PDF restored from the Recycle Bin

Open sergsako opened this issue 1 year ago • 10 comments

SumatraPDF version pre-release 3.6.16004 but stable 3.5.2 is not affected

Describe the bug When I open a PDF file restored from the Recycle Bin, SumatraPDF forgets its window size and opens half-screen wide

To Reproduce Steps to reproduce the behavior:

  1. Open a PDF from a folder by double-click.
  2. SumatraPDF respects its last used window size (maximized in my case).
  3. Close the document tab, close the program window, delete the PDF file to the Recycle Bin (using the Windows Explorer, but the same happens if the new command of the pre-release to delete a document is used).
  4. Restore the PDF file from the Recycle Bin to the initial folder and open the PDF by double-click.
  5. SumatraPDF starts with the PDF doc, but in a half-width window.

Expected behavior SumatraPDF should remember its window size.

File that reproduces the problem Not related to a specific document.

Screenshots No screenshots.

Additional context Windows 10 x64 21H2. SumatraPDF setting: option: default zoom: fit width.

sergsako avatar Apr 05 '24 12:04 sergsako

Can't reproduce. Is it with tabs enabled?

Can you make a video?

kjk avatar Apr 05 '24 13:04 kjk

Yes, tabs are enabled. Portable SumatraPDF pre-release was used, only the new EXE was put to a usual folder of the program, the sumatrapdfcache folder and SumatraPDF-settings.txt file of the previous version were removed, then after the first start the window was maximized and the "default zoom: fit width" setting was made, no more. No, I am not able to make a video. Sorry, if the problem is irreproducible.

sergsako avatar Apr 05 '24 15:04 sergsako

https://github.com/sumatrapdfreader/sumatrapdf/assets/106967914/379c4adc-c89b-44d3-8a16-6de9adfe6133

I made a video, somehow.

sergsako avatar Apr 05 '24 17:04 sergsako

Thanks, it's not related to deleting a file, restoring maximized state doesn't work.

Simpler repro:

  • open a file
  • maximize window
  • close the file
  • close the window
  • double-click the file

It should open maximized but instead of flashes as if were maximized and then it seems it gets resized. At the same time the maxmize button shows as if the window was maximized.

So I suspect we first maxmize the window and then resize / reposition the window.

kjk avatar Apr 05 '24 18:04 kjk

I can fix it by adding:

        args->noPlaceWindow = true;

in SumatraPDF.cpp@1856 but then it causes to fullscreen even if window wasn't fullscreen when closed.

kjk avatar Apr 05 '24 18:04 kjk

This was mentioned in https://github.com/sumatrapdfreader/sumatrapdf/issues/4036

Started after this commit https://github.com/sumatrapdfreader/sumatrapdf/commit/ea9d8deebe4c592a637288ac4793a8188d481c3c

reccate avatar Apr 06 '24 03:04 reccate

Yeah, that logic is complicated and I frankly don't truly understand it anymore.

kjk avatar Apr 06 '24 14:04 kjk

I can't figure out what the function of the code at line 1257 to 1275 is for.

Remove this block of code work for me.

    bool shouldPlace = args->isNewWindow || args->placeWindow && fs;
    if (args->noPlaceWindow) {
        shouldPlace = false;
    }
    if (shouldPlace) {
        if (args->isNewWindow && fs && !fs->windowPos.IsEmpty()) {
            // Make sure it doesn't have a position like outside of the screen etc.
            Rect rect = ShiftRectToWorkArea(fs->windowPos);
            // This shouldn't happen until !win.IsAboutWindow(), so that we don't
            // accidentally update gGlobalState with this window's dimensions
            MoveWindow(win->hwndFrame, rect);
        }
        if (args->showWin) {
            ShowWindow(win->hwndFrame, showType);
        }
        if (win) {
            UpdateWindow(win->hwndFrame);
        }
    }

reccate avatar May 29 '24 08:05 reccate

Well, the reason I couldn't fix this bug so far is because the logic for opening files became very complex over time (unfortunately).

The purpose of this code is what it says: if a window happens to outside of screen (e.g. you had window on the right side, you close sumatra, you lower resolution of the screen, you re-open sumatra, position of windows is restored and now invisible) it repositions the window so it's fully visible.

I'm sure removing that fixes the one case you're testing but the reason this code is complex is because there are many ways to open a file (restore session at startup, dde commands, force reload with 'r', file / open, drag&drop, lazy loading) and just removing that part likely breaks some other scenario.

kjk avatar May 29 '24 09:05 kjk

Thanks for your explanation.

Your Solution work flawless for me.

 args->noPlaceWindow = true;

reccate avatar May 29 '24 14:05 reccate