bug#7390: 24.0.50; pos-col-row gives incorrect row

bug#7390: 24.0.50; pos-col-row gives incorrect row

[Drew Adams]

emacs -Q
 
Load David Ponce's tabbar.el version 1.3, from here:
http://sourceforge.net/projects/emhacks/files/tabbar/1.3/tabbar-1.3.tar.gz/downl
oad
 
M-x tabbar-mode
 
That adds a tab bar to each window.  I don't see anything in the
tabbar.el code that should perturb the behavior of pos-col-row, but
something does.  I'm no expert on the tabbar code or pos-col-row.  I
hope that someone can debug this and fix it.  My guess is that
pos-col-row is incorrectly thinking that the tab bar is part of the
buffer.
 
The easiest way to see the problem is to do this:
 
M-x picture-mode
 
Click mouse-1 somewhere - e.g. the fourth char in some buffer.  The
result is that the cursor (point) is moved down one line from where you
clicked, so the active region shows from the click position to point
(one line down).
 
Do the same thing with tabbar-mode turned off, and there is no problem:
point does not move down a line: it is exactly where you clicked.
 
The problem arises in picture-mouse-set-point, where it calls
pos-col-row: the wrong row is returned.  This is presumably because it
is counting the tabbar row, which is not part of the buffer.
 
Dunno whether this is a bug in pos-col-row (that would be my guess) or
in tabbar.el.  I shouldn't think that tabbar.el should need to do
something to compensate and make pos-col-row DTRT.  But again, I'm no
expert on this.

In GNU Emacs 24.0.50.1 (i386-mingw-nt5.1.2600)
 of 2010-11-09 on 3249CTO
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (4.4) --no-opt --cflags
-Ic:/imagesupport/include'
 

bug#7390: 24.0.50; pos-col-row gives incorrect row

[Eli Zaretskii]

> Resent-Sender: help-debbugs@gnu.org
> From: "Drew Adams" <drew.adams@oracle.com>
> Date: Sat, 13 Nov 2010 09:55:15 -0800
> Cc: 
> 
> emacs -Q
>  
> Load David Ponce's tabbar.el version 1.3, from here:
> http://sourceforge.net/projects/emhacks/files/tabbar/1.3/tabbar-1.3.tar.gz/downl
> oad
>  
> M-x tabbar-mode
>  
> That adds a tab bar to each window.  I don't see anything in the
> tabbar.el code that should perturb the behavior of pos-col-row, but
> something does.  I'm no expert on the tabbar code or pos-col-row.  I
> hope that someone can debug this and fix it.  My guess is that
> pos-col-row is incorrectly thinking that the tab bar is part of the
> buffer.
>  
> The easiest way to see the problem is to do this:
>  
> M-x picture-mode
>  
> Click mouse-1 somewhere - e.g. the fourth char in some buffer.  The
> result is that the cursor (point) is moved down one line from where you
> clicked, so the active region shows from the click position to point
> (one line down).

As a matter of fact, this problem is present in Emacs 23.2 as well,
and in the current pretest of Emacs 23.3.

An easier way to reproduce is this:

  emacs -Q
  C-x b foo RET
  M-: (setq header-line-format "foo") RET
  M-x picture-mode RET

Now type 2 lines of text, and click the mouse on the first line.

bug#7390: 24.0.50; pos-col-row gives incorrect row

[Drew Adams]

> As a matter of fact, this problem is present in Emacs 23.2 as well,
> and in the current pretest of Emacs 23.3.
> 
> An easier way to reproduce is this:
> 
>   emacs -Q
>   C-x b foo RET
>   M-: (setq header-line-format "foo") RET
>   M-x picture-mode RET
> 
> Now type 2 lines of text, and click the mouse on the first line.

Thanks for narrowing this down, in particular removing tabbar.el from the
equation.

bug#7390: 24.0.50; pos-col-row gives incorrect row

[Eli Zaretskii]

> Date: Sat, 13 Nov 2010 21:02:13 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 7390@debbugs.gnu.org
> 
>   emacs -Q
>   C-x b foo RET
>   M-: (setq header-line-format "foo") RET
>   M-x picture-mode RET
> 
> Now type 2 lines of text, and click the mouse on the first line.

I see that Chong modified picture.el to not use posn-col-row.  But I
think that posn-col-row should pay attention to the existence of the
header line, so I fixed that as well (on the emacs-23 branch).

bug#7390: 24.0.50; pos-col-row gives incorrect row

[Eli Zaretskii]

> Date: Sat, 13 Nov 2010 23:10:56 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 
> 
> I think that posn-col-row should pay attention to the existence of
> the header line

E.g., artist.el is broken in the same way as picture.el, when there's
a header line in the window where you draw.

Many users of posn-col-row only use it to compute the difference
between two rows -- those will be unaffected by my change in
posn-col-row.

So I still maintain that the fix in posn-col-row should stay in the
emacs-23 branch.

bug#7390: 24.0.50; pos-col-row gives incorrect row

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

