unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#13560: 24.2.92; tabulated-list header buttons are broken
@ 2013-01-26 18:31 Jonas Bernoulli
  2013-01-27  2:52 ` Glenn Morris
  2013-01-28 17:11 ` Glenn Morris
  0 siblings, 2 replies; 10+ messages in thread
From: Jonas Bernoulli @ 2013-01-26 18:31 UTC (permalink / raw)
  To: 13560

Starting with 24.2.92 clicking on a column button created by a
Tabulated-List mode derived mode does not sort correctly anymore.

When clicking the first time the order should not be reversed, but it
is.  Clicking some more does not change the order, it stays reversed.  I
looked into why this is so and noticed that the command the mouse event
is bound to is actually called twice.

Thinking that for some reason clicking on the button on the header-line
generates two events I modified tabulated-list-sort-button-map to only
contain a binding for [header-line mouse-1].  Then I clicked again with
the mouse button 1 and got the following warning in the echo area:

,----
| <header-line> <mouse-2> is undefined
`----

It mentions mouse-2 even though I pressed mouse-1.  Also the message did
not appear in *Messages* and I could not get a backtrace after turning
on debug-on-error.

To check whether this is specific to Tabulated-List mode I tried the
following:

,----
| (setq header-line-format
|       (propertize "button"
|                   'keymap (let ((map (make-sparse-keymap)))
|                             (define-key map [header-line mouse-1]
|                               (lambda (e)
|                                 (interactive "e")
|                                 (message "%s" e)))
|                             map)))
`----

Here everything works as expected: I don't get a warning about mouse-2
not being undefined, the command is only called once and it reports that
mouse-1 was pressed.

  Jonas






^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#13560: 24.2.92; tabulated-list header buttons are broken
  2013-01-26 18:31 bug#13560: 24.2.92; tabulated-list header buttons are broken Jonas Bernoulli
@ 2013-01-27  2:52 ` Glenn Morris
  2013-01-28  1:36   ` Glenn Morris
  2013-01-28 17:11 ` Glenn Morris
  1 sibling, 1 reply; 10+ messages in thread
From: Glenn Morris @ 2013-01-27  2:52 UTC (permalink / raw)
  To: Jonas Bernoulli; +Cc: 13560

Jonas Bernoulli wrote:

> Starting with 24.2.92 clicking on a column button created by a
> Tabulated-List mode derived mode does not sort correctly anymore.

Thanks for the report. Eg visible in buffer-menu, where mouse-1 does not
sort.

I don't know what's going on here, but it seems to have some relation to
mouse-1-click-follows-link. If you set it to nil, or leave it alone but
make a long mouse-1 click, then it works. Otherwise it calls the sort
function twice as you say.

This seems like something that should be fixed for 24.3.

> When clicking the first time the order should not be reversed, but it
> is.  Clicking some more does not change the order, it stays reversed.  I
> looked into why this is so and noticed that the command the mouse event
> is bound to is actually called twice.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#13560: 24.2.92; tabulated-list header buttons are broken
  2013-01-27  2:52 ` Glenn Morris
@ 2013-01-28  1:36   ` Glenn Morris
  2013-01-30  1:27     ` Glenn Morris
  0 siblings, 1 reply; 10+ messages in thread
From: Glenn Morris @ 2013-01-28  1:36 UTC (permalink / raw)
  To: Jonas Bernoulli; +Cc: 13560


