bug#22105: 25.1.50; REGRESSION - `modify-frame-parameters'

bug#22105: 25.1.50; REGRESSION - `modify-frame-parameters'

[Drew Adams]

[-- Attachment #1: Type: text/plain, Size: 6512 bytes --]

I really hope that you can fix this problem.  It is important to the way
I use Emacs.  I'm reporting this from a dev snapshot (master), but the
same recipe exhibits the same problem with (pre)release 25 builds.

Starting with a build of 2014-10-20, my code that thumbifies frames
(shrinks them to thumbnail size, and restores them) has been broken.
There is no problem with release 24.5 (which came out after 2014-10-20,
IIRC, but from a different dev branch) or with earlier releases or
builds.

All builds since then have problems for frame thumbifying, but there has
seemingly been a lot of changes to the frame code, so the broken
behavior has changed in different ways over the past year or so.  For
this reason of instability (dev changes), and because debugging these
problem(s) seems to take me a long time, I've not filed a bug report
about this until now.

I have a recipe to reproduce a problem, but I don't know whether it is
the only problem.  In general, thumbifying not only does not resize
frames properly, it even changes - if I thumbify & dethumbify the same
frame repeatedly - the thumb size can change a lot.

But such wild changes are not what you will see with this test recipe.
The recipe here reports a single problem that is easy to see.

[This is not the same as bug #14032, which was fixed, though I see now
(after long hours of debugging) that the description seems a bit
similar.  In particular, suppressing the use of scroll bars does not
help.]


To reproduce the problem, use the attached file, test-thumb.el.  Start
with: runemacs.exe -Q --debug-init -l "/PATH/TO/test-thumb.el"

You will use `C-z' to thumbify and dethumbify the *scratch* frame (it's
a toggle).  There is a call to `debug' in `thumfr-thumbify-frame', which
thumbifies.  To see the problem quickly you can comment that out.

The intended behavior (and the behavior in Emacs release 24.5 and prior)
is that once a frame has been thumbified, thumbifying it again, after
dethumbifying, should show it in the same way - its frame parameters
should be restored to those it had when first thumbified.  And this,
regardless of whether the dethumbified frame has changed parameter
values.  IOW, the shape etc. of a thumbnail frame should be determined
once and for all the first time the frame is thumbified.

The behavior instead is that thumbifying results in mirroring the latest
unthumbified frame parameters (with only the `font' parameter changed).
At least that's what I see with the recipe, which shows this problem for
parameter `height'.

Essentially the recipe is this:

1. Thumbify, then dethumbify, to see the expected behavior (can be
   repeated).

2. Then when unthumbified, manually resize the frame, to reduce its
   height noticeably.  Then thumbify.  The thumbnail frame should be the
   same size (e.g., same height) as before, but instead it mirrors the
   aspet ratio of the frame before thumbifying (reduced height).

Below is the detailed recipe.  Note that if you comment-out the call to
`(debug)', or if you just use `c' when the debugger opens, then you can
see the problem - the call to `modify-frame-parameters' after `(debug)'
seems to do nothing.

If instead you use `d' to step through the debugger, so that the first
sexp (the call to `modify-frame-parameters) is stepped through, then
there is no problem - the frame gets the proper size because of that
call.  It is as if the call to `modify-frame-parameters' is ignored
(or optimized away?), unless the debugger steps through it.


RECIPE 1:

1. runemacs.exe -Q --debug-init -l "/PATH/TO/test-thumb.el"

2. Optional, to see frame parameters using `C-o' (command `foo', at end
   of file): `C-x 5 b *Messages*'.

3. With buffer *scratch* selected (focused): `C-z', to thumbify.
   Then `c' to skip through the debugger.

4. `C-z', to dethumbify.

5. Repeat #3 and #4 as many times as you like.  No problem, so far.

6. With the frame normal size (dethumbified), manually (e.g., with
   the mouse) resize it to reduce the height noticeably.

7. `C-z', to thumbify.  Use `c' to skip through the debugger.

You will see that the thumbified height (e.g. aspect ratio) reflects
what the unthumbified height had.  This is the bug.  The thumbnail
should be the same as before - its height should not change.