> I could revert my change to emacs-23 branch and install it on the
> trunk instead.  (Although I cannot see how this could hurt existing
> code -- did you see Lisp code that actually depends on this?)
>
> E.g., artist.el is broken in the same way as picture.el, when there's
> a header line in the window where you draw.
>
> Many users of posn-col-row only use it to compute the difference
> between two rows -- those will be unaffected by my change in
> posn-col-row.
>
> So I still maintain that the fix in posn-col-row should stay in the
> emacs-23 branch.

OK, but then this should be documented in etc/NEWS, just in case there
is someone who cares about the exact behavior of posn-col-row.  Would
you mind doing that?

Also, we have to remember to change posn-col-row again, when we fix the
deeper problem---the X and Y pixel coordinate mismatch that I described
earlier.

bug#7390: 24.0.50; pos-col-row gives incorrect row

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Cc: 7390@debbugs.gnu.org
> Date: Sat, 13 Nov 2010 16:37:35 -0500
> 
> > So I still maintain that the fix in posn-col-row should stay in the
> > emacs-23 branch.
> 
> OK, but then this should be documented in etc/NEWS, just in case there
> is someone who cares about the exact behavior of posn-col-row.  Would
> you mind doing that?

Done.

> Also, we have to remember to change posn-col-row again, when we fix the
> deeper problem---the X and Y pixel coordinate mismatch that I described
> earlier.

Yes, it's a mess.  How about filing a bug report about this?  That's
the best way not to forget, I think.

bug#7390: 24.0.50; pos-col-row gives incorrect row

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

>> Also, we have to remember to change posn-col-row again, when we fix the
>> deeper problem---the X and Y pixel coordinate mismatch that I described
>> earlier.
>
> Yes, it's a mess.  How about filing a bug report about this?  That's
> the best way not to forget, I think.

I've just changed this in the trunk.

bug#7390: 24.0.50; pos-col-row gives incorrect row

[YAMAMOTO Mitsuharu]

>>>>> On Sat, 13 Nov 2010 23:10:56 +0200, Eli Zaretskii <eliz@gnu.org> said:

> I see that Chong modified picture.el to not use posn-col-row.  But I
> think that posn-col-row should pay attention to the existence of the
> header line, so I fixed that as well (on the emacs-23 branch).

Perhaps `header-line-height' should be enclosed with
(with-current-buffer (window-buffer window) ...) as in the case of
`line-spacing'?

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

bug#7390: 24.0.50; pos-col-row gives incorrect row

[Eli Zaretskii]

> Date: Sun, 12 Dec 2010 16:15:04 +0900
> From: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
> 
> >>>>> On Sat, 13 Nov 2010 23:10:56 +0200, Eli Zaretskii <eliz@gnu.org> said:
> 
> > I see that Chong modified picture.el to not use posn-col-row.  But I
> > think that posn-col-row should pay attention to the existence of the
> > header line, so I fixed that as well (on the emacs-23 branch).
> 
> Perhaps `header-line-height' should be enclosed with
> (with-current-buffer (window-buffer window) ...) as in the case of
> `line-spacing'?

What would be the use-cases where this is necessary?

bug#7390: 24.0.50; pos-col-row gives incorrect row

[YAMAMOTO Mitsuharu]

>>>>> On Sun, 12 Dec 2010 02:29:14 -0500, Eli Zaretskii <eliz@gnu.org> said:

>> Perhaps `header-line-height' should be enclosed with
>> (with-current-buffer (window-buffer window) ...) as in the case of
>> `line-spacing'?

> What would be the use-cases where this is necessary?

I don't know.  But I don't think posn-* functions assume the buffer of
the window containing the given position is the current buffer.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

bug#7390: 24.0.50; pos-col-row gives incorrect row

[Eli Zaretskii]

> Date: Sun, 12 Dec 2010 16:37:56 +0900
> From: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
> Cc: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>,
> 	7390@debbugs.gnu.org
> 
> >>>>> On Sun, 12 Dec 2010 02:29:14 -0500, Eli Zaretskii <eliz@gnu.org> said:
> 
> >> Perhaps `header-line-height' should be enclosed with
> >> (with-current-buffer (window-buffer window) ...) as in the case of
> >> `line-spacing'?
> 
> > What would be the use-cases where this is necessary?
> 
> I don't know.  But I don't think posn-* functions assume the buffer of
> the window containing the given position is the current buffer.

You are right.  I fixed that.

bug#7390: 24.0.50; pos-col-row gives incorrect row

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

> As a matter of fact, this problem is present in Emacs 23.2 as well,
> and in the current pretest of Emacs 23.3.
>
> An easier way to reproduce is this:
>
>   emacs -Q
>   C-x b foo RET
>   M-: (setq header-line-format "foo") RET
>   M-x picture-mode RET
>
> Now type 2 lines of text, and click the mouse on the first line.

I've checked a fix for picture-mouse-set-point into the emacs-23 branch.

The underlying situation is currently somewhat unsatisfactory---the
pixel position of a mouse-event in the text area counts X from the start
of the text area (excluding margins and fringes), and Y from the top of
the window (including the header line).

