bug#60841: 30.0.50; kill-ring-save pauses despite region being highlighted

[Eli Zaretskii]

> From: Kévin Le Gouguec <kevin.legouguec@gmail.com>
> Date: Mon, 16 Jan 2023 00:38:02 +0100
> 
> The docstring says that this is done "if there is currently no active
> region highlighting"; in practice, this translates to:
> 
>       ;; Swap point-and-mark quickly so as to show the region that
>       ;; was selected.  Don't do it if the region is highlighted.
>       (when (and (numberp copy-region-blink-delay)
>                  (> copy-region-blink-delay 0)
>                  (or (not (region-active-p))                  ; (a)
>                      (not (face-background 'region nil t))))  ; (b)
> 
> IOW "active region highlighting" means "(a) an active region, and (b)
> any background for the region face".  face-background, however, is not
> enough to asses (b); consider, from emacs -Q:
> 
> M-: (custom-set-faces '(region ((t (:foreground "blue" :inverse-video t)))))
> C-x h
> M-w
> 
> In this situation, the region face has a clearly visible background, yet
> indicate-copied-region still behaves as if the region is not
> "highlighted", and triggers the (harmless and entirely interruptible)
> point-and-mark swap followed by a pause.

Yes, the existing conditions are a bit naïve.

> The attached patch handles this foreground + inverse-video switcheroo.
> Not sure how many themes actually do that in the wild, so I'd understand
> if there wasn't much enthusiasm for applying.  That face-background
> check has been with us since 2004; haven't found any hint in the BTS
> that anyone else has been bothered by this false negative.

IMNSHO, this just makes another mini-step (albeit in the right
direction), instead of going all the way.  There are other face
attributes that can make the region stand out on display: what about
underline or italics, for example?  And vice versa: the face could
have a background that is identical or almost identical to the default
face.

We could keep adding conditions for more and more face attributes, one
by one.  But it would be better to have there a test which would tell
us whether the region face is "visually different" from the default
face.  Can we do something like that?

Alternatively, we could add a user option to make the swap
unconditional, because maybe some users would prefer that to splitting
hair in this case.  Then we could stop worrying about all those fine
differences.

Thanks.