bug#41338: Toolbar-bug in Emacs 27.0.91/Pretest

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#41338: Toolbar-bug in Emacs 27.0.91/Pretest

Stephen Berman
On Sat, 16 May 2020 22:43:25 +0200 Konrad Podczeck <[hidden email]> wrote:

> To reproduce:
>
> (1) Start Emacs (without any customizations).
>
> (2) Open a file which has the standard toolbar, say mouse.el
>
> (3) Do C-x m, to open a message buffer; this buffer has a toolbar _different_
> from the standard one.
>
> (4) Do C-x 5 2, to get the message buffer shown in a second frame.
>
> (5) In the first frame, make the buffer showing mouse.el active again.
>
> (6) In that buffer, do C-s, to invoke isearch and search for some word.
>
> (7) Using the mouse, close the frame showing mouse.el, but do so in a state
> where still isearch overlays show up.
>
> (8) Now click with mouse-1 in the remaining frame, i.e., in that showing the
> message buffer, and the toolbar changes to the standard one.
>
> I consider this as a bug. In case it is not platform independent, I us an NS-build.

I see this too, on GNU/Linux (both in 27.0.91 and a recent build from
master).  Moreover, if in the remaining frame I switch from the message
buffer to the buffer in which isearch was invoked, the tool bar is now
the isearch tool bar and the mode line has the "Isearch" lighter, but
there are no isearch overlays and point is where it was before invoking
isearch.  This seems surprising.  But now typing C-s restores the
isearch state the buffer had (i.e. the same overlays) before closing the
other frame.

Steve Berman



Reply | Threaded
Open this post in threaded view
|

bug#41338: Toolbar-bug in Emacs 27.0.91/Pretest

Juri Linkov-2
>> To reproduce:
>>
>> (1) Start Emacs (without any customizations).
>>
>> (2) Open a file which has the standard toolbar, say mouse.el
>>
>> (3) Do C-x m, to open a message buffer; this buffer has a toolbar _different_
>> from the standard one.
>>
>> (4) Do C-x 5 2, to get the message buffer shown in a second frame.
>>
>> (5) In the first frame, make the buffer showing mouse.el active again.
>>
>> (6) In that buffer, do C-s, to invoke isearch and search for some word.
>>
>> (7) Using the mouse, close the frame showing mouse.el, but do so in a state
>> where still isearch overlays show up.
>>
>> (8) Now click with mouse-1 in the remaining frame, i.e., in that showing the
>> message buffer, and the toolbar changes to the standard one.
>>
>> I consider this as a bug. In case it is not platform independent, I us an NS-build.
>
> I see this too, on GNU/Linux (both in 27.0.91 and a recent build from
> master).  Moreover, if in the remaining frame I switch from the message
> buffer to the buffer in which isearch was invoked, the tool bar is now
> the isearch tool bar and the mode line has the "Isearch" lighter, but
> there are no isearch overlays and point is where it was before invoking
> isearch.  This seems surprising.  But now typing C-s restores the
> isearch state the buffer had (i.e. the same overlays) before closing the
> other frame.

isearch-mode-map has such bindings:

    ;; Pass frame events transparently so they won't exit the search.
    ;; In particular, if we have more than one display open, then a
    ;; switch-frame might be generated by someone typing at another keyboard.
    (define-key map [switch-frame] nil)
    (define-key map [delete-frame] nil)

and indeed the ‘switch-frame’ event is fired when the frame is switched
during isearch, and it exits isearch.

But I don't know why the ‘delete-frame’ event is not fired on frame deletion.

Perhaps isearch.el should explicitly use the hook ‘delete-frame-functions’
to exit isearch.



Reply | Threaded
Open this post in threaded view
|

bug#41338: Toolbar-bug in Emacs 27.0.91/Pretest

martin rudalics
 > But I don't know why the ‘delete-frame’ event is not fired on frame deletion.

How do you delete the frame?  You don't get a 'delete-frame' event when
you delete a frame via C-x 5 0 or C-x 5 1.

martin




