bug#69860: 29.2; ERC 5.6-git: erc: Incorrect face formatting applied for fg=99 bg=x (irccontrols module with erc-interpret-mirc-color=t)

bug#69860: 29.2; ERC 5.6-git: erc: Incorrect face formatting applied for fg=99 bg=x (irccontrols module with erc-interpret-mirc-color=t)

[Alcor via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 7568 bytes --]


On Emacs 29.2, using erc-5.6snapshot0.20240310.61129:

1. Enable the irccontrols module (for erc)
2. Enable `erc-interpret-mirc-color'
3. Login onto any IRC network, any channel, or perform a /query session
4. Either send, or have someone send you the string ^C99,4Text with a
red background^C (Key sequence: C-q C-c 99,4Text with a red background
C-q C-c)

According to https://modern.ircdocs.horse/formatting.html the text
*should* get formatted with a default foreground, and a red background. The
observed behavior - however - is that the text is formatted with multiple
background "bg" faces: (erc-control-default-bg bg:erc-color-face4
erc-input-face).

A screenshot illustrating the issue is attached to this message.


In GNU Emacs 29.2 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.24.20,
 cairo version 1.16.0) of 2024-03-13 built on lcy02-amd64-051
Repository revision: 900dc1b4bf3011e685a1ec7d7ce4dcd0262ec880
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12201001
System Description: Ubuntu 22.04.4 LTS

Configured using:
 'configure --prefix=/snap/emacs/current/usr --with-x-toolkit=gtk3
 --without-xaw3d --with-modules --with-cairo
 --with-native-compilation=aot --without-pgtk --with-xinput2
 --with-tree-sitter --with-json
 'CFLAGS=-isystem/build/emacs/parts/emacs/install/usr/include
 -isystem/build/emacs/parts/emacs/install/usr/include/x86_64-linux-gnu
 -isystem/build/emacs/stage/usr/include -O2'
 'CPPFLAGS=-isystem/build/emacs/parts/emacs/install/usr/include
 -isystem/build/emacs/parts/emacs/install/usr/include/x86_64-linux-gnu
 -isystem/build/emacs/stage/usr/include'
 'LDFLAGS=-L/build/emacs/parts/emacs/install/lib
 -L/build/emacs/parts/emacs/install/usr/lib
 -L/build/emacs/parts/emacs/install/lib/x86_64-linux-gnu
 -L/build/emacs/parts/emacs/install/usr/lib/x86_64-linux-gnu
 -L/build/emacs/stage/usr/lib''

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

Important settings:
  value of $LANG: de_DE.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Group

Minor modes in effect:
  gnus-undo-mode: t
  shell-dirtrack-mode: t
  erc-ring-mode: t
  erc-notifications-mode: t
  erc-netsplit-mode: t
  erc-menu-mode: t
  erc-list-mode: t
  erc-irccontrols-mode: t
  erc-keep-place-mode: t
  erc-move-to-prompt-mode: t
  erc-readonly-mode: t
  erc-scrolltobottom-mode: t
  erc-imenu-mode: t
  erc-button-mode: t
  erc-fill-mode: t
  erc-stamp-mode: t
  erc-bufbar-mode: t
  erc-track-mode: t
  erc-match-mode: t
  erc-autojoin-mode: t
  erc-autoaway-mode: t
  recentf-mode: t
  pixel-scroll-precision-mode: t
  minibuffer-depth-indicate-mode: t
  global-whitespace-mode: t
  global-goto-address-mode: t
  goto-address-mode: t
  global-auto-revert-mode: t
  fido-vertical-mode: t
  icomplete-vertical-mode: t
  icomplete-mode: t
  fido-mode: t
  erc-networks-mode: t
  desktop-save-mode: t
  windmove-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  buffer-read-only: t
  column-number-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Features:
(shadow emacsbug mailalias smtpmail ispell cl-print help-fns radix-tree
sort smiley gnus-cite mail-extr textsec uni-scripts idna-mapping
ucs-normalize uni-confusable textsec-check gnus-bcklg gnus-async gnus-ml
nndraft nnmh nnfolder utf-7 rfc2104 network-stream nsm gnus-agent
gnus-srvr gnus-score score-mode nnvirtual gnus-msg nntp gnus-cache
display-line-numbers org-element org-persist org-id org-refile avl-tree
generator oc-basic ol-eww eww url-queue mm-url ol-rmail ol-mhe ol-irc
ol-info ol-gnus nnselect gnus-art mm-uu mml2015 mm-view mml-smime smime
gnutls dig gnus-sum shr pixel-fill kinsoku url-file svg dom gnus-group
gnus-undo gnus-start gnus-dbus gnus-cloud nnimap nnmail mail-source utf7
nnoo gnus-spec gnus-int gnus-range message sendmail yank-media puny
rfc822 mml mml-sec epa epg rfc6068 epg-config mm-decode mm-bodies
mm-encode mail-parse rfc2231 rfc2047 rfc2045 ietf-drums mailabbrev
gmm-utils mailheader gnus-win gnus nnheader gnus-util
text-property-search mail-utils range mm-util mail-prsvr ol-docview
doc-view jka-compr image-mode exif dired dired-loaddefs ol-bibtex bibtex
ol-bbdb ol-w3m ol-doi org-link-doi org ob ob-tangle ob-ref ob-lob
ob-table ob-exp org-macro org-src ob-comint org-pcomplete org-list
org-footnote org-faces org-entities noutline outline ob-emacs-lisp
ob-core ob-eval org-cycle org-table ol org-fold org-fold-core org-keys
oc org-loaddefs find-func cal-menu calendar cal-loaddefs org-version
org-compat org-macs tramp-cache time-stamp tramp-sh tramp tramp-loaddefs
trampver tramp-integration files-x tramp-compat shell parse-time iso8601
disp-table erc-ring erc-desktop-notifications notifications dbus xml
erc-nicks color erc-netsplit erc-menu erc-list erc-goodies erc-imenu
imenu erc-pcomplete time-date pcomplete comint ansi-osc ansi-color
erc-button erc-fill erc-stamp erc-status-sidebar erc-track erc-match
erc-join erc-autoaway leuven-dark-theme recentf tree-widget wid-edit
pixel-scroll cua-base ring mb-depth whitespace goto-addr thingatpt
autorevert filenotify icomplete erc derived format-spec erc-backend
erc-networks easy-mmode erc-common inline erc-compat pcase compat
erc-loaddefs desktop frameset cus-load windmove xdg site-start comp
comp-cstr warnings icons rx cl-extra help-mode erc-autoloads info
compat-autoloads markdown-mode-autoloads package browse-url url
url-proxy url-privacy url-expand url-methods url-history url-cookie
generate-lisp-file url-domsuf url-util mailcap url-handlers url-parse
auth-source cl-seq eieio eieio-core cl-macs password-cache json subr-x
map byte-opt gv bytecomp byte-compile url-vars cl-loaddefs cl-lib rmc
iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook
vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode register
page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind inotify lcms2 dynamic-setting system-font-setting
font-render-setting cairo move-toolbar gtk x-toolkit xinput2 x multi-tty
make-network-process native-compile emacs)

Memory information:
((conses 16 615423 61113)
 (symbols 48 32158 26)
 (strings 32 117195 5131)
 (string-bytes 1 3518684)
 (vectors 16 71836)
 (vector-slots 8 1268576 98438)
 (floats 8 591 755)
 (intervals 56 1811 223)
 (buffers 984 22))


[-- Attachment #2: Screenshot demonstrating the issue --]
[-- Type: image/png, Size: 132824 bytes --]

bug#69860: 29.2; ERC 5.6-git: erc: Incorrect face formatting applied for fg=99 bg=x (irccontrols module with erc-interpret-mirc-color=t)

[J.P.]

Alcor via General discussion about ERC <emacs-erc@gnu.org> writes:

> On Emacs 29.2, using erc-5.6snapshot0.20240310.61129:
>
> 1. Enable the irccontrols module (for erc)
> 2. Enable `erc-interpret-mirc-color'
> 3. Login onto any IRC network, any channel, or perform a /query session
> 4. Either send, or have someone send you the string ^C99,4Text with a
> red background^C (Key sequence: C-q C-c 99,4Text with a red background
> C-q C-c)
>
> According to https://modern.ircdocs.horse/formatting.html the text
> *should* get formatted with a default foreground, and a red background. The
> observed behavior - however - is that the text is formatted with multiple
> background "bg" faces: (erc-control-default-bg bg:erc-color-face4
> erc-input-face).
>
> A screenshot illustrating the issue is attached to this message.