Bisection suggests this stopped working with:

    On revision 110949 (cyd@gnu.org-20121124081500-x95lz4fz0j0t052c):
    Fix follow-mouse clicks on undraggable mode/header lines.

    * mouse.el (mouse-drag-line): Even if the line is not draggable,
    keep reading until we get the up-event anyway, in order to process
    the up-event for mouse-1-click-follows-link.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#13560: 24.2.92; tabulated-list header buttons are broken
  2013-01-26 18:31 bug#13560: 24.2.92; tabulated-list header buttons are broken Jonas Bernoulli
  2013-01-27  2:52 ` Glenn Morris
@ 2013-01-28 17:11 ` Glenn Morris
  2013-01-29  3:11   ` Glenn Morris
  1 sibling, 1 reply; 10+ messages in thread
From: Glenn Morris @ 2013-01-28 17:11 UTC (permalink / raw)
  To: Jonas Bernoulli; +Cc: 13560

Jonas Bernoulli wrote:

> Thinking that for some reason clicking on the button on the header-line
> generates two events I modified tabulated-list-sort-button-map to only
> contain a binding for [header-line mouse-1].  Then I clicked again with
> the mouse button 1 and got the following warning in the echo area:
>
> ,----
> | <header-line> <mouse-2> is undefined
> `----

I guess this is due to the mouse-1-click-follows-link issue. Because the
link you click has mouse-face, clicking mouse-1 tries to run the mouse-2
binding.

I tried only binding mouse-2 in tabulated-list-sort-button-map, but that
did not help. Not sure why it binds mouse-1 explicitly, yet uses
follow-link as well.

> To check whether this is specific to Tabulated-List mode I tried the
> following:
>
> ,----
> | (setq header-line-format
> |       (propertize "button"
> |                   'keymap (let ((map (make-sparse-keymap)))
> |                             (define-key map [header-line mouse-1]
> |                               (lambda (e)
> |                                 (interactive "e")
> |                                 (message "%s" e)))
> |                             map)))
> `----
>
> Here everything works as expected: I don't get a warning about mouse-2
> not being undefined, the command is only called once and it reports that
> mouse-1 was pressed.

A test that uses follow-link would be more informative, but at the
moment this seems like a general mouse.el issue rather than anything
specific to tabulated-list. Are there any other modes that use
follow-link in the header line?





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#13560: 24.2.92; tabulated-list header buttons are broken
  2013-01-28 17:11 ` Glenn Morris
@ 2013-01-29  3:11   ` Glenn Morris
  0 siblings, 0 replies; 10+ messages in thread
From: Glenn Morris @ 2013-01-29  3:11 UTC (permalink / raw)
  To: Jonas Bernoulli; +Cc: 13560

Glenn Morris wrote:

> A test that uses follow-link would be more informative, but at the
> moment this seems like a general mouse.el issue rather than anything
> specific to tabulated-list. Are there any other modes that use
> follow-link in the header line?

Yes; info.el, and this is busted in the same way.

Clicking mouse-1 in the Next/Prev breadcrumbs in the header-line of an
Info page moves by two nodes rather than one. Clicking mouse-2 moves
one.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#13560: 24.2.92; tabulated-list header buttons are broken
  2013-01-28  1:36   ` Glenn Morris
