Re: [Emacs-diffs] emacs-25 d4e1549: Guard terminal parameter in XTerm mouse mode

Re: [Emacs-diffs] emacs-25 d4e1549: Guard terminal parameter in XTerm mouse mode

[Stefan Monnier]

> +	(setf (terminal-parameter nil 'xterm-mouse-last-down)
> +              ;; EVENT might be handed back to the input queue, which
> +              ;; might modify it.  Copy it into the terminal parameter
> +              ;; to guard against that.
> +              (copy-sequence event))

Do you have further information about when/where events might be
modified in place?  It sounds like a risky operation.


        Stefan

Re: [Emacs-diffs] emacs-25 d4e1549: Guard terminal parameter in XTerm mouse mode

[Philipp Stephani]

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

Stefan Monnier <monnier@iro.umontreal.ca> schrieb am Mi., 23. Nov. 2016 um
02:08 Uhr:

> > +     (setf (terminal-parameter nil 'xterm-mouse-last-down)
> > +              ;; EVENT might be handed back to the input queue, which
> > +              ;; might modify it.  Copy it into the terminal parameter
> > +              ;; to guard against that.
> > +              (copy-sequence event))
>
> Do you have further information about when/where events might be
> modified in place?  It sounds like a risky operation.
>
>
I assume it's in the code that translates consecutive mouse-down events to
mouse-movement events. It's easy to reproduce in HTerm: without the patch,
dragging the mouse while holding a button causes "drag-mouse-0" events to
be generated. These come from spurious mouse-movement events in
 xterm-mouse-last-down. Since nothing places these events explicitly there,
the root cause must be that the event list is overwritten.
Agreed that the input loop probably shouldn't modify event objects. But it
might be a necessary optimization somewhere. Unfortunately I wasn't able to
find the specific piece of code responsible for this modification.
-- 

Google Germany GmbH
Erika-Mann-Straße 33
80636 München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle

Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind,
leiten Sie diese bitte nicht weiter, informieren Sie den Absender und
löschen Sie die E-Mail und alle Anhänge. Vielen Dank.

This e-mail is confidential. If you are not the right addressee please do
not forward it, please inform the sender, and please erase this e-mail
including any attachments. Thanks.

[-- Attachment #2: Type: text/html, Size: 2478 bytes --]

Re: [Emacs-diffs] emacs-25 d4e1549: Guard terminal parameter in XTerm mouse mode

[Eli Zaretskii]

> From: Philipp Stephani <phst@google.com>
> Date: Wed, 23 Nov 2016 12:20:16 +0000
> 
> Stefan Monnier <monnier@iro.umontreal.ca> schrieb am Mi., 23. Nov. 2016 um 02:08 Uhr:
> 
>  > + (setf (terminal-parameter nil 'xterm-mouse-last-down)
>  > + ;; EVENT might be handed back to the input queue, which
>  > + ;; might modify it. Copy it into the terminal parameter
>  > + ;; to guard against that.
>  > + (copy-sequence event))
> 
>  Do you have further information about when/where events might be
>  modified in place? It sounds like a risky operation.
> 
> I assume it's in the code that translates consecutive mouse-down events to mouse-movement events. It's
> easy to reproduce in HTerm: without the patch, dragging the mouse while holding a button causes
> "drag-mouse-0" events to be generated. These come from spurious mouse-movement events in
> xterm-mouse-last-down. Since nothing places these events explicitly there, the root cause must be that the
> event list is overwritten.
> Agreed that the input loop probably shouldn't modify event objects. But it might be a necessary optimization
> somewhere. Unfortunately I wasn't able to find the specific piece of code responsible for this modification.

Does this have to be applied to the release branch?  If so, can you
tell why?

Re: [Emacs-diffs] emacs-25 d4e1549: Guard terminal parameter in XTerm mouse mode

[Philipp Stephani]

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

Eli Zaretskii <eliz@gnu.org> schrieb am Mi., 23. Nov. 2016 um 16:54 Uhr:

> > From: Philipp Stephani <phst@google.com>
> > Date: Wed, 23 Nov 2016 12:20:16 +0000
> >
> > Stefan Monnier <monnier@iro.umontreal.ca> schrieb am Mi., 23. Nov. 2016
> um 02:08 Uhr:
> >
> >  > + (setf (terminal-parameter nil 'xterm-mouse-last-down)
> >  > + ;; EVENT might be handed back to the input queue, which
> >  > + ;; might modify it. Copy it into the terminal parameter
> >  > + ;; to guard against that.
> >  > + (copy-sequence event))
> >
> >  Do you have further information about when/where events might be
> >  modified in place? It sounds like a risky operation.
> >
> > I assume it's in the code that translates consecutive mouse-down events
> to mouse-movement events. It's
> > easy to reproduce in HTerm: without the patch, dragging the mouse while
> holding a button causes
> > "drag-mouse-0" events to be generated. These come from spurious
> mouse-movement events in
> > xterm-mouse-last-down. Since nothing places these events explicitly
> there, the root cause must be that the
> > event list is overwritten.
> > Agreed that the input loop probably shouldn't modify event objects. But
> it might be a necessary optimization
> > somewhere. Unfortunately I wasn't able to find the specific piece of
> code responsible for this modification.
>
> Does this have to be applied to the release branch?  If so, can you
> tell why?
>

It's a very annoying and highly visible bug, and the fix is trivial and
cannot possibly break anything.
-- 

Google Germany GmbH
Erika-Mann-Straße 33
80636 München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle

Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind,
leiten Sie diese bitte nicht weiter, informieren Sie den Absender und
löschen Sie die E-Mail und alle Anhänge. Vielen Dank.

This e-mail is confidential. If you are not the right addressee please do
not forward it, please inform the sender, and please erase this e-mail
including any attachments. Thanks.

[-- Attachment #2: Type: text/html, Size: 3344 bytes --]

Re: [Emacs-diffs] emacs-25 d4e1549: Guard terminal parameter in XTerm mouse mode

[Eli Zaretskii]

> From: Philipp Stephani <phst@google.com>
> Date: Wed, 23 Nov 2016 17:09:30 +0000
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
>  Does this have to be applied to the release branch? If so, can you
>  tell why?
> 
> It's a very annoying and highly visible bug, and the fix is trivial and cannot possibly break anything. 

It wasn't reported until now, was it?  So I guess it's only visible
under some conditions that don't happen very often.

Can you tell since when do we have this bug?  If it exists long
enough, I don't think we should do this on the release branch, it goes
against the criteria for bug fixes there.

Re: [Emacs-diffs] emacs-25 d4e1549: Guard terminal parameter in XTerm mouse mode

[Philipp Stephani]

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

Eli Zaretskii <eliz@gnu.org> schrieb am Mi., 23. Nov. 2016 um 18:23 Uhr:

> > From: Philipp Stephani <phst@google.com>
> > Date: Wed, 23 Nov 2016 17:09:30 +0000
> > Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> >
> >  Does this have to be applied to the release branch? If so, can you
> >  tell why?
> >
> > It's a very annoying and highly visible bug, and the fix is trivial and
> cannot possibly break anything.
>
> It wasn't reported until now, was it?  So I guess it's only visible
> under some conditions that don't happen very often.
>

It happens always when selecting text with the mouse in HTerm, I think that
happens very often. Perhaps people just ignore the error message most of
the time.


>
> Can you tell since when do we have this bug?  If it exists long
> enough, I don't think we should do this on the release branch, it goes
> against the criteria for bug fixes there.
>

What are the criteria? CONTRIBUTE states
"Release branches […] are mainly
intended for more-conservative changes such as bug fixes. […]
If you are fixing a bug that exists in the current release, be sure to
commit it to the release branch; […]"
For me this makes it clear that bug fixes are always appropriate on the
release branches. If the actual policy is different, please document it in
CONTRIBUTE.
I've seen some talk about bugfix-only releases, but I think the policy
should be clearer and better documented to avoid such discussions.
-- 

Google Germany GmbH
Erika-Mann-Straße 33
80636 München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle

Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind,
leiten Sie diese bitte nicht weiter, informieren Sie den Absender und
löschen Sie die E-Mail und alle Anhänge. Vielen Dank.

This e-mail is confidential. If you are not the right addressee please do
not forward it, please inform the sender, and please erase this e-mail
including any attachments. Thanks.

[-- Attachment #2: Type: text/html, Size: 3310 bytes --]

Re: [Emacs-diffs] emacs-25 d4e1549: Guard terminal parameter in XTerm mouse mode

[Eli Zaretskii]

> From: Philipp Stephani <phst@google.com>
> Date: Thu, 24 Nov 2016 08:58:28 +0000
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
>  > It's a very annoying and highly visible bug, and the fix is trivial and cannot possibly break anything.
> 
>  It wasn't reported until now, was it? So I guess it's only visible
>  under some conditions that don't happen very often.
> 
> It happens always when selecting text with the mouse in HTerm, I think that happens very often. Perhaps
> people just ignore the error message most of the time.

Perhaps.  But that probably means it isn't urgent to fix, especially
if we had this with us for quite some time.

>  Can you tell since when do we have this bug? If it exists long
>  enough, I don't think we should do this on the release branch, it goes
>  against the criteria for bug fixes there.
> 
> What are the criteria?

As described here:

  http://lists.gnu.org/archive/html/emacs-devel/2016-10/msg00007.html
  http://lists.gnu.org/archive/html/emacs-devel/2016-10/msg00460.html
  http://lists.gnu.org/archive/html/bug-gnu-emacs/2016-10/msg00787.html

> CONTRIBUTE states
> "Release branches […] are mainly
> intended for more-conservative changes such as bug fixes. […]
> If you are fixing a bug that exists in the current release, be sure to
> commit it to the release branch; […]"

That's a general policy, but Emacs 25.2 has special rules because we
want to put it out the door very soon.

> If the actual policy is different, please document it in CONTRIBUTE.

I don't think it's right to publish ad-hoc criteria in CONTRIBUTE,
that would be confusing to new contributors.

> I've seen some talk about bugfix-only releases, but I think the policy should be clearer and better documented
> to avoid such discussions.

I don't see any good alternative to reading the mailing lists, sorry.