Oof. Looks like `erc-get-fg-color-face' sets `erc-control-default-bg' in
its catch-all `cond' case. As you say, this produces:

  #("THIS TEXT IS FORMATTED"
    0 22 (font-lock-face (erc-control-default-bg bg:erc-color-face4)))

And `erc-get-bg-color-face' sets `erc-control-default-fg'. Clearly,
whoever's responsible for this outrage should be banished. (Thanks.)

bug#69860: 29.2; ERC 5.6-git: erc: Incorrect face formatting applied for fg=99 bg=x (irccontrols module with erc-interpret-mirc-color=t)

[Alcor via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 829 bytes --]

"J.P." <jp@neverwas.me> writes:

> Oof. Looks like `erc-get-fg-color-face' sets `erc-control-default-bg' in
> its catch-all `cond' case. As you say, this produces:
>
>   #("THIS TEXT IS FORMATTED"
>     0 22 (font-lock-face (erc-control-default-bg bg:erc-color-face4)))
>
> And `erc-get-bg-color-face' sets `erc-control-default-fg'. Clearly,
> whoever's responsible for this outrage should be banished. (Thanks.)

Yes, that's part of the issue. However, `erc-controls-propertize' should
also avoid adding the default fg/bg to the font lock face if n=99 to
avoid overriding an existing fg,bg setting if fg=99,bg=x or bg=99,fg=x.