This arises from some internal redisplay engine semantics, and is not
really intuitive.  Rightfully, the header-line should be excluded from
the pixel Y coordinates.  It's not feasible to change this for Emacs 23,
since existing code might depend on the current behavior.  But we can
contemplate changing this in Emacs 24.

(Such a change will not affect the picture-mouse-set-point fix.)

bug#7390: 24.0.50; pos-col-row gives incorrect row

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Cc: Drew Adams <drew.adams@oracle.com>, 7390@debbugs.gnu.org
> Date: Sat, 13 Nov 2010 16:13:14 -0500
> 
> I've checked a fix for picture-mouse-set-point into the emacs-23 branch.
> 
> The underlying situation is currently somewhat unsatisfactory---the
> pixel position of a mouse-event in the text area counts X from the start
> of the text area (excluding margins and fringes), and Y from the top of
> the window (including the header line).
> 
> This arises from some internal redisplay engine semantics, and is not
> really intuitive.  Rightfully, the header-line should be excluded from
> the pixel Y coordinates.  It's not feasible to change this for Emacs 23,
> since existing code might depend on the current behavior.  But we can
> contemplate changing this in Emacs 24.

I could revert my change to emacs-23 branch and install it on the
trunk instead.  (Although I cannot see how this could hurt existing
code -- did you see Lisp code that actually depends on this?)

bug#7390: 24.0.50; pos-col-row gives incorrect row

[Drew Adams]

Thanks to you both for fixing this so quickly.

> The underlying situation is currently somewhat unsatisfactory---the
> pixel position of a mouse-event in the text area counts X 
> from the start of the text area (excluding margins and fringes),
> and Y from the top of the window (including the header line).

Yes, that's an inconsistency.

But what do we want "pixel" position to mean? Both (1) a position within the
text area and (2) a position overall (including things like scroll bars and
header lines can be useful.

We should not sacrifice one for the other. What you need in terms of a position
depends on what you are trying to do.  It seems we need either two different
functions (for each such positioning function) or an argument that determines
the behavior.

> This arises from some internal redisplay engine semantics, and is not
> really intuitive.  Rightfully, the header-line should be excluded from
> the pixel Y coordinates.

No, I don't think there is any such "rightfully".  Both behaviors are important
and useful (position within text area and position within total frame/window
including non-text decorations).  We should not decide that one of them is
"right".

But each of them needs to be consistent.

> It's not feasible to change this for Emacs 23,
> since existing code might depend on the current behavior.  But we can
> contemplate changing this in Emacs 24.
> 
> (Such a change will not affect the picture-mouse-set-point fix.)

Let's contemplate coming up with functions that give both kinds of position, not
just one.

bug#7390: 24.0.50; pos-col-row gives incorrect row

[Eli Zaretskii]

> From: "Drew Adams" <drew.adams@oracle.com>
> Cc: <7390@debbugs.gnu.org>
> Date: Sat, 13 Nov 2010 13:38:01 -0800
> 
> But what do we want "pixel" position to mean? Both (1) a position within the
> text area and (2) a position overall (including things like scroll bars and
> header lines can be useful.

You already have (2) in the mouse event itself.  The posn-* functions
are about (1).

bug#7390: 24.0.50; pos-col-row gives incorrect row

[Chong Yidong]

Chong Yidong <cyd@stupidchicken.com> writes:

> I've checked a fix for picture-mouse-set-point into the emacs-23 branch.

Hmm; I took a closer look at your fix, and it doesn't seem quite right.
If you make an abnormally tall header-line, e.g.

(setq header-line-format (propertize "foo" 'face '(:height 5.0)))

then this new code gives the wrong value.  What's needed is to subtract
the pixel Y position of the top of the text area from the pixel Y
position of the top of the window.

bug#7390: 24.0.50; pos-col-row gives incorrect row

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Cc: Drew Adams <drew.adams@oracle.com>, 7390@debbugs.gnu.org
> Date: Sat, 13 Nov 2010 16:44:04 -0500
> 
> Chong Yidong <cyd@stupidchicken.com> writes:
> 
> > I've checked a fix for picture-mouse-set-point into the emacs-23 branch.
> 
> Hmm; I took a closer look at your fix, and it doesn't seem quite right.
> If you make an abnormally tall header-line, e.g.
> 
> (setq header-line-format (propertize "foo" 'face '(:height 5.0)))
> 
> then this new code gives the wrong value.

That's expected: posn-col-row is documented to return an
approximation, based on the assumption that all the lines are of the
canonical height.  Your example breaks that assumption.  Similar
problems will happen if one of the lines above the mouse click uses an
unusually large font.

bug#7390: 24.0.50; pos-col-row gives incorrect row

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

>> (setq header-line-format (propertize "foo" 'face '(:height 5.0)))
>>
>> then this new code gives the wrong value.
>
> That's expected: posn-col-row is documented to return an
> approximation, based on the assumption that all the lines are of the
> canonical height.  Your example breaks that assumption.  Similar
> problems will happen if one of the lines above the mouse click uses an
> unusually large font.

I know, but posn-col-row, when called for a text area event position,
ought to give the (approximate) row/column number counting from the top
of the text area.