unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#24804: 25.1; posn-at-point erroneously signals an error
@ 2016-10-26 19:43 Andreas Politz
       [not found] ` <handler.24804.B.147751104225420.ack@debbugs.gnu.org>
  2016-10-27 17:35 ` bug#24804: 25.1; posn-at-point erroneously signals an error martin rudalics
  0 siblings, 2 replies; 11+ messages in thread
From: Andreas Politz @ 2016-10-26 19:43 UTC (permalink / raw)
  To: 24804

[-- 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

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

* bug#24804: Acknowledgement (25.1; posn-at-point erroneously signals an error)
       [not found] ` <handler.24804.B.147751104225420.ack@debbugs.gnu.org>
@ 2016-10-26 19:49   ` Andreas Politz
  0 siblings, 0 replies; 11+ messages in thread
From: Andreas Politz @ 2016-10-26 19:49 UTC (permalink / raw)
  To: 24804


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

-ap





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

* bug#24804: 25.1; posn-at-point erroneously signals an error
  2016-10-26 19:43 bug#24804: 25.1; posn-at-point erroneously signals an error Andreas Politz
       [not found] ` <handler.24804.B.147751104225420.ack@debbugs.gnu.org>
@ 2016-10-27 17:35 ` martin rudalics
  2016-10-27 18:26   ` Andreas Politz
  2016-10-28  8:12   ` Eli Zaretskii
  1 sibling, 2 replies; 11+ messages in thread
From: martin rudalics @ 2016-10-27 17:35 UTC (permalink / raw)
  To: Andreas Politz, 24804

 > 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





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

* bug#24804: 25.1; posn-at-point erroneously signals an error
  2016-10-27 17:35 ` bug#24804: 25.1; posn-at-point erroneously signals an error martin rudalics
@ 2016-10-27 18:26   ` Andreas Politz
  2016-10-28  8:12   ` Eli Zaretskii
  1 sibling, 0 replies; 11+ messages in thread
From: Andreas Politz @ 2016-10-27 18:26 UTC (permalink / raw)
  To: martin rudalics; +Cc: 24804


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





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

* bug#24804: 25.1; posn-at-point erroneously signals an error
  2016-10-27 17:35 ` bug#24804: 25.1; posn-at-point erroneously signals an error martin rudalics
  2016-10-27 18:26   ` Andreas Politz
@ 2016-10-28  8:12   ` Eli Zaretskii
  2016-10-29 10:23     ` Eli Zaretskii
  1 sibling, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2016-10-28  8:12 UTC (permalink / raw)
  To: martin rudalics; +Cc: 24804, politza

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





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

* bug#24804: 25.1; posn-at-point erroneously signals an error
  2016-10-28  8:12   ` Eli Zaretskii
@ 2016-10-29 10:23     ` Eli Zaretskii
  2016-10-29 14:26       ` Andreas Politz
  0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2016-10-29 10:23 UTC (permalink / raw)
  To: politza; +Cc: 24804

> 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);
     }
 





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

* bug#24804: 25.1; posn-at-point erroneously signals an error
  2016-10-29 10:23     ` Eli Zaretskii
@ 2016-10-29 14:26       ` Andreas Politz
  2016-10-29 14:36         ` Eli Zaretskii
  0 siblings, 1 reply; 11+ messages in thread
From: Andreas Politz @ 2016-10-29 14:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 24804


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 ?





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

* bug#24804: 25.1; posn-at-point erroneously signals an error
  2016-10-29 14:26       ` Andreas Politz
@ 2016-10-29 14:36         ` Eli Zaretskii
  2016-10-29 14:42           ` Eli Zaretskii
  0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2016-10-29 14:36 UTC (permalink / raw)
  To: Andreas Politz; +Cc: 24804

> 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?





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

* bug#24804: 25.1; posn-at-point erroneously signals an error
  2016-10-29 14:36         ` Eli Zaretskii
@ 2016-10-29 14:42           ` Eli Zaretskii
  2016-10-29 20:33             ` Andreas Politz
  0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2016-10-29 14:42 UTC (permalink / raw)
  To: politza; +Cc: 24804

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





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

* bug#24804: 25.1; posn-at-point erroneously signals an error
  2016-10-29 14:42           ` Eli Zaretskii
@ 2016-10-29 20:33             ` Andreas Politz
  2016-10-29 22:03               ` Eli Zaretskii
  0 siblings, 1 reply; 11+ messages in thread
From: Andreas Politz @ 2016-10-29 20:33 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 24804

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





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

* bug#24804: 25.1; posn-at-point erroneously signals an error
  2016-10-29 20:33             ` Andreas Politz
@ 2016-10-29 22:03               ` Eli Zaretskii
  0 siblings, 0 replies; 11+ messages in thread
From: Eli Zaretskii @ 2016-10-29 22:03 UTC (permalink / raw)
  To: Andreas Politz; +Cc: 24804-done

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





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

end of thread, other threads:[~2016-10-29 22:03 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-26 19:43 bug#24804: 25.1; posn-at-point erroneously signals an error Andreas Politz
     [not found] ` <handler.24804.B.147751104225420.ack@debbugs.gnu.org>
2016-10-26 19:49   ` bug#24804: Acknowledgement (25.1; posn-at-point erroneously signals an error) Andreas Politz
2016-10-27 17:35 ` bug#24804: 25.1; posn-at-point erroneously signals an error martin rudalics
2016-10-27 18:26   ` Andreas Politz
2016-10-28  8:12   ` Eli Zaretskii
2016-10-29 10:23     ` Eli Zaretskii
2016-10-29 14:26       ` Andreas Politz
2016-10-29 14:36         ` Eli Zaretskii
2016-10-29 14:42           ` Eli Zaretskii
2016-10-29 20:33             ` Andreas Politz
2016-10-29 22:03               ` Eli Zaretskii

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