bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

[Drew Adams]

.. shows the error:
window-divider-first-pixel is void as a variable.

Help should show the doc string of the face, not the variable,


In GNU Emacs 25.0.50.1 (i686-pc-mingw32)
 of 2014-10-20 on LEG570
Bzr revision: 118168 rgm@gnu.org-20141020195941-icp42t8ttcnud09g
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --enable-checking=yes,glyphs CPPFLAGS=-DGLYPH_DEBUG=1'

bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

[Eli Zaretskii]

> Date: Tue, 21 Apr 2015 13:02:01 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> 
> .. shows the error:
> window-divider-first-pixel is void as a variable.

Not here, it doesn't.  It's possible that this problem was already
fixed (you are using an ancient binary).  Or it's possible that the
recipe you used to trigger this is different from what I used (click
"Help->Describe->Describe Face", then type "window-divider RET" at the
prompt), in which case please show your recipe.

bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

[martin rudalics]

 >> .. shows the error:
 >> window-divider-first-pixel is void as a variable.
 >
 > Not here, it doesn't.  It's possible that this problem was already
 > fixed (you are using an ancient binary).  Or it's possible that the
 > recipe you used to trigger this is different from what I used (click
 > "Help->Describe->Describe Face", then type "window-divider RET" at the
 > prompt), in which case please show your recipe.

It happens in customization buffers.  Here I can trigger it by

M-x customize-face RET window-divider RET

and mouse-clicking 'window-divider-first-pixel' in the doc-string.

martin

bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

[Eli Zaretskii]

> Date: Wed, 22 Apr 2015 11:34:16 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: 20398@debbugs.gnu.org
> 
> I can trigger it by
> 
> M-x customize-face RET window-divider RET
> 
> and mouse-clicking 'window-divider-first-pixel' in the doc-string.

That doesn't cause an error, just a message that it's not a variable.

bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

[Drew Adams]

> > I can trigger it by
> > M-x customize-face RET window-divider RET
> > and mouse-clicking 'window-divider-first-pixel' in the doc-string.
> 
> That doesn't cause an error, just a message that it's not a
> variable.

The point is that from that customization buffer, for the related
face whose help mentions this face, you should be able to get
to this face.  A guess is that it might suffice to put the word
"face" before the face name in the doc string.  Just a wild guess.

bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

[Glenn Morris]

Eli Zaretskii wrote:

>> M-x customize-face RET window-divider RET
>> 
>> and mouse-clicking 'window-divider-first-pixel' in the doc-string.
>
> That doesn't cause an error, just a message that it's not a variable.

widget-documentation-link-action is surprisingly simplistic.
It doesn't even know that faces exist.
It should do what `help-make-xrefs' does, and use the presence of "face"
etc in the doc string as a cue. Ideally it would reuse the same code.

bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

[martin rudalics]

 > widget-documentation-link-action is surprisingly simplistic.

I tried to make following links less simplistic in 557c7d6..62fe329 on
master.

 > It doesn't even know that faces exist.
 > It should do what `help-make-xrefs' does, and use the presence of "face"
 > etc in the doc string as a cue. Ideally it would reuse the same code.

Ideally, yes.  Currently, I'm troubled by


