bug#24804: 25.1; posn-at-point erroneously signals an error

bug#24804: 25.1; posn-at-point erroneously signals an error

[Andreas Politz]

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


The documentation of the function posn-at-point states

"Return nil if position is not visible in window.",

but it may also signal an error in this case.  This happens, if
Fpos_visible_in_window_p returns a list of (X Y RTOP RBOT ROWH VPOS)
and at least Y is negative (which indicates, that pos is not visible
IIUC).  The error is then signaled by Fposn_at_x_y, which only accpets
non-negative numbers (neglecting the exceptional case of -1 for X).

I think this function should include a similar test for y, as is already
in place for x, returning nil if it is negative (see below).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: posn_at_point.diff --]
[-- Type: text/x-diff, Size: 686 bytes --]

diff --git a/src/keyboard.c b/src/keyboard.c
index bb411e7..215fcb9 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -10791,9 +10791,10 @@ The `posn-' functions access elements of such lists.  */)
       Lisp_Object x = XCAR (tem);
       Lisp_Object y = XCAR (XCDR (tem));
 
-      /* Point invisible due to hscrolling?  X can be -1 when a
-	 newline in a R2L line overflows into the left fringe.  */
-      if (XINT (x) < -1)
+      /* Point invisible due to hscrolling or vscrolling?  X can be -1
+	 when a newline in a R2L line overflows into the left
+	 fringe.  */
+      if (XINT (x) < -1 || XINT (y) < 0)
 	return Qnil;
       tem = Fposn_at_x_y (x, y, window, Qnil);
     }

[-- Attachment #3: Type: text/plain, Size: 616 bytes --]


--

Reproducing this state may be a little bit tricky, anyway evaluate the
following lines, starting with `emacs -Q'.

(defvar img "foo.png");; The image should be taller then the window
                      ;; it's displayed in.
(setq debug-on-error t)
(find-file img)
(redisplay t)         ;; If in batch mode.
(image-scroll-up 999)
;; The image should be scrolled to the bottom now, while point equals 1.
(posn-at-point (point-max) (selected-window))

--

Note, that this bug may make switching buffers difficult in certain
situations, as was reported here:
https://github.com/politza/pdf-tools/issues/200

-ap

bug#24804: Acknowledgement (25.1; posn-at-point erroneously signals an error)

[Andreas Politz]

Duplicate of #23809 and #21732 -- should have checked first.

-ap

bug#24804: 25.1; posn-at-point erroneously signals an error

[martin rudalics]

 > The documentation of the function posn-at-point states
 >
 > "Return nil if position is not visible in window.",
 >
 > but it may also signal an error in this case.  This happens, if
 > Fpos_visible_in_window_p returns a list of (X Y RTOP RBOT ROWH VPOS)
 > and at least Y is negative (which indicates, that pos is not visible
 > IIUC).  The error is then signaled by Fposn_at_x_y, which only accpets
 > non-negative numbers (neglecting the exceptional case of -1 for X).
 >
 > I think this function should include a similar test for y, as is already
 > in place for x, returning nil if it is negative (see below).

Thanks.  But according to Eli the problem is in Fpos_visible_in_window_p
which should never return a negative y in the first place.

 > Reproducing this state may be a little bit tricky, anyway evaluate the
 > following lines, starting with `emacs -Q'.
 >
 > (defvar img "foo.png");; The image should be taller then the window
 >                        ;; it's displayed in.
 > (setq debug-on-error t)
 > (find-file img)
 > (redisplay t)         ;; If in batch mode.
 > (image-scroll-up 999)
 > ;; The image should be scrolled to the bottom now, while point equals 1.
 > (posn-at-point (point-max) (selected-window))

That's a valuable information.  I indeed can reproduce the problem with
this scenario (but only on another machine where I can display images
and the Emacs there must be repaired).  Could you try answering the
question Eli asked for bug#23809 namely:

   . Can you show the entire value returned by Fpos_visible_in_window_p
     when its call from Fposn_at_point returns, when this problem is
     reproduced?

Thanks again, martin

bug#24804: 25.1; posn-at-point erroneously signals an error

[Andreas Politz]

martin rudalics <rudalics@gmx.at> writes:

> Thanks.  But according to Eli the problem is in Fpos_visible_in_window_p
> which should never return a negative y in the first place.

Ok, that's different.

>   . Can you show the entire value returned by Fpos_visible_in_window_p
>     when its call from Fposn_at_point returns, when this problem is
>     reproduced?

Result for the mentioned setup:

(747 -36 36 6 925 0)

-ap

bug#24804: 25.1; posn-at-point erroneously signals an error

[Eli Zaretskii]

> Date: Thu, 27 Oct 2016 19:35:55 +0200
> From: martin rudalics <rudalics@gmx.at>
> 
>  > The documentation of the function posn-at-point states
>  >
>  > "Return nil if position is not visible in window.",
>  >
>  > but it may also signal an error in this case.  This happens, if
>  > Fpos_visible_in_window_p returns a list of (X Y RTOP RBOT ROWH VPOS)
>  > and at least Y is negative (which indicates, that pos is not visible
>  > IIUC).  The error is then signaled by Fposn_at_x_y, which only accpets
>  > non-negative numbers (neglecting the exceptional case of -1 for X).
>  >
>  > I think this function should include a similar test for y, as is already
>  > in place for x, returning nil if it is negative (see below).
> 
> Thanks.  But according to Eli the problem is in Fpos_visible_in_window_p
> which should never return a negative y in the first place.

Maybe it could, when an image is vscrolled?

In any case, the proposed patch is IMO incorrect: when point is at a
large image (or even a character glyph that's very tall relative to
its window's height), this patch will pretend that point is invisible,
whereas in fact it's partially visible (as is clear from the use case
discussed here).

The key to understanding why the patch is wrong is to remember that
the Y coordinate of a glyph on display is the coordinate of the top of
the glyph.  For a large glyph, its top can be off-screen, but most of
it could still be visible.  This is evident in the value returned by
pos-visible-in-window-p in this case: (747 -36 36 6 925 0).  It
exactly means that the image starts 36 pixels above the window top
(and ends 6 pixels below its bottom).

Perhaps if RTOP is non-nil, we should pass Y + RTOP to posn-at-x-y.

bug#24804: 25.1; posn-at-point erroneously signals an error

[Eli Zaretskii]

> Date: Fri, 28 Oct 2016 11:12:19 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 24804@debbugs.gnu.org, politza@hochschule-trier.de
> 
> > Thanks.  But according to Eli the problem is in Fpos_visible_in_window_p
> > which should never return a negative y in the first place.
> 
> Maybe it could, when an image is vscrolled?
> 
> In any case, the proposed patch is IMO incorrect: when point is at a
> large image (or even a character glyph that's very tall relative to
> its window's height), this patch will pretend that point is invisible,
> whereas in fact it's partially visible (as is clear from the use case
> discussed here).
> 
> The key to understanding why the patch is wrong is to remember that
> the Y coordinate of a glyph on display is the coordinate of the top of
> the glyph.  For a large glyph, its top can be off-screen, but most of
> it could still be visible.  This is evident in the value returned by
> pos-visible-in-window-p in this case: (747 -36 36 6 925 0).  It
> exactly means that the image starts 36 pixels above the window top
> (and ends 6 pixels below its bottom).
> 
> Perhaps if RTOP is non-nil, we should pass Y + RTOP to posn-at-x-y.

Andreas, can you try the patch below?  Thanks.

diff --git a/src/keyboard.c b/src/keyboard.c
index bb411e7..65938a5 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -10790,11 +10790,19 @@ The `posn-' functions access elements of such lists.  */)
     {
       Lisp_Object x = XCAR (tem);
       Lisp_Object y = XCAR (XCDR (tem));
+      Lisp_Object aux_info = XCDR (XCDR (tem));
+      int y_coord = XINT (y);
 
       /* Point invisible due to hscrolling?  X can be -1 when a
 	 newline in a R2L line overflows into the left fringe.  */
       if (XINT (x) < -1)
 	return Qnil;
+      if (!NILP (aux_info) && y_coord < 0)
+	{
+	  int rtop = XINT (XCAR (aux_info));
+
+	  y = make_number (y_coord + rtop);
+	}
       tem = Fposn_at_x_y (x, y, window, Qnil);
     }
 