@ 2013-01-30  1:27     ` Glenn Morris
  2013-01-30 16:05       ` martin rudalics
  2013-01-30 19:28       ` Stefan Monnier
  0 siblings, 2 replies; 10+ messages in thread
From: Glenn Morris @ 2013-01-30  1:27 UTC (permalink / raw)
  To: 13560; +Cc: Jonas Bernoulli, Chong Yidong


Ok, I think the issue here is mouse-drag-line pushing the same event
onto unread-command-events twice in some cases.

The recent history is a bit convoluted:

2012-07-08 changes to fix bug#374, and simplify mouse-drag-line loop.

This caused bug#12006, fixed 2012-07-26. These changes reverted some of
the loop simplifications, for reasons I don't understand.

These changes in turn caused bug#12971 (no mouse-1 events in header),
fixed 2012-11-24. It seems the fix actually gave us two sets of mouse-1
events in headers though (this bug).

And then we have 2012-10-26 changes for bug#12731, introduced at some
point along the way.


What seems to work for me is going back to the 2012-07-26 changes, and
reverting some of reversion. Basically, just check for switch-frame and
select-window events and ignore them. Then I found I needed one more
tweak to preserve the bug#12731 fix.

The following change seems to not break any of bug 374, 12006, 12971,
12731, and to fix this bug; but I won't be surprised to hear that it
breaks something else...



*** lisp/mouse.el	2013-01-01 09:11:05 +0000
--- lisp/mouse.el	2013-01-29 22:22:44 +0000
***************
*** 425,431 ****
  				   (frame-parameters frame)))
  			'right)))
  	 (draggable t)
! 	 finished event position growth dragged)
      (cond
       ((eq line 'header)
        ;; Check whether header-line can be dragged at all.
--- 425,431 ----
  				   (frame-parameters frame)))
  			'right)))
  	 (draggable t)
! 	 event position growth dragged)
      (cond
       ((eq line 'header)
        ;; Check whether header-line can be dragged at all.
***************
*** 456,494 ****
  
      ;; Start tracking.
      (track-mouse
!       ;; Loop reading events and sampling the position of the mouse.
!       (while (not finished)
! 	(setq event (read-event))
  	(setq position (mouse-position))
  	;; Do nothing if
  	;;   - there is a switch-frame event.
  	;;   - the mouse isn't in the frame that we started in
  	;;   - the mouse isn't in any Emacs frame
- 	;; Drag if
- 	;;   - there is a mouse-movement event
- 	;;   - there is a scroll-bar-movement event (Why? -- cyd)
- 	;;     (same as mouse movement for our purposes)
- 	;; Quit if
- 	;;   - there is a keyboard event or some other unknown event.
  	(cond
- 	 ((not (consp event))
- 	  (setq finished t))
  	 ((memq (car event) '(switch-frame select-window))
  	  nil)
! 	 ((not (memq (car event) '(mouse-movement scroll-bar-movement)))
! 	  (when (consp event)
! 	    ;; Do not unread a drag-mouse-1 event to avoid selecting
! 	    ;; some other window.  For vertical line dragging do not
! 	    ;; unread mouse-1 events either (but only if we dragged at
! 	    ;; least once to allow mouse-1 clicks get through).
! 	    (unless (and dragged
! 			 (if (eq line 'vertical)
! 			     (memq (car event) '(drag-mouse-1 mouse-1))
! 			   (eq (car event) 'drag-mouse-1)))
! 	      (push event unread-command-events)))
! 	  (setq finished t))
! 	 ((not (and (eq (car position) frame)
! 		    (cadr position)))
  	  nil)
  	 ((eq line 'vertical)
  	  ;; Drag vertical divider.
--- 456,480 ----
  
      ;; Start tracking.
      (track-mouse
!       ;; Loop reading events and sampling the position of the mouse,
!       ;; until there is a non-mouse-movement event.  Also,
!       ;; scroll-bar-movement events are the same as mouse movement for
!       ;; our purposes.  (Why? -- cyd)
!       (while (progn
! 	       (setq event (read-event))
! 	       (memq (car-safe event)
!                      '(mouse-movement scroll-bar-movement
!                                       switch-frame select-window)))
  	(setq position (mouse-position))
  	;; Do nothing if
  	;;   - there is a switch-frame event.
  	;;   - the mouse isn't in the frame that we started in
  	;;   - the mouse isn't in any Emacs frame
  	(cond
  	 ((memq (car event) '(switch-frame select-window))
  	  nil)
!  	 ((not (and (eq (car position) frame)
!  		    (cadr position)))
  	  nil)
  	 ((eq line 'vertical)
  	  ;; Drag vertical divider.
***************
*** 512,523 ****
  						  growth
  						(- growth)))))))
      ;; Process the terminating event.
!     (when (and (mouse-event-p event) on-link (not dragged)
! 	       (mouse--remap-link-click-p start-event event))
!       ;; If mouse-2 has never been done by the user, it doesn't have
!       ;; the necessary property to be interpreted correctly.
!       (put 'mouse-2 'event-kind 'mouse-click)
!       (setcar event 'mouse-2)
        (push event unread-command-events))))
  
  (defun mouse-drag-mode-line (start-event)
--- 498,510 ----
  						  growth
  						(- growth)))))))
      ;; Process the terminating event.
!     (unless dragged
!       (when (and (mouse-event-p event) on-link
!                  (mouse--remap-link-click-p start-event event))
!         ;; If mouse-2 has never been done by the user, it doesn't have
!         ;; the necessary property to be interpreted correctly.
!         (put 'mouse-2 'event-kind 'mouse-click)
!         (setcar event 'mouse-2))
        (push event unread-command-events))))
  
  (defun mouse-drag-mode-line (start-event)






^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#13560: 24.2.92; tabulated-list header buttons are broken
  2013-01-30  1:27     ` Glenn Morris
