bug#31920: 26.1; frame appears in wrong part of desktop after restoring frameset from fullscreen

[Robert Pluim <rpluim@gmail.com>]

martin rudalics <rudalics@gmx.at> writes:

>> Which ends up calling frameset--restore-frame, so the problem is elsewhere.
>
> Aha.  I have no idea how to debug these cl-forms so I usually end up
> in some sort of nirvana.  On Windows I call WM_EMACS_SETWINDOWPOS (in
> my_set_window_pos) with
>
>   x = 902,
>   y = 18,
>   cx = 0,
>   cy = 0,
>
> and Windows gets back to me with a WM_MOVE for (0, 0) - the values
> offered by GetWindowRect being
>
>   left = 0,
>   top = 0,
>   right = 680,
>   bottom = 658
>
> I have no idea what to learn from this: (902, 18) is the correct
> request and I see no intervening action from there until Windows
> returns (0, 0).
>

Thatʼs similar to what Iʼm seeing. gtk_window_move is called with the
right parameters, but the frame ends up in the wrong place.

>> The code that causes the frame to be restored in the wrong place is
>> this:
>>
>>      (modify-frame-parameters frame
>> 			     (if (eq (frame-parameter frame 'fullscreen) fullscreen)
>> 				 ;; Workaround for bug#14949
>> 				 (assq-delete-all 'fullscreen filtered-cfg)
>> 			       filtered-cfg))
>>
>> in framset--restore-frame, which means Iʼm going to have to break out
>> gdb and/or printf.
>
> And removing the special fullsreen handling doesn't change anything?
> Maybe we _should_ do something special when a fullscreen frame is
> restored to a non-fullscreen one.

If I understand that code, it says "if the old and new fullscreen
states are the same, donʼt pass fullscreen to modify-frame-parameters"
(itʼs Friday afternoon, I may be wrong, and the fullscreen variable
there has an unhelpful name :-) )

> Basically, Emacs has been doing something inherently wrong all the
> time: It asks to resize a frame while that frame is in fullscreen (or
> maximized) state.  The correct interpretation on behalf of the window
> manager would be to store the new sizes and apply them when the frame
> is returned to its normal (non-fullscreen/non-maximized) state, IMHO.
> For some reason, the approach chosen by Emacs has worked so I never
> tried to fiddle with it.  But maybe it bites us this time.

So youʼre saying we should un-maximize, and then set the frame size
afterwards? The patch below tries that, it works for me, although it
does of course cause the frame to resize and then move in two steps.

>>(Iʼm surprised Eli is seeing this on MS-Windows
>> though, I thought the low-level frame implementation was completely
>> separate)
>
> I see this on Windows too.  Normally, buggy behavior consistent across
> platforms is an asset.  For some reason, this doesn't apply here yet.

It turns our that most of the code is common, only the implementations
of things like x_set_offset and x_calc_absolute_position are
platform-specific. Iʼm still surprised they share the same bugs.

Robert

diff --git i/lisp/frame.el w/lisp/frame.el
index 29c31f41cb..a58fad6481 100644
--- i/lisp/frame.el
+++ w/lisp/frame.el
@@ -2413,7 +2413,7 @@ toggle-frame-maximized
      (t
       (set-frame-parameter nil 'fullscreen 'maximized)))))
 
-(defun toggle-frame-fullscreen ()
+(defun toggle-frame-fullscreen (&optional frame)
   "Toggle fullscreen state of selected frame.
 Make selected frame fullscreen or restore its previous size if it
 is already fullscreen.
@@ -2431,14 +2431,14 @@ toggle-frame-fullscreen
 
 See also `toggle-frame-maximized'."
   (interactive)
-  (let ((fullscreen (frame-parameter nil 'fullscreen)))
+  (let ((fullscreen (frame-parameter frame 'fullscreen)))
     (if (memq fullscreen '(fullscreen fullboth))
-	(let ((fullscreen-restore (frame-parameter nil 'fullscreen-restore)))
+	(let ((fullscreen-restore (frame-parameter frame 'fullscreen-restore)))
 	  (if (memq fullscreen-restore '(maximized fullheight fullwidth))
-	      (set-frame-parameter nil 'fullscreen fullscreen-restore)
-	    (set-frame-parameter nil 'fullscreen nil)))
+	      (set-frame-parameter frame 'fullscreen fullscreen-restore)
+	    (set-frame-parameter frame 'fullscreen nil)))
       (modify-frame-parameters
-       nil `((fullscreen . fullboth) (fullscreen-restore . ,fullscreen))))
+       frame `((fullscreen . fullboth) (fullscreen-restore . ,fullscreen))))
     ;; Manipulating a frame without waiting for the fullscreen
     ;; animation to complete can cause a crash, or other unexpected
     ;; behavior, on macOS (bug#28496).
diff --git i/lisp/frameset.el w/lisp/frameset.el
index 0e3363d7ae..ffbf6722a7 100644
--- i/lisp/frameset.el
+++ w/lisp/frameset.el
@@ -1085,6 +1085,11 @@ frameset--restore-frame
       (when (frame-live-p parent-frame)
         (set-frame-parameter frame 'parent-frame parent-frame)))
 
+    (let ((old-fullscreen (frame-parameter frame 'fullscreen)))
+      (and (not (eq old-fullscreen fullscreen))
+           (memq old-fullscreen '(fullscreen fullboth))
+           (not fullscreen)
+           (toggle-frame-fullscreen frame)))
     (modify-frame-parameters frame
 			     (if (eq (frame-parameter frame 'fullscreen) fullscreen)
 				 ;; Workaround for bug#14949