Side effects in `tooltip-show'

Side effects in `tooltip-show'

[Po Lu]

I'd like to install a fix for a regression in the release branch: since
`equal-including-properties' is used in Emac 28 to compare tooltip
strings with their previous values, this branch in tooltip-show-help is
never reached:

	      ((equal-including-properties previous-help msg)
	       ;; Same help as before (but possibly the mouse has moved).
	       ;; Keep what we have.
	       )

Because tooltip-show mutates the text properties of any string passed to
it.  This causes a lot of flicker when the mouse is moved around.

The fix is to copy the string passed to `tooltip-show' before modifying
its text properties, like this:

diff --git a/lisp/tooltip.el b/lisp/tooltip.el
index 1cf16fdb5d..142c16a336 100644
--- a/lisp/tooltip.el
+++ b/lisp/tooltip.el
@@ -252,17 +252,18 @@ tooltip-show
 	    (setf (alist-get 'border-color params) fg))
 	  (when (stringp bg)
 	    (setf (alist-get 'background-color params) bg))
-          ;; Use non-nil APPEND argument below to avoid overriding any
-          ;; faces used in our TEXT.  Among other things, this allows
-          ;; tooltips to use the `help-key-binding' face used in
-          ;; `substitute-command-keys' substitutions.
-          (add-face-text-property 0 (length text) 'tooltip t text)
-          (x-show-tip text
-		      (selected-frame)
-		      params
-		      tooltip-hide-delay
-		      tooltip-x-offset
-		      tooltip-y-offset))
+          (let ((copy (copy-sequence text)))
+            ;; Use non-nil APPEND argument below to avoid overriding any
+            ;; faces used in our TEXT.  Among other things, this allows
+            ;; tooltips to use the `help-key-binding' face used in
+            ;; `substitute-command-keys' substitutions.
+            (add-face-text-property 0 (length text) 'tooltip t copy)
+            (x-show-tip copy
+		        (selected-frame)
+		        params
+		        tooltip-hide-delay
+		        tooltip-x-offset
+		        tooltip-y-offset)))
       (error
        (message "Error while displaying tooltip: %s" error)
        (sit-for 1)

Does anyone mind if I install this change on the release branch?

Re: Side effects in `tooltip-show'

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Date: Mon, 17 Jan 2022 09:30:16 +0800
> 
> I'd like to install a fix for a regression in the release branch: since
> `equal-including-properties' is used in Emac 28 to compare tooltip
> strings with their previous values, this branch in tooltip-show-help is
> never reached:
> 
> 	      ((equal-including-properties previous-help msg)
> 	       ;; Same help as before (but possibly the mouse has moved).
> 	       ;; Keep what we have.
> 	       )
> 
> Because tooltip-show mutates the text properties of any string passed to
> it.  This causes a lot of flicker when the mouse is moved around.
> 
> The fix is to copy the string passed to `tooltip-show' before modifying
> its text properties, like this:

I'm not happy about consing a new string each time we have a help-echo
event.  That'd definitely increase the GC pressure, which is
undesirable.

How about using 'equal' to compare the strings instead?  What are the
chances that two help-echo strings of 2 different tooltips will
compare equal without the properties, but not with the properties?

Re: Side effects in `tooltip-show'

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> I'm not happy about consing a new string each time we have a help-echo
> event.  That'd definitely increase the GC pressure, which is
> undesirable.
>
> How about using 'equal' to compare the strings instead?  What are the
> chances that two help-echo strings of 2 different tooltips will
> compare equal without the properties, but not with the properties?

That's fine by me as well, but the resulting minor snafu where the
tooltip might not be updated if the string contents are identical should
be documented somewhere, I think.

Thanks.

Re: Side effects in `tooltip-show'

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Mon, 17 Jan 2022 21:00:46 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > How about using 'equal' to compare the strings instead?  What are the
> > chances that two help-echo strings of 2 different tooltips will
> > compare equal without the properties, but not with the properties?
> 
> That's fine by me as well, but the resulting minor snafu where the
> tooltip might not be updated if the string contents are identical should
> be documented somewhere, I think.

In the comments to the code that makes  these comparisons, I think.

Re: Side effects in `tooltip-show'

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Po Lu <luangruo@yahoo.com>
>> Cc: emacs-devel@gnu.org
>> Date: Mon, 17 Jan 2022 21:00:46 +0800
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > How about using 'equal' to compare the strings instead?  What are the
>> > chances that two help-echo strings of 2 different tooltips will
>> > compare equal without the properties, but not with the properties?
>> 
>> That's fine by me as well, but the resulting minor snafu where the
>> tooltip might not be updated if the string contents are identical should
>> be documented somewhere, I think.
>
> In the comments to the code that makes  these comparisons, I think.

Thanks.  Would you mind if I installed this on the release branch?

diff --git a/lisp/tooltip.el b/lisp/tooltip.el
index 1cf16fdb5d..1355eecfcd 100644
--- a/lisp/tooltip.el
+++ b/lisp/tooltip.el
@@ -383,9 +383,12 @@ tooltip-show-help
 	       ;; Cancel display.  This also cancels a delayed tip, if
 	       ;; there is one.
 	       (tooltip-hide))
-	      ((equal-including-properties previous-help msg)
-	       ;; Same help as before (but possibly the mouse has moved).
-	       ;; Keep what we have.
+	      ((equal previous-help msg)
+	       ;; Same help as before (but possibly the mouse has
+	       ;; moved or the text properties have changed).  Keep
+	       ;; what we have.  If only text properties have changed,
+	       ;; the tooltip won't be updated, but that shouldn't
+	       ;; occur often enough to be noticable.
 	       )
 	      (t
 	       ;; A different help.  Remove a previous tooltip, and

Re: Side effects in `tooltip-show'

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Mon, 17 Jan 2022 21:29:53 +0800
> 
> Thanks.  Would you mind if I installed this on the release branch?

That's okay, but:

> diff --git a/lisp/tooltip.el b/lisp/tooltip.el
> index 1cf16fdb5d..1355eecfcd 100644
> --- a/lisp/tooltip.el
> +++ b/lisp/tooltip.el
> @@ -383,9 +383,12 @@ tooltip-show-help
>  	       ;; Cancel display.  This also cancels a delayed tip, if
>  	       ;; there is one.
>  	       (tooltip-hide))
> -	      ((equal-including-properties previous-help msg)
> -	       ;; Same help as before (but possibly the mouse has moved).
> -	       ;; Keep what we have.
> +	      ((equal previous-help msg)
> +	       ;; Same help as before (but possibly the mouse has
> +	       ;; moved or the text properties have changed).  Keep
> +	       ;; what we have.  If only text properties have changed,
> +	       ;; the tooltip won't be updated, but that shouldn't
> +	       ;; occur often enough to be noticable.

I'd say "that shouldn't occur", period.

Re: Side effects in `tooltip-show'

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Po Lu <luangruo@yahoo.com>
>> Cc: emacs-devel@gnu.org
>> Date: Mon, 17 Jan 2022 21:29:53 +0800
>> 
>> Thanks.  Would you mind if I installed this on the release branch?
>
> That's okay, but:
>
>> diff --git a/lisp/tooltip.el b/lisp/tooltip.el
>> index 1cf16fdb5d..1355eecfcd 100644
>> --- a/lisp/tooltip.el
>> +++ b/lisp/tooltip.el
>> @@ -383,9 +383,12 @@ tooltip-show-help
>>  	       ;; Cancel display.  This also cancels a delayed tip, if
>>  	       ;; there is one.
>>  	       (tooltip-hide))
>> -	      ((equal-including-properties previous-help msg)
>> -	       ;; Same help as before (but possibly the mouse has moved).
>> -	       ;; Keep what we have.
>> +	      ((equal previous-help msg)
>> +	       ;; Same help as before (but possibly the mouse has
>> +	       ;; moved or the text properties have changed).  Keep
>> +	       ;; what we have.  If only text properties have changed,
>> +	       ;; the tooltip won't be updated, but that shouldn't
>> +	       ;; occur often enough to be noticable.
>
> I'd say "that shouldn't occur", period.

Thanks, I will install it that way.

Re: Side effects in `tooltip-show'

[Pankaj Jangid]

I am getting this debugger pop-up again and again. Especially after
starting Gnus.

--8<---------------cut here---------------start------------->8---
Debugger entered--Lisp error: (void-variable haiku-use-system-tooltips)
  tooltip-show-help(nil)
--8<---------------cut here---------------end--------------->8---

Is this related to recent changes?

Re: Side effects in `tooltip-show'

[Po Lu]

Pankaj Jangid <pankaj@codeisgreat.org> writes:

> I am getting this debugger pop-up again and again. Especially after
> starting Gnus.
>
> Debugger entered--Lisp error: (void-variable haiku-use-system-tooltips)
>   tooltip-show-help(nil)
>
> Is this related to recent changes?

Thanks, now fixed.