@ 2013-01-30 16:05       ` martin rudalics
  2013-01-30 17:18         ` Glenn Morris
  2013-01-30 19:28       ` Stefan Monnier
  1 sibling, 1 reply; 10+ messages in thread
From: martin rudalics @ 2013-01-30 16:05 UTC (permalink / raw)
  To: Glenn Morris; +Cc: Jonas Bernoulli, 13560, Chong Yidong

> The following change seems to not break any of bug 374, 12006, 12971,
> 12731, and to fix this bug; but I won't be surprised to hear that it
> breaks something else...

So far it works here.  I'd suggest to install and port to the trunk
immediately so it gets more coverage.

Thanks, martin







^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#13560: 24.2.92; tabulated-list header buttons are broken
  2013-01-30 16:05       ` martin rudalics
@ 2013-01-30 17:18         ` Glenn Morris
  2013-01-31 16:04           ` Jonas Bernoulli
  0 siblings, 1 reply; 10+ messages in thread
From: Glenn Morris @ 2013-01-30 17:18 UTC (permalink / raw)
  To: martin rudalics; +Cc: Jonas Bernoulli, 13560, Chong Yidong

martin rudalics wrote:

> So far it works here.  I'd suggest to install and port to the trunk
> immediately so it gets more coverage.

Done.






^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#13560: 24.2.92; tabulated-list header buttons are broken
  2013-01-30  1:27     ` Glenn Morris
  2013-01-30 16:05       ` martin rudalics
@ 2013-01-30 19:28       ` Stefan Monnier
  1 sibling, 0 replies; 10+ messages in thread
From: Stefan Monnier @ 2013-01-30 19:28 UTC (permalink / raw)
  To: Glenn Morris; +Cc: Jonas Bernoulli, 13560, Chong Yidong

> The following change seems to not break any of bug 374, 12006, 12971,
> 12731, and to fix this bug; but I won't be surprised to hear that it
> breaks something else...

Sounds better than what we have now.


        Stefan "who hates unread-command-events"





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#13560: 24.2.92; tabulated-list header buttons are broken
  2013-01-30 17:18         ` Glenn Morris
@ 2013-01-31 16:04           ` Jonas Bernoulli
  0 siblings, 0 replies; 10+ messages in thread
From: Jonas Bernoulli @ 2013-01-31 16:04 UTC (permalink / raw)
  To: Glenn Morris; +Cc: 13560


Glenn Morris <rgm@gnu.org> writes:

> martin rudalics wrote:
>
>> So far it works here.  I'd suggest to install and port to the trunk
>> immediately so it gets more coverage.
>
> Done.

Thanks a lot!





^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2013-01-31 16:04 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-26 18:31 bug#13560: 24.2.92; tabulated-list header buttons are broken Jonas Bernoulli
2013-01-27  2:52 ` Glenn Morris
2013-01-28  1:36   ` Glenn Morris
2013-01-30  1:27     ` Glenn Morris
2013-01-30 16:05       ` martin rudalics
2013-01-30 17:18         ` Glenn Morris
2013-01-31 16:04           ` Jonas Bernoulli
2013-01-30 19:28       ` Stefan Monnier
2013-01-28 17:11 ` Glenn Morris
2013-01-29  3:11   ` Glenn Morris

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).