Reply | Threaded
Open this post in threaded view
|

bug#41338: Toolbar-bug in Emacs 27.0.91/Pretest

Stephen Berman
On Sun, 17 May 2020 10:20:55 +0200 martin rudalics <[hidden email]> wrote:

>> But I don't know why the ‘delete-frame’ event is not fired on frame deletion.
>
> How do you delete the frame?  You don't get a 'delete-frame' event when
> you delete a frame via C-x 5 0 or C-x 5 1.
>
> martin

In my test I followed the OP's recipe:

> (7) Using the mouse, close the frame showing mouse.el, but do so in a state
> where still isearch overlays show up.

I.e., I clicked on the 'X' (close frame button) in the frame's title
bar.

Steve Berman



Reply | Threaded
Open this post in threaded view
|

bug#41338: Toolbar-bug in Emacs 27.0.91/Pretest

Stephen Berman
On Sun, 17 May 2020 10:56:50 +0200 Stephen Berman <[hidden email]> wrote:

> On Sun, 17 May 2020 10:20:55 +0200 martin rudalics <[hidden email]> wrote:
>
>>> But I don't know why the ‘delete-frame’ event is not fired on frame deletion.
>>
>> How do you delete the frame?  You don't get a 'delete-frame' event when
>> you delete a frame via C-x 5 0 or C-x 5 1.
>>
>> martin
>
> In my test I followed the OP's recipe:
>
>> (7) Using the mouse, close the frame showing mouse.el, but do so in a state
>> where still isearch overlays show up.
>
> I.e., I clicked on the 'X' (close frame button) in the frame's title
> bar.

But when I close the frame with `C-x 5 0' instead and then proceed with
the OP's recipe, the tool bar does not change and when I switch to the
buffer where isearch had been invoked, the search is now abandoned,
i.e., no isearch tool bar or lighter, no overlays.  So only closing the
frame by clicking the close frame button with the mouse induces the
surprising behavior.

Steve Berman



Reply | Threaded
Open this post in threaded view
|

bug#41338: Toolbar-bug in Emacs 27.0.91/Pretest

Eli Zaretskii
On May 17, 2020 12:11:04 PM GMT+03:00, Stephen Berman <[hidden email]> wrote:

> On Sun, 17 May 2020 10:56:50 +0200 Stephen Berman
> <[hidden email]> wrote:
>
> > On Sun, 17 May 2020 10:20:55 +0200 martin rudalics <[hidden email]>
> wrote:
> >
> >>> But I don't know why the ‘delete-frame’ event is not fired on
> frame deletion.
> >>
> >> How do you delete the frame?  You don't get a 'delete-frame' event
> when
> >> you delete a frame via C-x 5 0 or C-x 5 1.
> >>
> >> martin
> >
> > In my test I followed the OP's recipe:
> >
> >> (7) Using the mouse, close the frame showing mouse.el, but do so in
> a state
> >> where still isearch overlays show up.
> >
> > I.e., I clicked on the 'X' (close frame button) in the frame's title
> > bar.
>
> But when I close the frame with `C-x 5 0' instead and then proceed
> with
> the OP's recipe, the tool bar does not change and when I switch to the
> buffer where isearch had been invoked, the search is now abandoned,
> i.e., no isearch tool bar or lighter, no overlays.  So only closing
> the
> frame by clicking the close frame button with the mouse induces the
> surprising behavior.
>
> Steve Berman

Could someone please explain to me what is deemed to be the bug here?  Because I'm confused wrt what is the part of the original description that seems to be the problem.  In particular, the Isearch overlays are per-window, AFAIK.



Reply | Threaded
Open this post in threaded view
|

bug#41338: Toolbar-bug in Emacs 27.0.91/Pretest

Stephen Berman
On Sun, 17 May 2020 15:36:10 +0300 Eli Zaretskii <[hidden email]> wrote:

> On May 17, 2020 12:11:04 PM GMT+03:00, Stephen Berman <[hidden email]> wrote:
>> On Sun, 17 May 2020 10:56:50 +0200 Stephen Berman
>> <[hidden email]> wrote:
>>
>> > On Sun, 17 May 2020 10:20:55 +0200 martin rudalics <[hidden email]>
>> wrote:
>> >
>> >>> But I don't know why the ‘delete-frame’ event is not fired on
>> frame deletion.
>> >>
>> >> How do you delete the frame?  You don't get a 'delete-frame' event
>> when
>> >> you delete a frame via C-x 5 0 or C-x 5 1.
>> >>
>> >> martin
>> >
>> > In my test I followed the OP's recipe:
>> >
>> >> (7) Using the mouse, close the frame showing mouse.el, but do so in
>> a state
>> >> where still isearch overlays show up.
>> >
>> > I.e., I clicked on the 'X' (close frame button) in the frame's title
>> > bar.
>>
>> But when I close the frame with `C-x 5 0' instead and then proceed
>> with
>> the OP's recipe, the tool bar does not change and when I switch to the
>> buffer where isearch had been invoked, the search is now abandoned,
>> i.e., no isearch tool bar or lighter, no overlays.  So only closing
>> the
>> frame by clicking the close frame button with the mouse induces the
>> surprising behavior.
>>
>> Steve Berman
>
> Could someone please explain to me what is deemed to be the bug here?  Because
> I'm confused wrt what is the part of the original description that seems to be
> the problem.  In particular, the Isearch overlays are per-window, AFAIK.

The bug reported in the OP (the unexpected tool bar change) seems to be
triggered by isearch (it was part of the OP's recipe and I haven't been
able to reproduce it without using isearch), but maybe the observation
about isearch I reported in my followup is a separate bug or perhaps no
bug, even though it surprised me.

Steve Berman



Reply | Threaded
Open this post in threaded view
|

bug#41338: Toolbar-bug in Emacs 27.0.91/Pretest

Eli Zaretskii
On May 17, 2020 3:59:55 PM GMT+03:00, Stephen Berman <[hidden email]> wrote:

> On Sun, 17 May 2020 15:36:10 +0300 Eli Zaretskii <[hidden email]> wrote:
>
> > On May 17, 2020 12:11:04 PM GMT+03:00, Stephen Berman
> <[hidden email]> wrote:
> >> On Sun, 17 May 2020 10:56:50 +0200 Stephen Berman
> >> <[hidden email]> wrote:
> >>
> >> > On Sun, 17 May 2020 10:20:55 +0200 martin rudalics
> <[hidden email]>
> >> wrote:
> >> >
> >> >>> But I don't know why the ‘delete-frame’ event is not fired on
> >> frame deletion.
> >> >>
> >> >> How do you delete the frame?  You don't get a 'delete-frame'
> event
> >> when
> >> >> you delete a frame via C-x 5 0 or C-x 5 1.
> >> >>
> >> >> martin
> >> >
> >> > In my test I followed the OP's recipe:
> >> >
> >> >> (7) Using the mouse, close the frame showing mouse.el, but do so
> in
> >> a state
> >> >> where still isearch overlays show up.
> >> >
> >> > I.e., I clicked on the 'X' (close frame button) in the frame's
> title
> >> > bar.
> >>
> >> But when I close the frame with `C-x 5 0' instead and then proceed
> >> with
> >> the OP's recipe, the tool bar does not change and when I switch to
> the
> >> buffer where isearch had been invoked, the search is now abandoned,
> >> i.e., no isearch tool bar or lighter, no overlays.  So only closing
> >> the
> >> frame by clicking the close frame button with the mouse induces the
> >> surprising behavior.
> >>
> >> Steve Berman
> >
> > Could someone please explain to me what is deemed to be the bug
> here?  Because
> > I'm confused wrt what is the part of the original description that
> seems to be
> > the problem.  In particular, the Isearch overlays are per-window,
> AFAIK.
>
> The bug reported in the OP (the unexpected tool bar change) seems to
> be
> triggered by isearch (it was part of the OP's recipe and I haven't
> been
> able to reproduce it without using isearch), but maybe the observation
> about isearch I reported in my followup is a separate bug or perhaps
> no
> bug, even though it surprised me.
>
> Steve Berman

