bug#51012: 29.0.50; Problems with emojis and vertical-motion

bug#51012: 29.0.50; Problems with emojis and vertical-motion

[Lars Ingebrigtsen]

The following signals an error:

(with-temp-buffer
  (insert "🏳️‍🌈")
  (vertical-motion 1))

Args out of range: 0

In some circumstances (when edebug is in action or it's running from an
async callback), it'll freeze Emacs altogether, so it's not totally safe
to eval.

I'm assuming it has something to do with the new emoji stuff, but I
haven't tried to debug.


In GNU Emacs 29.0.50 (build 9, x86_64-pc-linux-gnu, GTK+ Version 3.24.24, cairo version 1.16.0)
 of 2021-10-04 built on elva
Repository revision: 3f14418b1152e8455b0fcd28f496187dec9b4257
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Debian GNU/Linux 11 (bullseye)

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS
X11 XDBE XIM XPM GTK3 ZLIB


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

bug#51012: 29.0.50; Problems with emojis and vertical-motion

[Robert Pluim]

>>>>> On Mon, 04 Oct 2021 17:02:58 +0200, Lars Ingebrigtsen <larsi@gnus.org> said:

    Lars> The following signals an error:

    Lars> (with-temp-buffer
    Lars>   (insert "🏳️‍🌈")
    Lars>   (vertical-motion 1))

    Lars> Args out of range: 0

    Lars> In some circumstances (when edebug is in action or it's running from an
    Lars> async callback), it'll freeze Emacs altogether, so it's not totally safe
    Lars> to eval.

    Lars> I'm assuming it has something to do with the new emoji stuff, but I
    Lars> haven't tried to debug.

Gaah. What are the three most important problems in computer science
today?

1. Dependencies in code, especially when moving it
2. Off by one errors

This fixes it for me:

diff --git a/src/font.c b/src/font.c
index 82a1dffc01..83f0f8296a 100644
--- a/src/font.c
+++ b/src/font.c
@@ -3918,7 +3918,7 @@ font_range (ptrdiff_t pos, ptrdiff_t pos_byte, ptrdiff_t *limit,
 	    val = XCAR (val);
 	  else if (VECTORP (val))
 	    val = AREF (val, 0);
-	  font_object = font_for_char (face, XFIXNAT (val), pos - 1, string);
+	  font_object = font_for_char (face, XFIXNAT (val), pos, string);
 	}
     }
 

bug#51012: 29.0.50; Problems with emojis and vertical-motion

[Lars Ingebrigtsen]

Robert Pluim <rpluim@gmail.com> writes:

> 1. Dependencies in code, especially when moving it
> 2. Off by one errors
>
> This fixes it for me:

That was quick.  :-)

> -	  font_object = font_for_char (face, XFIXNAT (val), pos - 1, string);
> +	  font_object = font_for_char (face, XFIXNAT (val), pos, string);

I can confirm that this fixes the issue for me, too.

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

bug#51012: 29.0.50; Problems with emojis and vertical-motion

[Robert Pluim]

tags 51012 fixed
close 51012 28.1
quit

>>>>> On Mon, 04 Oct 2021 18:51:01 +0200, Lars Ingebrigtsen <larsi@gnus.org> said:

    Lars> Robert Pluim <rpluim@gmail.com> writes:
    >> 1. Dependencies in code, especially when moving it
    >> 2. Off by one errors
    >> 
    >> This fixes it for me:

    Lars> That was quick.  :-)

It was one of those "Uhm, that was dumb Robert, better save some face
by getting a move on" moments.

    >> -	  font_object = font_for_char (face, XFIXNAT (val), pos - 1, string);
    >> +	  font_object = font_for_char (face, XFIXNAT (val), pos, string);

    Lars> I can confirm that this fixes the issue for me, too.

Thanks for testing.

Closing.
Committed as 9750e78202

bug#51012: 29.0.50; Problems with emojis and vertical-motion

[Lars Ingebrigtsen]

Robert Pluim <rpluim@gmail.com> writes:

> It was one of those "Uhm, that was dumb Robert, better save some face
> by getting a move on" moments.

:-)

I'm cherry-picking the patch for master, too, because it locks up Emacs
in some circumstances (and waiting until the next merge is too long).

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

bug#51012: 29.0.50; Problems with emojis and vertical-motion

[Glenn Morris]

Lars Ingebrigtsen wrote:

> I'm cherry-picking the patch for master, too, because it locks up Emacs
> in some circumstances (and waiting until the next merge is too long).

Anyone should free to merge emacs-28 to master at any time.

bug#51012: 29.0.50; Problems with emojis and vertical-motion

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Date: Mon, 04 Oct 2021 18:43:53 +0200
> Cc: 51012@debbugs.gnu.org
> 
> This fixes it for me:
> 
> diff --git a/src/font.c b/src/font.c
> index 82a1dffc01..83f0f8296a 100644
> --- a/src/font.c
> +++ b/src/font.c
> @@ -3918,7 +3918,7 @@ font_range (ptrdiff_t pos, ptrdiff_t pos_byte, ptrdiff_t *limit,
>  	    val = XCAR (val);
>  	  else if (VECTORP (val))
>  	    val = AREF (val, 0);
> -	  font_object = font_for_char (face, XFIXNAT (val), pos - 1, string);
> +	  font_object = font_for_char (face, XFIXNAT (val), pos, string);
>  	}
>      }

Sounds obvious to me.  Why was there a minus there in the first place?
some remnant of debugging or the previous version?

bug#51012: 29.0.50; Problems with emojis and vertical-motion

[Robert Pluim]

>>>>> On Mon, 04 Oct 2021 20:04:50 +0300, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Robert Pluim <rpluim@gmail.com>
    >> Date: Mon, 04 Oct 2021 18:43:53 +0200
    >> Cc: 51012@debbugs.gnu.org
    >> 
    >> This fixes it for me:
    >> 
    >> diff --git a/src/font.c b/src/font.c
    >> index 82a1dffc01..83f0f8296a 100644
    >> --- a/src/font.c
    >> +++ b/src/font.c
    >> @@ -3918,7 +3918,7 @@ font_range (ptrdiff_t pos, ptrdiff_t pos_byte, ptrdiff_t *limit,
    >> val = XCAR (val);
    >> else if (VECTORP (val))
    >> val = AREF (val, 0);
    >> -	  font_object = font_for_char (face, XFIXNAT (val), pos - 1, string);
    >> +	  font_object = font_for_char (face, XFIXNAT (val), pos, string);
    >> }
    >> }

    Eli> Sounds obvious to me.  Why was there a minus there in the first place?
    Eli> some remnant of debugging or the previous version?

The previous version ran that code after pos had been incremented. So
when I moved it up I forgot to undo the adjustment of 'pos'.

Robert
--