* 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-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 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
* 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-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
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).