The call to `enlarge-font' just before that `modify-frame-parameters'
shrinks the frame proportionately, so after manually reducing the
height, the result is a similarly reduced-height thumbnail.  But the
`modify-frame-parameters' that follows `enlarge-font' should correct
this by restoring the set of frame parameters that were recorded from
the first thumbifying.  (They are recorded in frame parameter
`thumfr-non-thumbnail-frame'.)


RECIPE 2: Same as RECIPE 1, but this time, at step 7, step through the
debugger.  You will see that there is no bug - the thumbified frame has
the right height.  The code is correct, I think.  But that call to
`modify-frame-parameters' does not seem to do what it should, unless you
step through it with the debugger.

The behavior without using the debugger at all (e.g., with `(debug)'
commented out) shows the bug too.  The call to `modify-parameters' seems
to be optimized away or otherwise neglects to apply the parameter
changes.  The height parameter (as an example) is left as it was after
the call to `enlarge-font'.

Here is perhaps a clue to the problem: Variable `thumfr-frame-parameters'
has value ((menu-bar-lines . 0)).  If you comment-out that line, to
give it a value of (()), then there is no bug.  You can also uncomment
the commented-out lines, to give it a value of, say,
((tool-bar-lines . 0) (scroll-bar-width . 6) (scroll-bar-height . 6)),
and then there is no bug either.  It seems that it is the presence of
the `menu-bar-lines' sexp that is problematic.

I think this bug was introduced on or before 2014-10-20.  At least
that's when I started seeing similar problems.  Thanks for taking
a look.


In GNU Emacs 25.1.50.1 (i686-pc-mingw32)
 of 2015-12-04
Bzr revision: ffefb6e899fbcdcbd79cb34292d57b7bc3043fcc
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --prefix=/c/Devel/emacs/snapshot/trunk --enable-checking=yes
 --enable-check-lisp-object-type --without-compress-install 'CFLAGS=-Og
 -ggdb3' LDFLAGS=-Lc:/Devel/emacs/lib 'CPPFLAGS=-DGC_MCHECK=1
 -Ic:/Devel/emacs/include''

[-- Attachment #2: test-thumb.el --]
[-- Type: application/octet-stream, Size: 7638 bytes --]

;; RECIPE 1:

;; 1. runemacs.exe -Q --debug-init -l "/PATH/TO/test-thumb.el"

;; 2. Optional, to check frame parameters, using `foo' (at end of file):
;;    C-x 5 b *Messages

;; 3. With buffer *scratch* selected: C-z  ; To thumbify.  Then `c' to skip through the debugger.

;; 4. C-z  ; To dethumbify

;; 5. Repeat #3 and #4, if you like.

;; 6. With the frame normal size (dethumbified), manually (e.g., with the
;;    mouse) resize it to reduce the height.

;; 7. C-z  ; To thumbify.  Use `c' to skip through the debugger.

;; You will see that the thumbified height (e.g. aspect ratio) reflects
;; what the unthumbified height was.  This is the bug.  The thumbnail
;; should be the same as before - its height should not change.

;; RECIPE 2: Same as recipe 1, but this time, at step 7, step through the
;; debugger.  You will see that there is no bug - the thumbified frame
;; has the right height.

;; The behavior without using the debugger at all shows the bug.  The
;; call to `modify-parameters' seems to be optimized away or otherwise
;; neglects to apply the parameter changes.  The height parameter (as
;; an example) is left as it was after the call to `enlarge-font'.

;; I think this bug was introduced on or before 2014-10-20.  At least
;; that's when I started seeing similar problems.



(defun enlarge-font (&optional increment frame)
  ""
  (interactive "p")
  (setq frame  (or frame  (selected-frame)))
  (let ((fontname  (cdr (assq 'font (frame-parameters frame))))
        (count     100))
    (setq fontname  (frcmds-enlarged-font-name fontname frame increment))
    (while (and (not (x-list-fonts fontname))  (wholenump (setq count  (1- count))))
      (setq fontname  (frcmds-enlarged-font-name fontname frame increment)))
    (unless (x-list-fonts fontname) (error "Cannot change font size"))
    (modify-frame-parameters frame (list (cons 'font fontname)))))

(defun frcmds-enlarged-font-name (fontname frame increment)
  ""
  (when (query-fontset fontname)
    (let ((ascii  (assq 'ascii (aref (fontset-info fontname frame) 2))))
      (when ascii (setq fontname  (nth 2 ascii)))))
  (let ((xlfd-fields  (x-decompose-font-name fontname)))
    (unless xlfd-fields (error "Cannot decompose font name"))
    (let ((new-size  (+ (string-to-number (aref xlfd-fields xlfd-regexp-pixelsize-subnum))
                        increment)))
      (unless (> new-size 0) (signal 'font-too-small (list new-size)))
      (aset xlfd-fields xlfd-regexp-pixelsize-subnum (number-to-string new-size)))
    ;; Set point size & width to "*", so frame width will adjust to new font size
    (aset xlfd-fields xlfd-regexp-pointsize-subnum "*")
    (aset xlfd-fields xlfd-regexp-avgwidth-subnum "*")
    (x-compose-font-name xlfd-fields)))

(defvar thumfr-font-difference 8 "")

(defun thumfr--frame-parameters-:set-function (sym defs)
  ""
  (custom-set-default sym defs)
  (dolist (frm  (frame-list))
    (when (and (frame-live-p frm)  (frame-parameter frm 'thumfr-thumbnail-frame))
      (modify-frame-parameters frm thumfr-frame-parameters))))

(defcustom thumfr-frame-parameters
  '(
    (menu-bar-lines . 0) ; <======================
;;;;    (tool-bar-lines . 0)
;;;     (scroll-bar-width . 6)
;;;     (scroll-bar-height . 6)
    )
  ""
  :type '(repeat (cons symbol sexp))
  :group 'convenience
  :set 'thumfr--frame-parameters-:set-function)

(defun thumfr-thumfr-parameter-p (parameter+value)
  ""
  (memq (car parameter+value) '(;; thumfr-is-a-thumbnail-frame
                                thumfr-thumbnail-frame
                                thumfr-non-thumbnail-frame)))

(defun thumfr-thumbnail-frame-p (&optional frame)
  ""
  (interactive)
  (frame-parameter frame 'thumfr-is-a-thumbnail-frame))

(defun thumfr-toggle-thumbnail-frame (&optional frame)
  ""
  (interactive)
  (setq frame  (or frame  (selected-frame)))
  (if (thumfr-thumbnail-frame-p frame)
      (thumfr-dethumbify-frame frame)
    (thumfr-thumbify-frame frame)))

(defun thumfr-remove-if (pred xs)
  ""
  (let ((result  ()))
    (dolist (x  xs)  (unless (funcall pred x) (push x result)))
    (nreverse result)))

(defun thumfr-thumbify-frame (&optional frame)
  ""
  (interactive)
  (message "TH 1, THUMBNAIL-P: %S" (thumfr-thumbnail-frame-p))
  (setq frame  (or frame  (selected-frame)))
  (let* ((fr-params  (frame-parameters frame))
         (to-thumb   (frame-parameter frame 'thumfr-non-thumbnail-frame))
         (other      (thumfr-remove-if #'thumfr-thumfr-parameter-p fr-params)))

    (unless (thumfr-thumbnail-frame-p)
      (set-frame-parameter frame 'thumfr-thumbnail-frame other)
      (condition-case thumfr-thumbify-frame
          (progn
            (enlarge-font (- thumfr-font-difference) frame)
            (debug) ; @@@@@ Comment out `(debug)', or use `c' in debugger, to see the problem.
            ;; @@@@@@ THIS CALL TO `modify-frame-parameters' does not work (does nothing).
            (when to-thumb (modify-frame-parameters frame to-thumb))
            (modify-frame-parameters frame thumfr-frame-parameters)
            (set-frame-parameter frame 'thumfr-is-a-thumbnail-frame t)) ; SUCCESSFUL - thumbified
        (font-too-small                 ; Try again, with a larger font.
         (set-frame-parameter frame 'thumfr-non-thumbnail-frame to-thumb)
         (set-frame-parameter frame 'thumfr-is-a-thumbnail-frame nil)
         (unless (> thumfr-font-difference 0)
           (error (error-message-string thumfr-thumbify-frame)))
         (let ((thumfr-font-difference  (1- thumfr-font-difference)))
           (thumfr-thumbify-frame frame)))
        (error
         (set-frame-parameter frame 'thumfr-non-thumbnail-frame to-thumb)
         (set-frame-parameter frame 'thumfr-is-a-thumbnail-frame nil)
         (error (error-message-string thumfr-thumbify-frame))))))
  (message "TH 2, THUMBNAIL-P: %S" (thumfr-thumbnail-frame-p)))

(defun thumfr-dethumbify-frame (&optional frame)
  "Restore thumbnail FRAME to original size (default: selected frame)."
  (interactive)
  (message "DE 1, THUMBNAIL-P: %S" (thumfr-thumbnail-frame-p))
  (setq frame  (or frame  (selected-frame)))
  (let* ((fr-params  (frame-parameters frame))
         (to-normal  (frame-parameter frame 'thumfr-thumbnail-frame))
         (other      (thumfr-remove-if #'thumfr-thumfr-parameter-p fr-params)))
    (when (thumfr-thumbnail-frame-p)    ; No-op if not a thumbnail.
      (set-frame-parameter frame 'thumfr-non-thumbnail-frame other) ; ????
      (condition-case thumfr-dethumbify-frame
          (progn
            (enlarge-font thumfr-font-difference frame) ; In `frame-cmds.el'.
            (modify-frame-parameters frame to-normal)
            (set-frame-parameter frame 'thumfr-is-a-thumbnail-frame nil)) ; SUCCESSFUL - dethumbified
        (error
         (set-frame-parameter frame 'thumfr-thumbnail-frame to-normal)
         (set-frame-parameter frame 'thumfr-is-a-thumbnail-frame t)
         (error (error-message-string thumfr-dethumbify-frame))))
      (select-frame-set-input-focus frame)))
  (message "DE 2, THUMBNAIL-P: %S" (thumfr-thumbnail-frame-p)))


;; This setting didn't help.  I tried it, hoping that perhaps this bug was related to bug #14032.
;; (setq default-frame-alist '((vertical-scroll-bars) (horizontal-scroll-bars)
;;                             (scroll-bar-height . 0) (scroll-bar-width . 0)))


;; Misc. debug help.
(setq pop-up-frames t) ; To see *Messages*
(global-set-key "\C-z" 'thumfr-toggle-thumbnail-frame)
(defvar foo nil "")
(setq pop-up-frames t)
(defun foo ()
  (interactive)
  (setq foo (frame-parameters))
  (describe-variable 'foo))
(global-set-key "\C-o" 'foo)

(provide 'test-thumb)

bug#22105: 25.1.50; REGRESSION - `modify-frame-parameters'

[martin rudalics]

 > Here is perhaps a clue to the problem: Variable `thumfr-frame-parameters'
 > has value ((menu-bar-lines . 0)).  If you comment-out that line, to
 > give it a value of (()), then there is no bug.  You can also uncomment
 > the commented-out lines, to give it a value of, say,
 > ((tool-bar-lines . 0) (scroll-bar-width . 6) (scroll-bar-height . 6)),
 > and then there is no bug either.  It seems that it is the presence of
 > the `menu-bar-lines' sexp that is problematic.

I suppose the behavior is indeed caused by adding and removing the menu
bar.  At least when doing C-z repeatedly with the "(debug)" commented
out I see here both small and normal font frames continuously increase
in size.

You could try with

(setq frame-inhibit-implied-resize '(menu-bar-lines tool-bar-lines))

which inherently means to not resize the outer frame when adding or
removing the menu bar.  It seems to cure the problem I described above.

martin

bug#22105: 25.1.50; REGRESSION - `modify-frame-parameters'

[Drew Adams]

> I suppose the behavior is indeed caused by adding and removing the menu
> bar.  At least when doing C-z repeatedly with the "(debug)" commented
> out I see here both small and normal font frames continuously increase
> in size.
> 
> You could try with
> (setq frame-inhibit-implied-resize '(menu-bar-lines tool-bar-lines))
> which inherently means to not resize the outer frame when adding or
> removing the menu bar.  It seems to cure the problem I described above.

Thanks for the quick reply.

Yes, that does seem to take care of this problem, but I will take a
closer look when I get a chance.  In any case, other woes introduced
on 2015-10-20 are not fixed by setting that option - I'll have to find
some time to track those down.

But this is a user option.  Even if changing the value is a "fix"
for this, I'm not sure how I should approach this wrt a library that
lets users thumbify frames and decide which frame parameter values
they want to impose on the thumbnail frames.  I guess I could change
the option value on the fly, if `menu-bar-lines' is changed to or
from 0).  But that seems ugly and awkward.

I see that the default value of this option gives you the same
behavior as you get in Emacs 24.5 (or earlier).  E.g., toggling
menu-bar-mode changes the height, but toggling tool-bar-mode does
not change the height.

But that compatibility does not apply to my thumbification.  That
still seems like a bug.

My code restores a set of frame parameters, but that behavior is
broken, it seems, because that particular call to
`modify-frame-parameters' has _no effect_.

It's not just that the frame height is changed by the height of
a menu-bar or not, depending on the option value.

The height is not changed _at all_ by that call, if I don't tweak
the option, and it should be changed a _lot_ (assuming you resized
the unthumbified frame to reduce the height a lot).

The `height' parameter that is being restored is apparently ignored.
My guess is that all of the parameters are being ignored - that
particular call to `modify-frame-parameters' appears to be a no-op.
That doesn't seem to follow what the option should do, regardless
of the option value.

And if you step through the debugger then it _has_ its intended
effect - the `modify-frame-parameters' call is not ignored - it
restores all of the parameters passed to it (including `height').

So I cannot think that this bug is fixed (or not a bug), but I
don't really understand all that is involved, and I will play
some more with that user option - when I get a chance.

I hope that you will investigate, following my recipe, why
that `modify-frame-parameters' call now has no effect unless
you step through the debugger.  But I can at least confirm that
the problem does indeed seem to be brought on by the code that
implements that option.

Thanks for looking into this, Martin.

bug#22105: 25.1.50; REGRESSION - `modify-frame-parameters'

[martin rudalics]

 > My code restores a set of frame parameters, but that behavior is
 > broken, it seems, because that particular call to
 > `modify-frame-parameters' has _no effect_.

What are the arguments of "that particular call"?

 > It's not just that the frame height is changed by the height of
 > a menu-bar or not, depending on the option value.
 >
 > The height is not changed _at all_ by that call, if I don't tweak
 > the option, and it should be changed a _lot_ (assuming you resized
 > the unthumbified frame to reduce the height a lot).

When I tried your functions the height changed considerably here.  But
every large frame was higher than its predecessor by the height of the
menu bar.

 > The `height' parameter that is being restored is apparently ignored.
 > My guess is that all of the parameters are being ignored - that
 > particular call to `modify-frame-parameters' appears to be a no-op.
 > That doesn't seem to follow what the option should do, regardless
 > of the option value.

Are your sure that all arguments are ignored?

 > And if you step through the debugger then it _has_ its intended
 > effect - the `modify-frame-parameters' call is not ignored - it
 > restores all of the parameters passed to it (including `height').

That's an interesting aspect.  Maybe you could try to simulate the
debugger's behavior by inserting a ‘sit-for’ wherever it produces the
intended effect.  Then we could "nail down" the critical section that
produces the unintended behavior.

 > So I cannot think that this bug is fixed (or not a bug), but I
 > don't really understand all that is involved, and I will play
 > some more with that user option - when I get a chance.
 >
 > I hope that you will investigate, following my recipe, why
 > that `modify-frame-parameters' call now has no effect unless
 > you step through the debugger.  But I can at least confirm that
 > the problem does indeed seem to be brought on by the code that
 > implements that option.

I don't think the option itself is related to the problem you see.  It's
rather the behavior when the option is not chosen.

martin

bug#22105: 25.1.50; REGRESSION - `modify-frame-parameters'

[martin rudalics]

 > I don't think the option itself is related to the problem you see.  It's
 > rather the behavior when the option is not chosen.

IIUC the problem I see can be described more easily as follows.
Evaluate in sequence the following expressions with emacs -Q:

(modify-frame-parameters nil '((menu-bar-lines . 0) (height . 30)))
(modify-frame-parameters nil '((menu-bar-lines . 1) (height . 40)))
(frame-parameter nil 'height)

Here the height reported by the last form is 41.  This seems bug#1348
again.  Until that gets fixed we can expect it to hit us everywhere we
set a frame size and some other parameter that affects the same size.

Your code behaves as intended on GNU/Linux (although my system is slow
enough so I can watch the single resize operations).  Maybe you should
change the OS.  Otherwise, never mix two parameters in one and the same
‘modify-frame-parameters’ call that may affect the same size parameter.
Or, if they miraculously work with some older Emacs version on Windows,
then stick to that version.

martin

bug#22105: 25.1.50; REGRESSION - `modify-frame-parameters'

[martin rudalics]

[-- Attachment #1: Type: text/plain, Size: 2110 bytes --]

 > IIUC the problem I see can be described more easily as follows.
 > Evaluate in sequence the following expressions with emacs -Q:
 >
 > (modify-frame-parameters nil '((menu-bar-lines . 0) (height . 30)))
 > (modify-frame-parameters nil '((menu-bar-lines . 1) (height . 40)))
 > (frame-parameter nil 'height)
 >
 > Here the height reported by the last form is 41.

Actually, the immediate cause of this problem goes as follows: When you
specify both addition of the menu bar and the new frame height,
‘modify-frame-parameters’ will first issue a request to add the menu bar
and then a request to calculate the new frame height as specified.

However, Emacs does not immediately add the menu bar but delay this
until prepare_menu_bars is run by the display engine.  Till then, only
the value stored in the external_menu_bar slot of the frame tells
whether the frame shall have a menu bar or not.

As a consequence, the request to set the frame height will be delivered
first to the toolkit.  The calculation of the new frame height on
Windows is done by calling the function AdjustWindowRect.  The third
argument of that function has to specify whether a menu bar should be
included in the calculation or not.  Currently, this value is set to
whatever the external_menu_bar slot stores.  But if the frame does not
have a menu bar so far but should get one in the future, the value
stored there is inherently wrong for calculating the size of the frame
height via AdjustWindowRect.

I attach a patch against Emasc-25 which fixes this bug here.  I also
added a remark to the doc-string of ‘modify-frame-parameters’ in the
sense that if you specify several parameters which might affect the size
of the frame, Emacs cannot always guarantee for every toolkit that the
final size will meet the expectations of the caller.  Inherently this
means, that the application programmer has to experiment with this
function on each and every toolkit involved.

I leave it to Eli to decide whether the patch should go into Emacs-25 or
master/trunk.

Thanks for your attention, martin

[-- Attachment #2: add-menu-bar.diff --]
[-- Type: text/plain, Size: 5762 bytes --]

diff --git a/doc/lispref/frames.texi b/doc/lispref/frames.texi
index 3c1a87a..0a43929 100644
--- a/doc/lispref/frames.texi
+++ b/doc/lispref/frames.texi
@@ -1007,12 +1007,28 @@ Parameter Access
 parameter.  If you don't mention a parameter in @var{alist}, its value
 doesn't change.  If @var{frame} is @code{nil}, it defaults to the selected
 frame.
+
+When @var{alist} specifies more than one parameter whose value can
+affect the new size of @var{frame}, the final size of the frame may
+differ according to the toolkit used.  For example, specifying that a
+frame should from now on have a menu and/or tool bar instead of none and
+simultaneously specifying the new height of the frame will inevitably
+lead to a recalculation of the frame's height.  Conceptually, in such
+case, this function will try to have the explicit height specification
+prevail.  It cannot be excluded, however, that the addition (or removal)
+of the menu or tool bar, when eventually performed by the toolkit, will
+defeat this intention.
+
+Sometimes, binding @code{frame-inhibit-implied-resize} (@pxref{Implied
+Frame Resizing}) to a non-@code{nil} value around calls to this function
+may fix the problem sketched here.  Sometimes, however, exactly such
+binding may be hit by the problem.
 @end defun

 @defun set-frame-parameter frame parm value
 This function sets the frame parameter @var{parm} to the specified
-@var{value}.  If @var{frame} is @code{nil}, it defaults to the
-selected frame.
+@var{value}.  If @var{frame} is @code{nil}, it defaults to the selected
+frame.
 @end defun

 @defun modify-all-frames-parameters alist
diff --git a/src/w32fns.c b/src/w32fns.c
index 208c980..f9ce762 100644
--- a/src/w32fns.c
+++ b/src/w32fns.c
@@ -1666,10 +1666,7 @@ x_set_menu_bar_lines (struct frame *f, Lisp_Object value, Lisp_Object oldval)
   FRAME_MENU_BAR_LINES (f) = 0;
   FRAME_MENU_BAR_HEIGHT (f) = 0;
   if (nlines)
-    {
-      FRAME_EXTERNAL_MENU_BAR (f) = 1;
-      windows_or_buffers_changed = 23;
-    }
+    FRAME_EXTERNAL_MENU_BAR (f) = 1;
   else
     {
       if (FRAME_EXTERNAL_MENU_BAR (f) == 1)
@@ -4620,8 +4617,7 @@ my_create_tip_window (struct frame *f)
   rect.right = FRAME_PIXEL_WIDTH (f);
   rect.bottom = FRAME_PIXEL_HEIGHT (f);

-  AdjustWindowRect (&rect, f->output_data.w32->dwStyle,
-		    FRAME_EXTERNAL_MENU_BAR (f));
+  AdjustWindowRect (&rect, f->output_data.w32->dwStyle, false);

   tip_window = FRAME_W32_WINDOW (f)
     = CreateWindow (EMACS_CLASS,
@@ -6681,8 +6677,7 @@ Text larger than the specified size is clipped.  */)
     rect.left = rect.top = 0;
     rect.right = width;
     rect.bottom = height;
-    AdjustWindowRect (&rect, f->output_data.w32->dwStyle,
-		      FRAME_EXTERNAL_MENU_BAR (f));
+    AdjustWindowRect (&rect, f->output_data.w32->dwStyle, false);

     /* Position and size tooltip, and put it in the topmost group.
        The add-on of FRAME_COLUMN_WIDTH to the 5th argument is a
diff --git a/src/w32menu.c b/src/w32menu.c
index 6af69f4..964b965 100644
--- a/src/w32menu.c
+++ b/src/w32menu.c
@@ -494,7 +494,10 @@ set_frame_menubar (struct frame *f, bool first_time, bool deep_p)
     /* Force the window size to be recomputed so that the frame's text
        area remains the same, if menubar has just been created.  */
     if (old_widget == NULL)
-      adjust_frame_size (f, -1, -1, 2, false, Qmenu_bar_lines);
+      {
+	windows_or_buffers_changed = 23;
+	adjust_frame_size (f, -1, -1, 2, false, Qmenu_bar_lines);
+      }
   }

   unblock_input ();
diff --git a/src/w32term.c b/src/w32term.c
index f48e725..0b8bef2 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -6115,9 +6115,22 @@ x_set_window_size (struct frame *f, bool change_gravity,
   int pixelwidth, pixelheight;
   Lisp_Object fullscreen = get_frame_param (f, Qfullscreen);
   RECT rect;
+  MENUBARINFO info;
+  int menu_bar_height;

   block_input ();

+  /* Get the height of the menu bar here.  It's used below to detect
+     whether the menu bar is wrapped.  It's also used to specify the
+     third argument for AdjustWindowRect.  FRAME_EXTERNAL_MENU_BAR which
+     has been used before for that reason is unreliable because it only
+     specifies whether we _want_ a menu bar for this frame and not
+     whether this frame _has_ a menu bar.  See bug#22105.  */
+  info.cbSize = sizeof (info);
+  info.rcBar.top = info.rcBar.bottom = 0;
+  GetMenuBarInfo (FRAME_W32_WINDOW (f), 0xFFFFFFFD, 0, &info);
+  menu_bar_height = info.rcBar.bottom - info.rcBar.top;
+
   if (pixelwise)
     {
       pixelwidth = FRAME_TEXT_TO_PIXEL_WIDTH (f, width);
@@ -6135,17 +6148,11 @@ x_set_window_size (struct frame *f, bool change_gravity,
 	 height of the frame then the wrapped menu bar lines are not
 	 accounted for (Bug#15174 and Bug#18720).  Here we add these
 	 extra lines to the frame height.  */
-      MENUBARINFO info;
       int default_menu_bar_height;
-      int menu_bar_height;

       /* Why is (apparently) SM_CYMENUSIZE needed here instead of
 	 SM_CYMENU ??  */
       default_menu_bar_height = GetSystemMetrics (SM_CYMENUSIZE);
-      info.cbSize = sizeof (info);
-      info.rcBar.top = info.rcBar.bottom = 0;
-      GetMenuBarInfo (FRAME_W32_WINDOW (f), 0xFFFFFFFD, 0, &info);
-      menu_bar_height = info.rcBar.bottom - info.rcBar.top;

       if ((default_menu_bar_height > 0)
 	  && (menu_bar_height > default_menu_bar_height)
@@ -6160,8 +6167,7 @@ x_set_window_size (struct frame *f, bool change_gravity,
   rect.right = pixelwidth;
   rect.bottom = pixelheight;

-  AdjustWindowRect (&rect, f->output_data.w32->dwStyle,
-		    FRAME_EXTERNAL_MENU_BAR (f));
+  AdjustWindowRect (&rect, f->output_data.w32->dwStyle, menu_bar_height > 0);

   if (!(f->after_make_frame)
       && !(f->want_fullscreen & FULLSCREEN_WAIT)

bug#22105: 25.1.50; REGRESSION - `modify-frame-parameters'

[Eli Zaretskii]

> Date: Sat, 12 Dec 2015 11:44:15 +0100
> From: martin rudalics <rudalics@gmx.at>
> 
> I attach a patch against Emasc-25 which fixes this bug here.  I also
> added a remark to the doc-string of ‘modify-frame-parameters’ in the
> sense that if you specify several parameters which might affect the size
> of the frame, Emacs cannot always guarantee for every toolkit that the
> final size will meet the expectations of the caller.  Inherently this
> means, that the application programmer has to experiment with this
> function on each and every toolkit involved.
> 
> I leave it to Eli to decide whether the patch should go into Emacs-25 or
> master/trunk.

Please commit to emacs-25, and thanks.

bug#22105: 25.1.50; REGRESSION - `modify-frame-parameters'

[martin rudalics]

 > Please commit to emacs-25, and thanks.

Done.  Drew, please check and tell me which are the remaining issues
also wrt bug#16028 and bug#16923.

martin

bug#22105: 25.1.50; REGRESSION - `modify-frame-parameters'

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

>> Please commit to emacs-25, and thanks.
>
> Done.  Drew, please check and tell me which are the remaining issues
> also wrt bug#16028 and bug#16923.

Drew, did this fix your problems here?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#22105: 25.1.50; REGRESSION - `modify-frame-parameters'

[Drew Adams]

> > Done.  Drew, please check and tell me which are the remaining issues
> > also wrt bug#16028 and bug#16923.
> 
> Drew, did this fix your problems here?

No, this is still broken - since October 2015.  I have tried
a few times to track this down (spent quite a while on it),
but I have not had a block of time to do it successfully.

It is also the case that the behavior, although still broken,
has changed several times since then - quite different
breakages, due to changes in the Emacs code (I have not
changed my code).

That is another reason that I have not found the time or
energy to track this down completely: it kept changing,
as other users reported problems that were similar or 
(I assume) related.

I still hope to get to the bottom of this.  And it seems
that the changes of the Emacs code have quieted down now.

bug#22105: 25.1.50; REGRESSION - `modify-frame-parameters'

[Stefan Kangas]

Drew Adams <drew.adams@oracle.com> writes:

>> > Done.  Drew, please check and tell me which are the remaining issues
>> > also wrt bug#16028 and bug#16923.
>>
>> Drew, did this fix your problems here?
>
> No, this is still broken - since October 2015.  I have tried
> a few times to track this down (spent quite a while on it),
> but I have not had a block of time to do it successfully.
>
> It is also the case that the behavior, although still broken,
> has changed several times since then - quite different
> breakages, due to changes in the Emacs code (I have not
> changed my code).
>
> That is another reason that I have not found the time or
> energy to track this down completely: it kept changing,
> as other users reported problems that were similar or
> (I assume) related.
>
> I still hope to get to the bottom of this.  And it seems
> that the changes of the Emacs code have quieted down now.

That was 3 years ago, so this is just a friendly reminder.

Did you ever get a chance to track this down?

Thanks,
Stefan Kangas

bug#22105: 25.1.50; REGRESSION - `modify-frame-parameters'

[Drew Adams]

> >> Drew, did this fix your problems here?
> >
> > No, this is still broken - since October 2015.  I have tried
> > a few times to track this down (spent quite a while on it),
> > but I have not had a block of time to do it successfully.
> >
> > It is also the case that the behavior, although still broken,
> > has changed several times since then - quite different
> > breakages, due to changes in the Emacs code (I have not
> > changed my code).
> >
> > That is another reason that I have not found the time or
> > energy to track this down completely: it kept changing,
> > as other users reported problems that were similar or
> > (I assume) related.
> >
> > I still hope to get to the bottom of this.  And it seems
> > that the changes of the Emacs code have quieted down now.
> 
> That was 3 years ago, so this is just a friendly reminder.
> Did you ever get a chance to track this down?

No, I haven't had the time.

I just went through the recipe again, and I see the problem.
It's 100% reproducible (I'm using Windows 10).

It would be great if someone knowledgeable about the C
code (e.g. `modify-frame-parameters') would follow the
recipe on Windows and track down what needs to be done.

The bug report identifies approximately when the
regression was introduced.  It would really be great
to have this fixed, restoring pre-regression behavior.

bug#22105: 25.1.50; REGRESSION - `modify-frame-parameters'

[Lars Ingebrigtsen]

Drew Adams <drew.adams@oracle.com> writes:

> I just went through the recipe again, and I see the problem.
> It's 100% reproducible (I'm using Windows 10).

As a reminder, the bug is that after doing:

(modify-frame-parameters nil '((menu-bar-lines . 0) (height . 30)))
(modify-frame-parameters nil '((menu-bar-lines . 1) (height . 40)))
(frame-parameter nil 'height)

The final form returns 41 (on Windows).

I've tried this on Debian, and I get 40 (as expected), so this seems
like a Windows-specific bug.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#22105: 25.1.50; REGRESSION - `modify-frame-parameters'

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Stefan Kangas <stefan@marxist.se>,  martin rudalics <rudalics@gmx.at>,
>   Eli Zaretskii <eliz@gnu.org>,  22105@debbugs.gnu.org
> Date: Mon, 07 Sep 2020 18:28:36 +0200
> 
> As a reminder, the bug is that after doing:
> 
> (modify-frame-parameters nil '((menu-bar-lines . 0) (height . 30)))
> (modify-frame-parameters nil '((menu-bar-lines . 1) (height . 40)))
> (frame-parameter nil 'height)
> 
> The final form returns 41 (on Windows).
> 
> I've tried this on Debian, and I get 40 (as expected), so this seems
> like a Windows-specific bug.

I also get 40 (both on master and in Emacs 27.1), so I guess this make
this an unreproducible bug...

bug#22105: 25.1.50; REGRESSION - `modify-frame-parameters'

[Drew Adams]

> > As a reminder, the bug is that after doing:
> >
> > (modify-frame-parameters nil '((menu-bar-lines . 0) (height . 30)))
> > (modify-frame-parameters nil '((menu-bar-lines . 1) (height . 40)))
> > (frame-parameter nil 'height)
> >
> > The final form returns 41 (on Windows).
> >
> > I've tried this on Debian, and I get 40 (as expected), so this seems
> > like a Windows-specific bug.
> 
> I also get 40 (both on master and in Emacs 27.1), so I guess this make
> this an unreproducible bug...

Thanks to all, especially Martin, for whatever fixed this.

With emacs -Q, I can see that this is broken for Emacs 24.5
and fixed starting with Emacs 25.3.1.

Thx.  I've closed this bug.