So the problem is that the toolbar is not reconfigured to reflect the fact that we are in Isearch?

Does it make sense to remain in Isearch in tbis scenario?  Could it be that we are over-engineering this stuff?



Reply | Threaded
Open this post in threaded view
|

bug#41338: Toolbar-bug in Emacs 27.0.91/Pretest

Stephen Berman
On Sun, 17 May 2020 16:12:38 +0300 Eli Zaretskii <[hidden email]> wrote:

> On May 17, 2020 3:59:55 PM GMT+03:00, Stephen Berman <[hidden email]> wrote:
>> The bug reported in the OP (the unexpected tool bar change) seems to
>> be triggered by isearch (it was part of the OP's recipe and I haven't
>> been able to reproduce it without using isearch), but maybe the
>> observation about isearch I reported in my followup is a separate bug
>> or perhaps no bug, even though it surprised me.
>>
>> Steve Berman
>
> So the problem is that the toolbar is not reconfigured to reflect the fact
> that we are in Isearch?

In the OP the tool bar change was in the message-mode buffer, not the
buffer where isearch was invoked.  My additional observation was that in
the latter buffer the tool bar and lighter indicated isearch in
progress, but the overlays were missing and point was in its pre-isearch
position.

> Does it make sense to remain in Isearch in tbis scenario?  Could it be
> that we are over-engineering this stuff?

As I reported in my followup to Martin, it is only when deleting the
frame by clicking the toolkit's close frame button that isearch remains
in progress, with `C-x 5 0' it is cancelled.  So the former seems to be
the problematic case, since it's also the method used in the OP.

Steve Berman



Reply | Threaded
Open this post in threaded view
|

bug#41338: Toolbar-bug in Emacs 27.0.91/Pretest

Eli Zaretskii
On May 17, 2020 4:37:12 PM GMT+03:00, Stephen Berman <[hidden email]> wrote:

> On Sun, 17 May 2020 16:12:38 +0300 Eli Zaretskii <[hidden email]> wrote:
>
> > On May 17, 2020 3:59:55 PM GMT+03:00, Stephen Berman
> <[hidden email]> wrote:
> >> The bug reported in the OP (the unexpected tool bar change) seems
> to
> >> be triggered by isearch (it was part of the OP's recipe and I
> haven't
> >> been able to reproduce it without using isearch), but maybe the
> >> observation about isearch I reported in my followup is a separate
> bug
> >> or perhaps no bug, even though it surprised me.
> >>
> >> Steve Berman
> >
> > So the problem is that the toolbar is not reconfigured to reflect
> the fact
> > that we are in Isearch?
>
> In the OP the tool bar change was in the message-mode buffer, not the
> buffer where isearch was invoked.  My additional observation was that
> in
> the latter buffer the tool bar and lighter indicated isearch in
> progress, but the overlays were missing and point was in its
> pre-isearch
> position.
>
> > Does it make sense to remain in Isearch in tbis scenario?  Could it
> be
> > that we are over-engineering this stuff?
>
> As I reported in my followup to Martin, it is only when deleting the
> frame by clicking the toolkit's close frame button that isearch
> remains
> in progress, with `C-x 5 0' it is cancelled.  So the former seems to
> be
> the problematic case, since it's also the method used in the OP.
>
> Steve Berman

Yes, I was asking whether wr should simply not try remaining in Isearch when frames are switched or deleted.  Juri seems to say that we try not to leave Isearch in some of those cases.



Reply | Threaded
Open this post in threaded view
|

bug#41338: Toolbar-bug in Emacs 27.0.91/Pretest

Stephen Berman
On Sun, 17 May 2020 17:22:14 +0300 Eli Zaretskii <[hidden email]> wrote:

> On May 17, 2020 4:37:12 PM GMT+03:00, Stephen Berman <[hidden email]> wrote:
>> > Does it make sense to remain in Isearch in tbis scenario?  Could it
>> > be that we are over-engineering this stuff?
>>
>> As I reported in my followup to Martin, it is only when deleting the
>> frame by clicking the toolkit's close frame button that isearch
>> remains
>> in progress, with `C-x 5 0' it is cancelled.  So the former seems to
>> be
>> the problematic case, since it's also the method used in the OP.

> Yes, I was asking whether wr should simply not try remaining in
> Isearch when frames are switched or deleted.  

I think it would certainly be good if isearch behaved the same in
response to a delete frame event, regardless of how it is generated.

>                                               Juri seems to say that
> we try not to leave Isearch in some of those cases.

I found Juri's post confusing, probably because I know next to nothing
about frame events.  He quotes this comment in isearch-mode-map:

    ;; Pass frame events transparently so they won't exit the search.

but then says "and indeed the ‘switch-frame’ event is fired when the
frame is switched during isearch, and it exits isearch", which seems to
contradict the comment.  But in any case, according to my tests, switch
the frame does indeed exit Isearch, whether via `C-x 5 o' or the
toolkits `Alt-TAB'.  Juri added: "But I don't know why the
‘delete-frame’ event is not fired on frame deletion."  But my tests
indicate that, unlike when switching the frame, when deleting the frame,
Isearch is exited only when the delete frame event comes from Emacs, not
from the toolkit.

Steve Berman



Reply | Threaded
Open this post in threaded view
|

bug#41338: Toolbar-bug in Emacs 27.0.91/Pretest

Juri Linkov-2
> I found Juri's post confusing, probably because I know next to nothing
> about frame events.  He quotes this comment in isearch-mode-map:
>
>     ;; Pass frame events transparently so they won't exit the search.

Actually this comment is confusing.  It makes no sense to me.

What makes more sense is that frame events should exit the search,
because such frame events as ‘switch-frame’ and ‘delete-frame’
change the current buffer (‘switch-frame’ changes the current buffer
to the buffer on the switched frame, and ‘delete-frame’ changes
the current buffer to the buffer on the remaining frame).

There are precautions in isearch.el that take care to exit isearch
when the current buffer changes because many things break in this case.

One of things that breaks is that reported in this bug report
where isearch-mode remembers tool-bar-map that was active
in the current buffer before starting isearch, then frame deletion
by clicking on the 'X' (close frame button) in the frame's title bar
changes the current buffer to the buffer of the remaining frame
while isearch is still active, then isearch-done called
in another buffer restores tool-bar-map in wrong buffer.

> but then says "and indeed the ‘switch-frame’ event is fired when the
> frame is switched during isearch, and it exits isearch", which seems to
> contradict the comment.

Yes, this contradicts the comment because the comment makes no sense.

> But in any case, according to my tests, switch
> the frame does indeed exit Isearch, whether via `C-x 5 o' or the
> toolkits `Alt-TAB'.

In my tests it exits isearch as well, and this is the right thing to do.

> Juri added: "But I don't know why the
> ‘delete-frame’ event is not fired on frame deletion."  But my tests
> indicate that, unlike when switching the frame, when deleting the frame,
> Isearch is exited only when the delete frame event comes from Emacs, not
> from the toolkit.

And my tests show the same: 'C-x 5 0' exits isearch, but clicking on the
close frame button doesn't exit isearch - that's the problem.



Reply | Threaded
Open this post in threaded view
|

bug#41338: Toolbar-bug in Emacs 27.0.91/Pretest

Juri Linkov-2
In reply to this post by martin rudalics
>> But I don't know why the ‘delete-frame’ event is not fired on frame deletion.
>
> How do you delete the frame?  You don't get a 'delete-frame' event when
> you delete a frame via C-x 5 0 or C-x 5 1.

The problem is that clicking on the 'X' (close frame button)
doesn't emit the ‘delete-frame’ event.



Reply | Threaded
Open this post in threaded view
|

bug#41338: Toolbar-bug in Emacs 27.0.91/Pretest

Juri Linkov-2
> If I use `emacs -Q', in the Emacs 27 pretest
> or in Emacs 26.3 or earlier, and if I do this
> and then click the `X' icon:
>
> (define-key special-event-map [delete-frame]
>   (lambda (&rest ignore)
>     (interactive)
>     (message "HHHHHHHHHHHHHH")))
>
> I just get the message, as expected.
>
> (Sorry, haven't been following this thread.)

Indeed, clicking the `X' emits the `delete-frame' event.