This is trivially fixable and it's possible you already have a fix, but
I'm attaching a patchset that works for me FWIW, just to illustrate what
I had to change in order to make it work.

Cheers,
-A


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-typo-in-erc-get-fg-bg-color-face.patch --]
[-- Type: text/x-diff, Size: 1579 bytes --]

From 9b8a55c705f305a4aa0da85ee43741e929fb9cfc Mon Sep 17 00:00:00 2001
From: "F. Moukayed" <smfadi+emacs@gmail.com>
Date: Sun, 17 Mar 2024 16:43:36 +0000
Subject: [PATCH 1/2] Fix typo in erc-get-{fg,bg}-color-face

* lisp/erc/erc-goodies.el (erc-get-{fg,bg}-color-face): Fix wrong return value when n=99.
---
 lisp/erc/erc-goodies.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/erc/erc-goodies.el b/lisp/erc/erc-goodies.el
index da14f5b..93b888a 100644
--- a/lisp/erc/erc-goodies.el
+++ b/lisp/erc/erc-goodies.el
@@ -812,7 +812,7 @@ The value `erc-interpret-controls-p' must also be t for this to work."
       (intern (concat "bg:erc-color-face" (number-to-string n))))
      ((< 15 n 99)
       (list :background (aref erc--controls-additional-colors (- n 16))))
-     (t (erc-log (format "   Wrong color: %s" n)) 'erc-control-default-fg))))
+     (t (erc-log (format "   Wrong color: %s" n)) 'erc-control-default-bg))))
 
 (defun erc-get-fg-color-face (n)
   "Fetches the right face for foreground color N (0-15)."
@@ -828,7 +828,7 @@ The value `erc-interpret-controls-p' must also be t for this to work."
       (intern (concat "fg:erc-color-face" (number-to-string n))))
      ((< 15 n 99)
       (list :foreground (aref erc--controls-additional-colors (- n 16))))
