From b7d2443d141e9f318dc4cae8d8a33e81fe36f859 Mon Sep 17 00:00:00 2001 From: Martin Rudalics Date: Fri, 9 May 2025 09:36:00 +0200 Subject: [PATCH] Fix infinite looping in 'next-frame' and associates (Bug#77985) * src/frame.c (next_frame): Rewrite to avoid infinite looping if FRAME itself does not qualify as candidate frame (Bug#77985). (Fnext_frame, Fprevious_frame): Adjust do-strings. * lisp/frame.el (other-frame): Adjust doc-string. (frame-list-1): New function. (make-frame-names-alist): Rewrite using 'frame-list-1' instead of 'next-frame' (Bug#77985). (delete-other-frames): Rewrite using 'frame-list' instead of 'next-frame'. * doc/lispref/frames.texi (Finding All Frames): Minor clarifications for 'frame-list' and 'next-frame'. (cherry picked from commit 004187350238a05a5c7f818c9feb648c792fe9db) --- doc/lispref/frames.texi | 17 +++++--- lisp/frame.el | 84 +++++++++++++++++++++++--------------- src/frame.c | 89 +++++++++++++++++++++++++++-------------- 3 files changed, 121 insertions(+), 69 deletions(-) diff --git a/doc/lispref/frames.texi b/doc/lispref/frames.texi index 73e6b6268d4..0ad4f52bfc0 100644 --- a/doc/lispref/frames.texi +++ b/doc/lispref/frames.texi @@ -2886,15 +2886,16 @@ the @code{delete-frame} call in a @code{condition-case} form. @defun frame-list This function returns a list of all the live frames, i.e., those that have not been deleted. It is analogous to @code{buffer-list} for -buffers, and includes frames on all terminals. The list that you get -is newly created, so modifying the list doesn't have any effect on the -internals of Emacs. +buffers, and includes frames on all terminals with the exception of +tooltip frames (@pxref{Tooltips}). The list that you get is newly +created, so modifying the list doesn't have any effect on the internals +of Emacs. @end defun @defun visible-frame-list This function returns a list of just the currently visible frames. -@xref{Visibility of Frames}. Frames on text terminals always count as -visible, even though only the selected one is actually displayed. +@xref{Visibility of Frames}. Frames on text terminals will count as +visible even though only the selected one is actually displayed. @end defun @defun frame-list-z-order &optional display @@ -2917,7 +2918,7 @@ This function lets you cycle conveniently through all the frames on a specific terminal from an arbitrary starting point. It returns the frame following @var{frame}, in the list of all live frames, on @var{frame}'s terminal. The argument @var{frame} must specify a live -frame and defaults to the selected frame. It never returns a frame +frame and defaults to the selected frame. It does not return a frame whose @code{no-other-frame} parameter (@pxref{Frame Interaction Parameters}) is non-@code{nil}. @@ -2937,6 +2938,10 @@ minibuffer window. @item anything else Consider all frames. @end table + +If this function does not find a suitable frame, it returns @var{frame} +even if it would not qualify according to the @var{minibuf} argument or +its @code{no-other-frame} parameter. @end defun @defun previous-frame &optional frame minibuf diff --git a/lisp/frame.el b/lisp/frame.el index b38b41af543..9ffdae698b2 100644 --- a/lisp/frame.el +++ b/lisp/frame.el @@ -1226,10 +1226,11 @@ recently selected windows nor the buffer list." (set-mouse-position frame (1- (frame-width frame)) 0)))) (defun other-frame (arg) - "Select the ARGth different visible frame on current display, and raise it. -All frames are arranged in a cyclic order. -This command selects the frame ARG steps away in that order. -A negative ARG moves in the opposite order. + "Select the ARGth visible frame on current display, and raise it. +All frames are arranged in a cyclic order. This command selects the +frame ARG steps away from the selected frame in that order. A negative +ARG moves in the opposite order. It does not select a minibuffer-only +frame. To make this command work properly, you must tell Emacs how the system (or the window manager) generally handles focus-switching @@ -1291,18 +1292,38 @@ Calls `suspend-emacs' if invoked from the controlling tty device, (suspend-tty))) (t (suspend-emacs)))) -(defun make-frame-names-alist () - ;; Only consider the frames on the same display. - (let* ((current-frame (selected-frame)) - (falist - (cons - (cons (frame-parameter current-frame 'name) current-frame) nil)) - (frame (next-frame nil 0))) - (while (not (eq frame current-frame)) - (progn - (push (cons (frame-parameter frame 'name) frame) falist) - (setq frame (next-frame frame 0)))) - falist)) +(defun frame-list-1 (&optional frame) + "Return list of all live frames starting with FRAME. +The optional argument FRAME must specify a live frame and defaults to +the selected frame. Tooltip frames are not included." + (let* ((frame (window-normalize-frame frame)) + (frames (frame-list))) + (unless (eq (car frames) frame) + (let ((tail frames)) + (while tail + (if (eq (cadr tail) frame) + (let ((head (cdr tail))) + (setcdr tail nil) + (setq frames (nconc head frames)) + (setq tail nil)) + (setq tail (cdr tail)))))) + frames)) + +(defun make-frame-names-alist (&optional frame) + "Return alist of frame names and frames starting with FRAME. +Only visible or iconified frames on the same terminal as FRAME are +listed. Frames with a non-nil `no-other-frame' parameter are not +listed. The optional argument FRAME must specify a live frame and +defaults to the selected frame." + (let ((frames (frame-list-1 frame)) + (terminal (frame-parameter frame 'terminal)) + alist) + (dolist (frame frames) + (when (and (frame-visible-p frame) + (eq (frame-parameter frame 'terminal) terminal) + (not (frame-parameter frame 'no-other-frame))) + (push (cons (frame-parameter frame 'name) frame) alist))) + (nreverse alist))) (defvar frame-name-history nil) (defun select-frame-by-name (name) @@ -2816,32 +2837,29 @@ deleting them." (interactive "i\nP") (setq frame (window-normalize-frame frame)) (let ((minibuffer-frame (window-frame (minibuffer-window frame))) - (this (next-frame frame t)) (parent (frame-parent frame)) - next) + (frames (frame-list))) ;; In a first round consider minibuffer-less frames only. - (while (not (eq this frame)) - (setq next (next-frame this t)) - (unless (or (eq (window-frame (minibuffer-window this)) this) + (dolist (this frames) + (unless (or (eq this frame) + (eq this minibuffer-frame) + (eq (window-frame (minibuffer-window this)) this) ;; When FRAME is a child frame, delete its siblings ;; only. (and parent (not (eq (frame-parent this) parent))) - ;; Do not delete a child frame of FRAME. - (eq (frame-parent this) frame)) - (if iconify (iconify-frame this) (delete-frame this))) - (setq this next)) + ;; Do not delete frame descending from FRAME. + (frame-ancestor-p frame this)) + (if iconify (iconify-frame this) (delete-frame this)))) ;; In a second round consider all remaining frames. - (setq this (next-frame frame t)) - (while (not (eq this frame)) - (setq next (next-frame this t)) - (unless (or (eq this minibuffer-frame) + (dolist (this frames) + (unless (or (eq this frame) + (eq this minibuffer-frame) ;; When FRAME is a child frame, delete its siblings ;; only. (and parent (not (eq (frame-parent this) parent))) - ;; Do not delete a child frame of FRAME. - (eq (frame-parent this) frame)) - (if iconify (iconify-frame this) (delete-frame this))) - (setq this next)))) + ;; Do not delete frame descending from FRAME. + (frame-ancestor-p frame this)) + (if iconify (iconify-frame this) (delete-frame this)))))) (defvar undelete-frame--deleted-frames nil "Internal variable used by `undelete-frame--save-deleted-frame'.") diff --git a/src/frame.c b/src/frame.c index ee6246dd7bd..7f4303dbb01 100644 --- a/src/frame.c +++ b/src/frame.c @@ -2205,24 +2205,39 @@ candidate_frame (Lisp_Object candidate, Lisp_Object frame, Lisp_Object minibuf) static Lisp_Object next_frame (Lisp_Object frame, Lisp_Object minibuf) { - Lisp_Object f, tail; - int passed = 0; + Lisp_Object f, tail, next = Qnil; + bool passed = false; eassume (CONSP (Vframe_list)); - while (passed < 2) - FOR_EACH_FRAME (tail, f) - { - if (passed) - { - f = candidate_frame (f, frame, minibuf); - if (!NILP (f)) - return f; - } - if (EQ (frame, f)) - passed++; - } - return frame; + FOR_EACH_FRAME (tail, f) + { + if (EQ (f, frame)) + /* If we encounter FRAME, set PASSED to true. */ + passed = true; + else + { + f = candidate_frame (f, frame, minibuf); + + if (!NILP (f)) + { + if (passed) + /* If we passed FRAME already, return first suitable + candidate following it. */ + return f; + else if (NILP (next)) + /* If we didn't pass FRAME and have no suitable + candidate yet, set NEXT to the first suitable + candidate preceding FRAME. */ + next = f; + } + } + } + + /* We have scanned all frames. Return first candidate preceding FRAME + if we have found one. Otherwise return FRAME regardless of whether + it is a suitable candidate or not. */ + return NILP (next) ? frame : next; } /* Return the previous frame in the frame list before FRAME. */ @@ -2237,21 +2252,26 @@ prev_frame (Lisp_Object frame, Lisp_Object minibuf) FOR_EACH_FRAME (tail, f) { if (EQ (frame, f) && !NILP (prev)) + /* If we encounter FRAME and already have found a suitable + candidate preceding it, return that candidate. */ return prev; + f = candidate_frame (f, frame, minibuf); + if (!NILP (f)) + /* PREV is always the last suitable candidate we found. */ prev = f; } /* We've scanned the entire list. */ if (NILP (prev)) /* We went through the whole frame list without finding a single - acceptable frame. Return the original frame. */ + acceptable frame. Return FRAME. */ return frame; else - /* There were no acceptable frames in the list before FRAME; otherwise, - we would have returned directly from the loop. Since PREV is the last - acceptable frame in the list, return it. */ + /* There were no acceptable frames in the list before FRAME; + otherwise, we would have returned directly from the loop. Since + PREV is the last suitable frame in the list, return it. */ return prev; } @@ -2259,7 +2279,7 @@ prev_frame (Lisp_Object frame, Lisp_Object minibuf) DEFUN ("next-frame", Fnext_frame, Snext_frame, 0, 2, 0, doc: /* Return the next frame in the frame list after FRAME. Only frames on the same terminal as FRAME are included in the list -of candidate frames. If omitted, FRAME defaults to the selected frame. +of candidate frames. FRAME defaults to the selected frame. If MINIFRAME is nil (the default), include all frames except minibuffer-only frames. @@ -2271,7 +2291,9 @@ If MINIFRAME is `visible', include only visible frames. If MINIFRAME is 0, include only visible and iconified frames. -If MINIFRAME is any other value, include all frames. */) +If MINIFRAME is any other value, include all frames. + +Return FRAME if no suitable next frame is found. */) (Lisp_Object frame, Lisp_Object miniframe) { if (NILP (frame)) @@ -2282,15 +2304,22 @@ If MINIFRAME is any other value, include all frames. */) DEFUN ("previous-frame", Fprevious_frame, Sprevious_frame, 0, 2, 0, doc: /* Return the previous frame in the frame list before FRAME. -It considers only frames on the same terminal as FRAME. -By default, skip minibuffer-only frames. -If omitted, FRAME defaults to the selected frame. -If optional argument MINIFRAME is nil, exclude minibuffer-only frames. -If MINIFRAME is a window, include only its own frame -and any frame now using that window as the minibuffer. -If MINIFRAME is `visible', include all visible frames. -If MINIFRAME is 0, include all visible and iconified frames. -Otherwise, include all frames. */) +Only frames on the same terminal as FRAME are included in the list +of candidate frames. FRAME defaults to the selected frame. + +If MINIFRAME is nil (the default), include all frames except +minibuffer-only frames. + +If MINIFRAME is a window, include only its own frame and any frame now +using that window as the minibuffer. + +If MINIFRAME is `visible', include only visible frames. + +If MINIFRAME is 0, include only visible and iconified frames. + +If MINIFRAME is any other value, include all frames. + +Return FRAME if no suitable previous frame is found. */) (Lisp_Object frame, Lisp_Object miniframe) { if (NILP (frame)) -- 2.39.5