Debugger entered--Lisp error: (error "No selection is available")
   signal(error ("No selection is available"))
   error("No selection is available")
   gui-get-primary-selection()
   mouse-yank-primary((mouse-2 (#<window 3 on *Customize Face: window-divider*> 615 (355 . 257) 4390875 nil 615 (42 . 14) nil (3 . 7) (8 . 16))))
   funcall-interactively(mouse-yank-primary (mouse-2 (#<window 3 on *Customize Face: window-divider*> 615 (355 . 257) 4390875 nil 615 (42 . 14) nil (3 . 7) (8 . 16))))
   call-interactively(mouse-yank-primary nil nil)
   command-execute(mouse-yank-primary)


with <mouse-1> on any such link in a customization buffer.  Can anyone
see this?  Does anyone have an idea where this mouse-2 yanking attempt
comes from?

martin

bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

[Eli Zaretskii]

> Date: Thu, 23 Apr 2015 09:50:33 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: 20398@debbugs.gnu.org
> 
> I'm troubled by
> 
> 
> Debugger entered--Lisp error: (error "No selection is available")
>    signal(error ("No selection is available"))
>    error("No selection is available")
>    gui-get-primary-selection()
>    mouse-yank-primary((mouse-2 (#<window 3 on *Customize Face: window-divider*> 615 (355 . 257) 4390875 nil 615 (42 . 14) nil (3 . 7) (8 . 16))))
>    funcall-interactively(mouse-yank-primary (mouse-2 (#<window 3 on *Customize Face: window-divider*> 615 (355 . 257) 4390875 nil 615 (42 . 14) nil (3 . 7) (8 . 16))))
>    call-interactively(mouse-yank-primary nil nil)
>    command-execute(mouse-yank-primary)
> 
> 
> with <mouse-1> on any such link in a customization buffer.  Can anyone
> see this?

I see this.  Everyone should, AFAICS.

> Does anyone have an idea where this mouse-2 yanking attempt comes
> from?

It comes from the command loop.  I think mouse-1 remapping, as
implied by the return value of mouse-on-link-p, has something to do
with that.

bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

[martin rudalics]

 >> Debugger entered--Lisp error: (error "No selection is available")
 >>     signal(error ("No selection is available"))
 >>     error("No selection is available")
 >>     gui-get-primary-selection()
 >>     mouse-yank-primary((mouse-2 (#<window 3 on *Customize Face: window-divider*> 615 (355 . 257) 4390875 nil 615 (42 . 14) nil (3 . 7) (8 . 16))))
 >>     funcall-interactively(mouse-yank-primary (mouse-2 (#<window 3 on *Customize Face: window-divider*> 615 (355 . 257) 4390875 nil 615 (42 . 14) nil (3 . 7) (8 . 16))))
 >>     call-interactively(mouse-yank-primary nil nil)
 >>     command-execute(mouse-yank-primary)
 >>
 >>
 >> with <mouse-1> on any such link in a customization buffer.  Can anyone
 >> see this?
 >
 > I see this.  Everyone should, AFAICS.
 >
 >> Does anyone have an idea where this mouse-2 yanking attempt comes
 >> from?
 >
 > It comes from the command loop.  I think mouse-1 remapping, as
 > implied by the return value of mouse-on-link-p, has something to do
 > with that.

If someone just told me how to investigate this.  So far I failed
miserably to do that :-(

martin

bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

[Eli Zaretskii]

> Date: Sat, 02 May 2015 10:57:28 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: rgm@gnu.org, 20398@debbugs.gnu.org
> 
>  >> Does anyone have an idea where this mouse-2 yanking attempt comes
>  >> from?
>  >
>  > It comes from the command loop.  I think mouse-1 remapping, as
>  > implied by the return value of mouse-on-link-p, has something to do
>  > with that.
> 
> If someone just told me how to investigate this.  So far I failed
> miserably to do that :-(

I cannot be that person: I also tried and failed.

Sorry.

bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

[Stefan Monnier]

> If someone just told me how to investigate this.  So far I failed
> miserably to do that :-(

I'll have to try and investigate it, to see how/if I could help you with
it,


        Stefan

bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

[martin rudalics]

 > I'll have to try and investigate it, to see how/if I could help you with
 > it,

It's been already reported as Bug#19185.  The only solution I can think
of is the trivial one below.

martin

--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -854,6 +854,9 @@ button end points."
      (define-key map [backtab] 'widget-backward)
      (define-key map [down-mouse-2] 'widget-button-click)
      (define-key map [down-mouse-1] 'widget-button-click)
+    ;; Avoid `mouse-yank-primary' with `mouse-1-click-follows-link'
+    ;; non-nil (Bug#19185).
+    (define-key map [mouse-2] 'widget-button-click)
      ;; The following definition needs to avoid using escape sequences that
      ;; might get converted to ^M when building loaddefs.el
      (define-key map [(control ?m)] 'widget-button-press)

bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

[Stefan Monnier]

>> I'll have to try and investigate it, to see how/if I could help you with it,
> It's been already reported as Bug#19185.  The only solution I can think
> of is the trivial one below.

Oh, yes I investigated already and IIRC the problem is in
widget-button-click: it's bound to the down-event but is really
a binding for the up event.

IIRC I didn't manage to fully understand the problem, but my impression
that widget-button-click should be split into two parts: one part bound
to the down-event (which would mostly do the "redraw the button as
pressed") and another bound to the up-event (which performs the actual
action associated with the click).

So I think the workaround you suggest is not that bad.  It's not a full
solution, but it does go in the right direction.


        Stefan

bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> It's been already reported as Bug#19185.  The only solution I can think
> of is the trivial one below.
>
> martin
>
> --- a/lisp/wid-edit.el
> +++ b/lisp/wid-edit.el
> @@ -854,6 +854,9 @@ button end points."
>      (define-key map [backtab] 'widget-backward)
>      (define-key map [down-mouse-2] 'widget-button-click)
>      (define-key map [down-mouse-1] 'widget-button-click)
> +    ;; Avoid `mouse-yank-primary' with `mouse-1-click-follows-link'
> +    ;; non-nil (Bug#19185).
> +    (define-key map [mouse-2] 'widget-button-click)
>      ;; The following definition needs to avoid using escape sequences that
>      ;; might get converted to ^M when building loaddefs.el

[...]

> So I think the workaround you suggest is not that bad.  It's not a full
> solution, but it does go in the right direction.

Martin, you never applied your patch.  Was there a reason for that?

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

bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

[Stefan Monnier]

>>      (define-key map [backtab] 'widget-backward)
>>      (define-key map [down-mouse-2] 'widget-button-click)
>>      (define-key map [down-mouse-1] 'widget-button-click)
>> +    ;; Avoid `mouse-yank-primary' with `mouse-1-click-follows-link'
>> +    ;; non-nil (Bug#19185).
>> +    (define-key map [mouse-2] 'widget-button-click)

That seems to imply that widget-button-click will be run twice (once on
the down part and once on the up part).
How 'bout binding one of the two to `undefined' instead?


        Stefan

bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

[Stefan Monnier]

>>> (define-key map [backtab] 'widget-backward)
>>> (define-key map [down-mouse-2] 'widget-button-click)
>>> (define-key map [down-mouse-1] 'widget-button-click)
>>> +    ;; Avoid `mouse-yank-primary' with `mouse-1-click-follows-link'
>>> +    ;; non-nil (Bug#19185).
>>> +    (define-key map [mouse-2] 'widget-button-click)

> That seems to imply that widget-button-click will be run twice (once on
> the down part and once on the up part).
> How 'bout binding one of the two to `undefined' instead?

Or just *move* the binding from [down-mouse-2] to [mouse-2] (since it's
a click binding and clicks should be bound to the up event rather than
the down event).


        Stefan

bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

[martin rudalics]

 >> So I think the workaround you suggest is not that bad.  It's not a full
 >> solution, but it does go in the right direction.
 >
 > Martin, you never applied your patch.  Was there a reason for that?

It was a half-baked idea IIRC.  This issue requires more work, I'm
afraid.

martin

bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

[martin rudalics]

 >>>> (define-key map [backtab] 'widget-backward)
 >>>> (define-key map [down-mouse-2] 'widget-button-click)
 >>>> (define-key map [down-mouse-1] 'widget-button-click)
 >>>> +    ;; Avoid `mouse-yank-primary' with `mouse-1-click-follows-link'
 >>>> +    ;; non-nil (Bug#19185).
 >>>> +    (define-key map [mouse-2] 'widget-button-click)
 >
 >> That seems to imply that widget-button-click will be run twice (once on
 >> the down part and once on the up part).
 >> How 'bout binding one of the two to `undefined' instead?
 >
 > Or just *move* the binding from [down-mouse-2] to [mouse-2] (since it's
 > a click binding and clicks should be bound to the up event rather than
 > the down event).

Shouldn't we then do the same here:

 >>>> (define-key map [down-mouse-1] 'widget-button-click)

martin

bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

[Stefan Monnier]

> Shouldn't we then do the same here:
>>>>> (define-key map [down-mouse-1] 'widget-button-click)

I think so, yes.


        Stefan

bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

[martin rudalics]

 >> Shouldn't we then do the same here:
 >>>>>> (define-key map [down-mouse-1] 'widget-button-click)
 >
 > I think so, yes.

So the patch is as below.  Any objections to apply this to the emacs-25
branch?

martin

diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index f0054be..0a0f458 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -852,8 +852,8 @@ widget-keymap
      (define-key map [(shift tab)] 'widget-backward)
      (put 'widget-backward :advertised-binding [(shift tab)])
      (define-key map [backtab] 'widget-backward)
-    (define-key map [down-mouse-2] 'widget-button-click)
-    (define-key map [down-mouse-1] 'widget-button-click)
+    (define-key map [mouse-2] 'widget-button-click)
+    (define-key map [mouse-1] 'widget-button-click)
      ;; The following definition needs to avoid using escape sequences that
      ;; might get converted to ^M when building loaddefs.el
      (define-key map [(control ?m)] 'widget-button-press)

bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> So the patch is as below.  Any objections to apply this to the emacs-25
> branch?

[...]

> -    (define-key map [down-mouse-2] 'widget-button-click)
> -    (define-key map [down-mouse-1] 'widget-button-click)
> +    (define-key map [mouse-2] 'widget-button-click)
> +    (define-key map [mouse-1] 'widget-button-click)

Isn't this a rather large change to go in at this point?  People may
have rebound `down-mouse-1' (etc) instead of `mouse-1'...

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

bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

[martin rudalics]

 >> So the patch is as below.  Any objections to apply this to the emacs-25
 >> branch?
 >
 > [...]
 >
 >> -    (define-key map [down-mouse-2] 'widget-button-click)
 >> -    (define-key map [down-mouse-1] 'widget-button-click)
 >> +    (define-key map [mouse-2] 'widget-button-click)
 >> +    (define-key map [mouse-1] 'widget-button-click)
 >
 > Isn't this a rather large change to go in at this point?  People may
 > have rebound `down-mouse-1' (etc) instead of `mouse-1'...

No idea.  But counted as objection and therefore committed to master.
Closing the bug.

martin

bug#20398: 25.0.50; Clicking `window-divider-last-pixel' in help for `window-divider'

[Stefan Monnier]

> No idea.  But counted as objection and therefore committed to master.

FWIW, I also think it was too risky for emacs-25.

> Closing the bug.

Thanks,


        Stefan