The problem is that no one understand this comment in isearch.el:

    ;; Pass frame events transparently so they won't exit the search.
    ;; In particular, if we have more than one display open, then a
    ;; switch-frame might be generated by someone typing at another keyboard.
    (define-key map [switch-frame] nil)
    (define-key map [delete-frame] nil)

added in 1995 by 5f48fc17d16.



Reply | Threaded
Open this post in threaded view
|

bug#41338: Toolbar-bug in Emacs 27.0.91/Pretest

Juri Linkov-2
In reply to this post by Juri Linkov-2
> Why doesn't clicking the `X' emit the
> `delete-frame' event?
>
> If I use `emacs -Q', in the Emacs 27 pretest
> or in Emacs 26.3 or earlier, and if I do this
> and then click the `X' icon:
>
> (define-key special-event-map [delete-frame]
>   (lambda (&rest ignore)
>     (interactive)
>     (message "HHHHHHHHHHHHHH")))
>
> I just get the message, as expected.

This is not what is used in isearch: isearch doesn't use special-event-map,
isearch uses overriding-terminal-local-map, so the proper test case is:

(setq overriding-terminal-local-map
      (let ((map (make-keymap)))
        (define-key map [delete-frame]
          (lambda (&rest ignore)
            (interactive)
            (message "DELETE-FRAME")))
        (define-key map "!"
          (lambda (&rest ignore)
            (interactive)
            (message "!")))
        map))

and indeed typing '!' you get the message,
but no message when clicking the `X' icon.



Reply | Threaded
Open this post in threaded view
|

bug#41338: Toolbar-bug in Emacs 27.0.91/Pretest

Juri Linkov-2
> isearch uses overriding-terminal-local-map, so the proper test case is:
>
> (setq overriding-terminal-local-map
>       (let ((map (make-keymap)))
> (define-key map [delete-frame]
>  (lambda (&rest ignore)
>    (interactive)
>    (message "DELETE-FRAME")))
> (define-key map "!"
>  (lambda (&rest ignore)
>    (interactive)
>    (message "!")))
> map))
>
> and indeed typing '!' you get the message,
> but no message when clicking the `X' icon.

The docstring of 'overriding-terminal-local-map' says:

  Per-terminal keymap that takes precedence over all other keymaps.

so maybe this is right for a *per-terminal* keymap not to react
to window events such as clicking the `X' icon?



Reply | Threaded
Open this post in threaded view
|

bug#41338: Toolbar-bug in Emacs 27.0.91/Pretest

Juri Linkov-2
>> isearch uses overriding-terminal-local-map, so the proper test case is:
>>
>> (setq overriding-terminal-local-map
>>       (let ((map (make-keymap)))
>> (define-key map [delete-frame]
>>  (lambda (&rest ignore)
>>    (interactive)
>>    (message "DELETE-FRAME")))
>> (define-key map "!"
>>  (lambda (&rest ignore)
>>    (interactive)
>>    (message "!")))
>> map))
>>
>> and indeed typing '!' you get the message,
>> but no message when clicking the `X' icon.
>
> The docstring of 'overriding-terminal-local-map' says:
>
>   Per-terminal keymap that takes precedence over all other keymaps.
>
> so maybe this is right for a *per-terminal* keymap not to react
> to window events such as clicking the `X' icon?

If there is no bug here, then I see two possible solutions for isearch:

1. use special-event-map in isearch.el the same way as isearch.el uses
   overriding-terminal-local-map;

2. use the hook ‘delete-frame-functions’ to explicitly exit isearch.