bug#24804: 25.1; posn-at-point erroneously signals an error

[Andreas Politz]

Eli Zaretskii <eliz@gnu.org> writes:

> Andreas, can you try the patch below?  Thanks.

I can confirm, that the error regarding above recpie goes away.

Though I'm pretty confused about what this function should actually
return in this case.  E.g. in the scenario above, evaluating (posn-x-y
(posn-at-point x)) with x=point-min, resp. x=point-max, returns (0 . 0),
resp.  (747 . 0), where 747 is the width of the image.  Does that sound
right ?

bug#24804: 25.1; posn-at-point erroneously signals an error

[Eli Zaretskii]

> From: Andreas Politz <politza@hochschule-trier.de>
> Cc: rudalics@gmx.at,  24804@debbugs.gnu.org
> Date: Sat, 29 Oct 2016 16:26:47 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Andreas, can you try the patch below?  Thanks.
> 
> I can confirm, that the error regarding above recpie goes away.

Great, I will push it to master soon.

> Though I'm pretty confused about what this function should actually
> return in this case.  E.g. in the scenario above, evaluating (posn-x-y
> (posn-at-point x)) with x=point-min, resp. x=point-max, returns (0 . 0),
> resp.  (747 . 0), where 747 is the width of the image.  Does that sound
> right ?

It sounds right to me: the first position is at the image, the second
is after the image, so its X coordinate should be the image width.
Does that make sense?

bug#24804: 25.1; posn-at-point erroneously signals an error

[Eli Zaretskii]

> Date: Sat, 29 Oct 2016 17:36:22 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 24804@debbugs.gnu.org
> 
> > From: Andreas Politz <politza@hochschule-trier.de>
> > Cc: rudalics@gmx.at,  24804@debbugs.gnu.org
> > Date: Sat, 29 Oct 2016 16:26:47 +0200
> > 
> > Eli Zaretskii <eliz@gnu.org> writes:
> > 
> > > Andreas, can you try the patch below?  Thanks.
> > 
> > I can confirm, that the error regarding above recpie goes away.
> 
> Great, I will push it to master soon.

Done.  Let me know if something else should be done before closing
this bug.

Thanks.

bug#24804: 25.1; posn-at-point erroneously signals an error

[Andreas Politz]

Eli Zaretskii <eliz@gnu.org> writes:

> Done.  Let me know if something else should be done before closing
> this bug.

Nothing.  I'm not clear about the general semantics of this function,
but we don't need to discuss this right now.

Thanks,
-ap

bug#24804: 25.1; posn-at-point erroneously signals an error

[Eli Zaretskii]

> From: Andreas Politz <politza@hochschule-trier.de>
> Cc: 24804@debbugs.gnu.org
> Date: Sat, 29 Oct 2016 22:33:00 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Done.  Let me know if something else should be done before closing
> > this bug.
> 
> Nothing.  I'm not clear about the general semantics of this function,
> but we don't need to discuss this right now.

OK, I'm therefore closing the bug (and the two merged with it).
People who think there are still leftovers are welcome to reopen with
details.

As for your questions about the semantics, I suggest to start a
discussion on emacs-devel.

Thanks.