-     (t (erc-log (format "   Wrong color: %s" n)) 'erc-control-default-bg))))
+     (t (erc-log (format "   Wrong color: %s" n)) 'erc-control-default-fg))))
 
 ;;;###autoload(autoload 'erc-irccontrols-mode "erc-goodies" nil t)
 (define-erc-module irccontrols nil
-- 
2.34.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Fix-wrong-color-rendering-when-bg-99-or-fg-99.patch --]
[-- Type: text/x-diff, Size: 998 bytes --]

From efd3fe14378a8f51dcc4946d573c5ae243778084 Mon Sep 17 00:00:00 2001
From: "F. Moukayed" <smfadi+emacs@gmail.com>
Date: Sun, 17 Mar 2024 17:16:35 +0000
Subject: [PATCH 2/2] Fix wrong color rendering when bg=99 or fg=99

* lisp/erc/erc-goodies.el (erc-controls-propertize): Avoid prepending the bg/fg face if it's defaulted (99).
---
 lisp/erc/erc-goodies.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/erc/erc-goodies.el b/lisp/erc/erc-goodies.el
index 93b888a..52bd177 100644
--- a/lisp/erc/erc-goodies.el
+++ b/lisp/erc/erc-goodies.el
@@ -1002,10 +1002,10 @@ to a region in the current buffer."
            (if underlinep
                '(erc-underline-face)
              nil)
-           (if fg
+           (if (and fg (not (equal fg 'erc-control-default-fg)))
                (list fg)
              nil)
-           (if bg
+           (if (and bg (not (equal bg 'erc-control-default-bg)))
                (list bg)
              nil))
    str)
-- 
2.34.1

bug#69860: 29.2; ERC 5.6-git: erc: Incorrect face formatting applied for fg=99 bg=x (irccontrols module with erc-interpret-mirc-color=t)

[J.P.]

[-- Attachment #1: Type: text/plain, Size: 1647 bytes --]

Alcor <alcor@tilde.club> writes:

> "J.P." <jp@neverwas.me> writes:
>
>> Oof. Looks like `erc-get-fg-color-face' sets `erc-control-default-bg' in
>> its catch-all `cond' case. As you say, this produces:
>>
>>   #("THIS TEXT IS FORMATTED"
>>     0 22 (font-lock-face (erc-control-default-bg bg:erc-color-face4)))
>>
>> And `erc-get-bg-color-face' sets `erc-control-default-fg'. Clearly,
>> whoever's responsible for this outrage should be banished. (Thanks.)
>
> Yes, that's part of the issue. However, `erc-controls-propertize' should
> also avoid adding the default fg/bg to the font lock face if n=99 to
> avoid overriding an existing fg,bg setting if fg=99,bg=x or bg=99,fg=x.

Here's my current understanding of what you're saying. When there's an
existing fg/bg combo in effect for a given span, and the parsing logic
encounters a ^Cx,99 (or a ^C99,x), the 99 shouldn't induce a clobbering
of the corresponding "incumbent" fg (or bg) face with a dedicated (and
superfluous) default face but instead trigger the _removal_ of said
incumbent face. This has the effect of falling through to honor the
relevant attributes of the underlying `erc-default-face', which ships as
a no-op. For example, given

  ^C03,08 hello ^C99,07 world

the "hello " portion should be green on yellow and " world" should be
${default-foreground} (likely black or white) on orange. Likewise, if
"^C99,07" were instead "^C04", then " world" should be red on yellow.
This behavior aligns with that exhibited by Weechat and (I'm hoping)
other popular clients. The revised patch set (attached) attempts to
mimic this in ERC. Corrections or alternatives welcome. Thanks.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-5.6-Remove-unused-faces-from-various-erc-goodies-tes.patch --]
[-- Type: text/x-patch, Size: 8709 bytes --]

From dd9302210706cc21ed1548a3a5ea769366c2e7be Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Tue, 19 Mar 2024 23:51:46 -0700
Subject: [PATCH 1/2] [5.6] Remove unused faces from various erc-goodies tests

; A note to anyone running ERC's test suite while bisecting and
; unlucky enough to land on this commit.  Apologies for the
; inconvenience.  It fails because it includes adjustments for fixes
; only introduced by the subsequent commit.  This is obviously
; objectionable but was done knowingly in order to duck the
; copyright-exemption threshold for new contributors.

* test/lisp/erc/erc-goodies-tests.el
(erc-controls-highlight--spoilers)
(erc-controls-highlight--inverse): Remove all mention of faces
`erc-control-default-fg' and `erc-control-default-bg'.
(erc-controls-highlight/default-foreground)
(erc-controls-highlight/default-background): New tests.
(Bug#69860)
---
 test/lisp/erc/erc-goodies-tests.el | 127 ++++++++++++++++++++++++++---
 1 file changed, 116 insertions(+), 11 deletions(-)

diff --git a/test/lisp/erc/erc-goodies-tests.el b/test/lisp/erc/erc-goodies-tests.el
index c8fb0544a72..7cbaa39d3f7 100644
--- a/test/lisp/erc/erc-goodies-tests.el
+++ b/test/lisp/erc/erc-goodies-tests.el
@@ -167,15 +167,13 @@ erc-controls-highlight--spoilers
        '(fg:erc-color-face1 bg:erc-color-face1))
       ;; Masked in all black.
       (erc-goodies-tests--assert-face
-       20 "BlackOnBlack" '(fg:erc-color-face1 bg:erc-color-face1)
-       '(erc-control-default-fg erc-control-default-bg))
+       20 "BlackOnBlack" '(fg:erc-color-face1 bg:erc-color-face1) nil)
       ;; Explicit "default" code ignoerd.
       (erc-goodies-tests--assert-face
-       34 "Default" '(erc-control-default-fg erc-control-default-bg)
+       34 "Default" '(erc-default-face)
        '(fg:erc-color-face1 bg:erc-color-face1))
       (erc-goodies-tests--assert-face
-       43 "END" 'erc-default-face
-       '(erc-control-default-bg erc-control-default-fg))))
+       43 "END" 'erc-default-face nil)))
   (when noninteractive
     (erc-tests-common-kill-buffers)))
 
@@ -214,17 +212,124 @@ erc-controls-highlight--inverse
        nil)
       ;; The inverse of `default' because reverse still in effect.
       (erc-goodies-tests--assert-face
-       32 "ReversedDefault" '(erc-inverse-face erc-control-default-fg
-                                               erc-control-default-bg)
+       32 "ReversedDefault" '(erc-inverse-face erc-default-face)
        '(fg:erc-color-face3 bg:erc-color-face13))
       (erc-goodies-tests--assert-face
-       49 "NormalDefault" '(erc-control-default-fg
-                            erc-control-default-bg)
+       49 "NormalDefault" '(erc-default-face)
        '(erc-inverse-face fg:erc-color-face1 bg:erc-color-face1))
       (erc-goodies-tests--assert-face
        64 "END" 'erc-default-face
-       '( erc-control-default-fg erc-control-default-bg
-          fg:erc-color-face0 bg:erc-color-face0))))
+       '(fg:erc-color-face0 bg:erc-color-face0))))
+  (when noninteractive
+    (erc-tests-common-kill-buffers)))
+
+;; This is meant to assert two behavioral properties:
+;;
+;; 1) The background is preserved when only a new foreground is
+;;    defined, in accordance with this bit from the spec: "If only the
+;;    foreground color is set, the background color stays the same."
+;;    https://modern.ircdocs.horse/formatting#color
+;;
+;; 2) The same holds true for a new, lone foreground of 99.  Rather
+;;    than prepend `erc-default-face', this causes the removal of an
+;;    existing foreground face and likewise doesn't clobber the
+;;    existing background.
+(ert-deftest erc-controls-highlight/default-foreground ()
+  (should (eq t erc-interpret-controls-p))
+  (erc-tests-common-make-server-buf)
+  (with-current-buffer (erc--open-target "#chan")
+    (setq-local erc-interpret-mirc-color t)
+    (defvar erc-fill-column)
+    (let ((erc-fill-column 90))
+      (erc-display-message nil nil (current-buffer)
+                           (erc-format-privmessage
+                            "bob" (concat "BEGIN "
+                                          "\C-c03,08 GreenOnYellow "
+                                          "\C-c99 BlackOnYellow "
+                                          "\C-o END")
+                            nil t)))
+    (forward-line -1)
+    (should (search-forward "<bob> " nil t))
+    (should (erc-tests-common-equal-with-props
+             (erc--remove-text-properties
+              (buffer-substring (point) (line-end-position)))
+             #("BEGIN  GreenOnYellow  BlackOnYellow  END"
+               0 6 (font-lock-face erc-default-face)
+               6 21 (font-lock-face (fg:erc-color-face3
+                                     bg:erc-color-face8
+                                     erc-default-face))
+               21 36 (font-lock-face (bg:erc-color-face8
+                                      erc-default-face))
+               36 40 (font-lock-face (erc-default-face)))))
+    (should (search-forward "BlackOnYellow"))
+    (let ((faces (get-text-property (point) 'font-lock-face)))
+      (should (equal (face-background (car faces) nil (cdr faces))
+                     "yellow")))
+
+    ;; Redefine background color alongside default foreground.
+    (let ((erc-fill-column 90))
+      (erc-display-message nil nil (current-buffer)
+                           (erc-format-privmessage
+                            "bob" (concat "BEGIN "
+                                          "\C-c03,08 GreenOnYellow "
+                                          "\C-c99,07 BlackOnOrange "
+                                          "\C-o END")
+                            nil t)))
+    (should (search-forward "<bob> " nil t))
+    (should (erc-tests-common-equal-with-props
+             (erc--remove-text-properties
+              (buffer-substring (point) (line-end-position)))
+             #("BEGIN  GreenOnYellow  BlackOnOrange  END"
+               0 6 (font-lock-face erc-default-face)
+               6 21 (font-lock-face (fg:erc-color-face3
+                                     bg:erc-color-face8
+                                     erc-default-face))
+               21 36 (font-lock-face (bg:erc-color-face7
+                                      erc-default-face))
+               36 40 (font-lock-face (erc-default-face)))))
+    (should (search-forward "BlackOnOrange"))
+    (let ((faces (get-text-property (point) 'font-lock-face)))
+      (should (equal (face-background (car faces) nil (cdr faces))
+                     "orange")))) ; as opposed to white or black
+  (when noninteractive
+    (erc-tests-common-kill-buffers)))
+
+;; This merely asserts our current interpretation of "default faces":
+;; that they reflect the foreground and background exhibited by normal
+;; chat messages before any control-code formatting is applied (rather
+;; than, e.g., some sort of negation or no-op).
+(ert-deftest erc-controls-highlight/default-background ()
+  (should (eq t erc-interpret-controls-p))
+  (erc-tests-common-make-server-buf)
+  (with-current-buffer (erc--open-target "#chan")
+    (setq-local erc-interpret-mirc-color t)
+    (defvar erc-fill-column)
+    (let ((erc-fill-column 90))
+      (erc-display-message nil nil (current-buffer)
+                           (erc-format-privmessage
+                            "bob" (concat "BEGIN "
+                                          "\C-c03,08 GreenOnYellow "
+                                          "\C-c05,99 BrownOnWhite "
+                                          "\C-o END")
+                            nil t)))
+    (forward-line -1)
+    (should (search-forward "<bob> " nil t))
+    (should (erc-tests-common-equal-with-props
+             (erc--remove-text-properties
+              (buffer-substring (point) (line-end-position)))
+             #("BEGIN  GreenOnYellow  BrownOnWhite  END"
+               0 6 (font-lock-face erc-default-face)
+               6 21 (font-lock-face (fg:erc-color-face3
+                                     bg:erc-color-face8
+                                     erc-default-face))
+               21 35 (font-lock-face (fg:erc-color-face5
+                                      erc-default-face))
+               35 39 (font-lock-face (erc-default-face)))))
+    ;; Ensure the background is white or black, rather than yellow.
+    (should (search-forward "BrownOnWhite"))
+    (let ((faces (get-text-property (point) 'font-lock-face)))
+      (should (equal (face-background (car faces) nil `(,@(cdr faces) default))
+                     (face-background 'default)))))
   (when noninteractive
     (erc-tests-common-kill-buffers)))
 
-- 
2.44.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-5.6-Remove-mishandled-erc-control-default-fg-bg-face.patch --]
[-- Type: text/x-patch, Size: 3544 bytes --]

From 22083686d58c4e43ab7ea05e0c54ff8ade2540f0 Mon Sep 17 00:00:00 2001
From: "F. Moukayed" <smfadi+emacs@gmail.com>
Date: Sun, 17 Mar 2024 16:43:36 +0000
Subject: [PATCH 2/2] [5.6] Remove mishandled erc-control-default-{fg,bg} faces

Partially revert the portions of 7b4ca9e609e "Leverage inverse-video
for erc-inverse-face" that introduced explicit faces for the "default"
99 color code.

* lisp/erc/erc-goodies.el (erc-control-default-fg)
(erc-control-default-bg): Remove unused faces originally meant to be
new in ERC 5.6.
(erc-get-fg-color-face, erc-get-bg-color-face): Return nil for n=99.
(erc-controls-interpret, erc-controls-highlight): Preserve an
interval's existing background so that "if only the foreground color
is set, the background color stays the same".  See
https://modern.ircdocs.horse/formatting#color.
(Bug#69860)

Copyright-paperwork-exempt: yes
---
 lisp/erc/erc-goodies.el | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/lisp/erc/erc-goodies.el b/lisp/erc/erc-goodies.el
index da14f5bd728..883f64d3109 100644
--- a/lisp/erc/erc-goodies.el
+++ b/lisp/erc/erc-goodies.el
@@ -673,14 +673,6 @@ erc-underline-face
   "ERC underline face."
   :group 'erc-faces)
 
-(defface erc-control-default-fg '((t :inherit default))
-  "ERC foreground face for the \"default\" color code."
-  :group 'erc-faces)
-
-(defface erc-control-default-bg '((t :inherit default))
-  "ERC background face for the \"default\" color code."
-  :group 'erc-faces)
-
 ;; FIXME rename these to something like `erc-control-color-N-fg',
 ;; and deprecate the old names via `define-obsolete-face-alias'.
 (defface fg:erc-color-face0 '((t :foreground "White"))
@@ -812,7 +804,7 @@ erc-get-bg-color-face
       (intern (concat "bg:erc-color-face" (number-to-string n))))
      ((< 15 n 99)
       (list :background (aref erc--controls-additional-colors (- n 16))))
-     (t (erc-log (format "   Wrong color: %s" n)) 'erc-control-default-fg))))
+     (t (erc-log (format "   Wrong color: %s" n)) nil))))
 
 (defun erc-get-fg-color-face (n)
   "Fetches the right face for foreground color N (0-15)."
@@ -828,7 +820,7 @@ erc-get-fg-color-face
       (intern (concat "fg:erc-color-face" (number-to-string n))))
      ((< 15 n 99)
       (list :foreground (aref erc--controls-additional-colors (- n 16))))
-     (t (erc-log (format "   Wrong color: %s" n)) 'erc-control-default-bg))))
+     (t (erc-log (format "   Wrong color: %s" n)) nil))))
 
 ;;;###autoload(autoload 'erc-irccontrols-mode "erc-goodies" nil t)
 (define-erc-module irccontrols nil
@@ -883,7 +875,7 @@ erc-controls-interpret
                     (setq s (replace-match "" nil nil s 1))
                     (cond ((and erc-interpret-mirc-color (or fg-color bg-color))
                            (setq fg fg-color)
-                           (setq bg bg-color))
+                           (when bg-color (setq bg bg-color)))
                           ((string= control "\C-b")
                            (setq boldp (not boldp)))
                           ((string= control "\C-]")
@@ -944,7 +936,7 @@ erc-controls-highlight
                (replace-match "" nil nil nil 1)
                (cond ((and erc-interpret-mirc-color (or fg-color bg-color))
                       (setq fg fg-color)
-                      (setq bg bg-color))
+                      (when bg-color (setq bg bg-color)))
                      ((string= control "\C-b")
                       (setq boldp (not boldp)))
                      ((string= control "\C-]")
-- 
2.44.0

bug#69860: 29.2; ERC 5.6-git: erc: Incorrect face formatting applied for fg=99 bg=x (irccontrols module with erc-interpret-mirc-color=t)

[Alcor via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 1781 bytes --]

"J.P." <jp@neverwas.me> writes:

> Alcor <alcor@tilde.club> writes:
> Here's my current understanding of what you're saying. When there's an
> existing fg/bg combo in effect for a given span, and the parsing logic
> encounters a ^Cx,99 (or a ^C99,x), the 99 shouldn't induce a clobbering
> of the corresponding "incumbent" fg (or bg) face with a dedicated (and
> superfluous) default face but instead trigger the _removal_ of said
> incumbent face. This has the effect of falling through to honor the
> relevant attributes of the underlying `erc-default-face', which ships as
> a no-op. For example, given
>
>   ^C03,08 hello ^C99,07 world
>
> the "hello " portion should be green on yellow and " world" should be
> ${default-foreground} (likely black or white) on orange. Likewise, if
> "^C99,07" were instead "^C04", then " world" should be red on yellow.
> This behavior aligns with that exhibited by Weechat and (I'm hoping)
> other popular clients. The revised patch set (attached) attempts to
> mimic this in ERC. Corrections or alternatives welcome. Thanks.

Yes, thanks for putting this in more precise words. "Clobbering" was the
term I was looking for. That was what was happening, with the 99 fg/bg
element introducing a face that clobbers the non-99 fg/bg element.

Just tried the revised patchset out on Emacs 29.2 with the latest Erc,
and things work as expected now (screenshots* attached).

Hopefully the formatting issues should be sorted out now. As usual,
thanks for the great effort & for the time invested into fixing the
"not-so-commonly-used" formatting functionality :)

+1

Cheers,
-A.

*) Checked that on both sender/receiver sides just to be sure it applies
 `erc-input-face' (for the sender) and `erc-default-face' (for the
 recipient side) correctly.


[-- Attachment #2: erc-c99-test-sender.png --]
[-- Type: image/png, Size: 24314 bytes --]

[-- Attachment #3: erc-c99-test-recvr.png --]
[-- Type: image/png, Size: 27225 bytes --]

bug#69860: 29.2; ERC 5.6-git: erc: Incorrect face formatting applied for fg=99 bg=x (irccontrols module with erc-interpret-mirc-color=t)

[J.P.]

Alcor <alcor@tilde.club> writes:

> Just tried the revised patchset out on Emacs 29.2 with the latest Erc,
> and things work as expected now (screenshots* attached).
>
> Hopefully the formatting issues should be sorted out now. As usual,
> thanks for the great effort & for the time invested into fixing the
> "not-so-commonly-used" formatting functionality :)
>
> +1

This has been installed as:

  525bc083155 * Remove mishandled erc-control-default-{fg,bg} faces

Thanks and closing.