bug#36550: mouse-face overlay calculation error

[Eli Zaretskii]

> From: Linus Källberg <linus.kallberg@outlook.com>
> CC: "larsi@gnus.org" <larsi@gnus.org>, "36550@debbugs.gnu.org"
>       <36550@debbugs.gnu.org>
> Date: Sat, 13 Jul 2019 19:49:07 +0000
> 
> > But that's not what mouse-face means and does.  It highlights
> > mouse-sensitive text, and thus it should NOT include a newline,
> > because a newline does not have a glyph in the buffer text.  That's
> > why you see the 1st character on the next line highlighted: it's the
> > next character after the newline, and since the newline is absent, it
> > gets highlighted instead, because when character positions are not
> > monotonous with screen coordinates, we have no alternative but
> > highlight that next character.
> 
> Are you saying that the mouse-face property should not be used on 
> overlays that contain newline characters, period, or simply those that 
> have a newline character as their *last* character?

The latter.

> I understand that a newline character cannot be clicked or highlighted 
> if it has no glyph in the text, but why can't it then simply not be 
> highlighted?

Because the algorithm to find what is the last glyph to be highlighted
is very complicated (since it cannot rely on buffer positions changing
monotonously with screen coordinates) and in this case it yields a
result that looks wrong.  If someone wants to find how to change the
algorithm so that it also covers this niche use case, feel free.

> is it really intended, in the sense that something else would break
> if it was "fixed"?

It depends on the fix.  I cannot tell until I see a proposal for such
a fix.  The existing code relies on some properties of glyphs as they
are laid out for display, and one of those properties is that the
screen line's end position is the position of the first character on
the next line.  That's why you see what you see in this case.

> >> or possibly one character further to the right (as if there was an
> >> imaginary space character inserted there).
> > 
> > Can't do that, because that imaginary character doesn't exist, and
> > therefore we cannot possible access its buffer position.
> 
> But doesn't it already do that? I use an Emacs theme that underlines 
> highlighted text, and when an overlay contains a newline character 
> (anywhere, not necessarily at the end), there is always one extra 
> "imaginary" character underlined after the last character before the 
> line break.

This is something specific to mouse-face, because unlike other faces,
it works with characters on display, not in the buffer.

> In this example, there is one extra underlined character after "foo":
> 
> (progn
>    (load-theme 'wombat t)
>    (let ((point (point)))
>      (insert "foo\nbar")
>      (let ((o (make-overlay point (point))))
>        (overlay-put o 'mouse-face 'highlight))))

Yes, as expected.

> >> In recentf, the newline after the file name is included in each link so
> >> that if point is positioned right after the last character -- which
> >> happens, e.g., if one i-searches for a file extension -- one can simply
> >> press enter to open the file (as said in the commit message).
> > 
> > But pressing Enter doesn't require mouse-face, does it?  It requires
> > an overlay with a suitable keymap property, right?
> 
> Exactly, I guess the button widget simply uses the same overlay for 
> everything.

Yes, I suggested to use a separate overlay for that.

> > If this fixes the issue, it's fine with me.  However, I think we
> > should have a comment there explaining why we add this space
> > character.
> 
> Yes, a comment should be added. However, I would prefer fixing the real 
> problem, which, if not in the display code, might be in the 
> implementation of the button widget.

Fine with me, if someone wants to work on that.