Subject: Re: bug#41441: 26.3; mhtml misbehaving font-lock extend region can cause Emacs to hang

From: Anders Lindgren <[email-masked]>
Date: Wed, 27 May 2020 22:40:55 +0200

Hi!

To me it's obviously a bug. First, the name of the function contains
"extend", not "change". Secondly, the loop in
font-lock-default-fontify-region is written so that it loops until all
functions in font-lock-extend-region-functions are satisfied (which is also
documented). To me, when code is written this way it's clear that monotonic
growth of the region is assumed (even though that isn't explicitly stated
in the documentation).

Anyway, I took a look at the mhtml code today. This is from
mhtml--extend-font-lock-region:

      (goto-char font-lock-beg)
      (unless (eobp)
        (forward-char))
      (setq font-lock-beg (previous-single-property-change
                           (point) 'mhtml-submode nil
                           (line-beginning-position)))

Without having a deep knowledge of this, I don't think this do the correct
thing when font-lock-beg is at the end of a line (as it is when the line is
empty). The problem is that the code move forward one character (the
newline) and then search backward with line-beginning-position (i.e. the
beginning of the line following the original font-lock-beg) as limit,
effectively shrinking the region.

On Mon, May 25, 2020 at 6:11 PM Tom Tromey <[email-masked]> wrote:

> Anders> * Fix the problem in `mhtml--extend-font-lock-region`.
>
> It's not entirely clear to me that (1) this is a bug, or (2) that it can
> be changed.
>
> My recollection is that mhtml mode has to shrink the region in some
> cases, because we don't want font-locking to extend beyond the end of a
> sub-mode.
>
> For example consider things like
>
> <script>some js here;</script><p>more html here</p>
>
> Here, we want to font-lock the body of the script using one set of
> rules, and the rest with another set.
>
> Looking at mhtml--submode-fontify-region, though, I wonder if maybe
> region extension isn't needed at all, since that function seems to
> handle sub-mode region boundaries.  So I guess that is one
> experiment that could be done.
>
> Anders> One minor mode that, when enabled, would cause Emacs to hang
> Anders> with the above buffer is
> Anders> https://github.com/Lindydancer/char-font-lock (this package
> Anders> highlights incorrect whitespace).
>
> Maybe this mode could be changed instead.

Unfortunately, with the current font-lock interface, it's not. (Well, at
least I haven't figured out a way to do it, and I've been writing Emacs
packages for 25 years.)

The problem is that when the "expand" function is called, the functions has
no way of knowing whether it's the first time it has been called, or if it
has been called repeatedly. Besides, what should it do if there is another
expand function that has adjusted the region in the opposite order? Who has
the right of way?

Anyway, it's not just a problem for this mode, but for all font-lock
packages that expand the region.

    -- Anders

> Tom