Subject: bug#45620: 28.0.50; Child frames should have their own border width and colour

From: Alexander Miller <[email-masked]>
Date: Wed, 6 Jan 2021 17:32:18 +0100

>> >> - Currently the actual width of the border is still controlled by
the
 >> >> `internal-border-width` parameter for both frame types. Should I
try to
 >> >> do something about that as well? If yes, what's my entry point?
 >> >
 >> > Add a 'child-frame-border-width' parameter. But in this case I would
 >> > propose to proceed as follows:
 >> >
 >> > - If for a frame the 'child-frame-border-width' was explicitly
set, use
 >> > it.
 >> >
 >> > - If it was not set, use the 'internal-border-width' parameter.
 >>
 >> That's done now too, at least for X. I'll attach patches of my first
 >> working drafts for both changes. Let me know if I'm on the right
path so
 >> far.
 >
 > You're already at the end of it. Please document the changes in the
 > Elisp manual and announce them as (probably incompatible) changes in
 > NEWS.
 >

Ok, all done now, see attached patch.

For the copyright I have asked my employer to make sure that the
copyright parts from my contract don't apply to the work I do on my own
time like this, but most of the company will still be on holidays or on
sick
leave, so it could be a few days before I get an answer.

From 623b6ab24b93ca21e7fa32dd344955d1138c157f Mon Sep 17 00:00:00 2001
From: Alexander Miller <[email-masked]>
Date: Wed, 6 Jan 2021 17:02:35 +0100
Subject: [PATCH] Add distinct controls for the appearance of child frames'
 borders.

The background of the `child-frame-border` face instead of the
`internal-border` face now controls the color of child frames'
borders.

The `child-frame-border-width` frame parameter is now used for the
width of child frames' borders instead of `internal-border-width`,
though we still fall back on using the latter if the former is not
set.
---
 doc/lispref/frames.texi | 19 +++++++++++---
 etc/NEWS                |  7 +++++
 lisp/faces.el           | 11 +++++++-
 src/dispextern.h        |  1 +
 src/frame.c             | 12 +++++++++
 src/frame.h             | 25 ++++++++++++++++--
 src/nsfns.m             | 18 +++++++++++++
 src/nsterm.m            | 10 ++++---
 src/w32fns.c            | 58 ++++++++++++++++++++++++++++++++++++++---
 src/xfaces.c            |  3 +++
 src/xfns.c              | 46 ++++++++++++++++++++++++++++++++
 src/xterm.c             | 20 +++++++++-----
 12 files changed, 212 insertions(+), 18 deletions(-)

diff --git a/doc/lispref/frames.texi b/doc/lispref/frames.texi
index 7f2a6f7542..ef1b661b2a 100644
--- a/doc/lispref/frames.texi
+++ b/doc/lispref/frames.texi
@@ -694,9 +694,17 @@ Frame Layout

 @item Internal Border
 The internal border is a border drawn by Emacs around the inner frame
-(see below).  Its width is specified by the @code{internal-border-width}
-frame parameter (@pxref{Layout Parameters}).  Its color is specified by
-the background of the @code{internal-border} face.
+(see below). The specification of its appearance depends on whether
+the given frame is a child frame (@pxref{Child Frames}) or not.
+
+For normal frames its width is specified by the @code{internal-border-width}
+frame parameter (@pxref{Layout Parameters}) and its color is specified by the
+background of the @code{internal-border} face.
+
+For child frames its width is specified by the @code{child-frame-border-width}
+frame parameter (but will use the the @code{internal-border-width} parameter as
+a fallback) and its color is specified by the background of the
+@code{child-frame-border} face.

 @item Inner Frame
 @cindex inner frame
@@ -1790,6 +1798,11 @@ Layout Parameters
 The width in pixels of the frame's internal border (@pxref{Frame
 Geometry}).

+@vindex child-frame-border-width@r{, a frame parameter}
+@item child-frame-border-width
+The width in pixels of the frame's internal border (@pxref{Frame
+Geometry}) if the given frame is a child frame (@pxref{Child Frames}).
+
 @vindex vertical-scroll-bars@r{, a frame parameter}
 @item vertical-scroll-bars
 Whether the frame has scroll bars (@pxref{Scroll Bars}) for vertical
diff --git a/etc/NEWS b/etc/NEWS
index d8f25ab362..f1e8443080 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -218,6 +218,13 @@ and other "slow scrolling" situations.  It is hoped it behaves better
 than 'fast-but-imprecise-scrolling' and 'jit-lock-defer-time'.
 It is not enabled by default.

+** New face 'child-frame-border' and new frame parameter child-frame-border-width
+The face and width of child frames can no be determined separately from normal
+frames. To minimize backwards incompatibility child-frames without a
+'child-frame-border-width' parameter will fallback to using 'internal-border-width'.
+However the new 'child-frame-border' face does constitute a breaking change since
+child frames' borders no longer use the 'internal-border' face.
+
 
 * Editing Changes in Emacs 28.1

diff --git a/lisp/faces.el b/lisp/faces.el
index 4e98338432..639dbeb09a 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -2683,11 +2683,20 @@ window-divider-last-pixel

 (defface internal-border
     '((t nil))
-  "Basic face for the internal border."
+  "Basic face for the internal border.
+For the internal border of child frames see `child-frame-border'."
   :version "26.1"
   :group 'frames
   :group 'basic-faces)

+(defface child-frame-border
+  '((t nil))
+  "Basic face for the internal border of child frames.
+For the internal border of non-child frames see `internal-border'."
+  :version "28.1"
+  :group 'frames
+  :group 'basic-faces)
+
 (defface minibuffer-prompt
   '((((background dark)) :foreground "cyan")
     ;; Don't use blue because many users of the MS-DOS port customize
diff --git a/src/dispextern.h b/src/dispextern.h
index 3ad98b8344..f4e872644d 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1826,6 +1826,7 @@ #define FACE_UNIBYTE_P(FACE) ((FACE)->charset < 0)
   WINDOW_DIVIDER_FIRST_PIXEL_FACE_ID,
   WINDOW_DIVIDER_LAST_PIXEL_FACE_ID,
   INTERNAL_BORDER_FACE_ID,
+  CHILD_FRAME_BORDER_FACE_ID,
   TAB_BAR_FACE_ID,
   TAB_LINE_FACE_ID,
   BASIC_FACE_ID_SENTINEL
diff --git a/src/frame.c b/src/frame.c
index 45ee96e962..4a4ad99395 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -3528,6 +3528,13 @@ DEFUN ("frame-fringe-width", Ffringe_width, Sfringe_width, 0, 1, 0,
   return make_fixnum (FRAME_TOTAL_FRINGE_WIDTH (decode_any_frame (frame)));
 }

+DEFUN ("frame-child-frame-border-width", Fframe_child_frame_border_width, Sframe_child_frame_border_width, 0, 1, 0,
+       doc: /* Return width of FRAME's child-frame border in pixels.  */)
+  (Lisp_Object frame)
+{
+  return make_fixnum (FRAME_CHILD_FRAME_BORDER_WIDTH (decode_any_frame (frame)));
+}
+
 DEFUN ("frame-internal-border-width", Fframe_internal_border_width, Sframe_internal_border_width, 0, 1, 0,
        doc: /* Return width of FRAME's internal border in pixels.  */)
   (Lisp_Object frame)
@@ -3744,6 +3751,7 @@ DEFUN ("set-frame-window-state-change", Fset_frame_window_state_change,
   {"foreground-color",		-1},
   {"icon-name",			SYMBOL_INDEX (Qicon_name)},
   {"icon-type",			SYMBOL_INDEX (Qicon_type)},
+  {"child-frame-border-width",	SYMBOL_INDEX (Qchild_frame_border_width)},
   {"internal-border-width",	SYMBOL_INDEX (Qinternal_border_width)},
   {"right-divider-width",	SYMBOL_INDEX (Qright_divider_width)},
   {"bottom-divider-width",	SYMBOL_INDEX (Qbottom_divider_width)},
@@ -4287,6 +4295,8 @@ gui_report_frame_params (struct frame *f, Lisp_Object *alistptr)

   store_in_alist (alistptr, Qborder_width,
 		  make_fixnum (f->border_width));
+  store_in_alist (alistptr, Qchild_frame_border_width,
+		  make_fixnum (FRAME_CHILD_FRAME_BORDER_WIDTH (f)));
   store_in_alist (alistptr, Qinternal_border_width,
 		  make_fixnum (FRAME_INTERNAL_BORDER_WIDTH (f)));
   store_in_alist (alistptr, Qright_divider_width,
@@ -5984,6 +5994,7 @@ syms_of_frame (void)
   DEFSYM (Qhorizontal_scroll_bars, "horizontal-scroll-bars");
   DEFSYM (Qicon_name, "icon-name");
   DEFSYM (Qicon_type, "icon-type");
+  DEFSYM (Qchild_frame_border_width, "child-frame-border-width");
   DEFSYM (Qinternal_border_width, "internal-border-width");
   DEFSYM (Qleft_fringe, "left-fringe");
   DEFSYM (Qline_spacing, "line-spacing");
@@ -6408,6 +6419,7 @@ focus (where a frame immediately loses focus when it's left by the mouse
   defsubr (&Sscroll_bar_width);
   defsubr (&Sscroll_bar_height);
   defsubr (&Sfringe_width);
+  defsubr (&Sframe_child_frame_border_width);
   defsubr (&Sframe_internal_border_width);
   defsubr (&Sright_divider_width);
   defsubr (&Sbottom_divider_width);
diff --git a/src/frame.h b/src/frame.h
index 8cf41dc004..6a94cee727 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -534,6 +534,10 @@ #define EMACS_FRAME_H
   /* Border width of the frame window as known by the (X) window system.  */
   int border_width;

+  /* Width of child frames' internal border. Acts as the
+     interal_border_width foy child frames.  */
+  int child_frame_border_width;
+
   /* Width of the internal border.  This is a line of background color
      just inside the window's border.  When the frame is selected,
      a highlighting is displayed inside the internal border.  */
@@ -1432,11 +1436,28 @@ FRAME_TOTAL_FRINGE_WIDTH (struct frame *f)
   return FRAME_LEFT_FRINGE_WIDTH (f) + FRAME_RIGHT_FRINGE_WIDTH (f);
 }

-/* Pixel-width of internal border lines.  */
+INLINE int
+FRAME_CHILD_FRAME_BORDER_WIDTH (struct frame *f)
+{
+  return frame_dimension (f->child_frame_border_width);
+}
+
+/* Pixel-width of internal border lines.
+   Will use child_frame_border_width for child frames if
+   possible, and fall back on internal_border_width
+   otherwise.  */
 INLINE int
 FRAME_INTERNAL_BORDER_WIDTH (struct frame *f)
 {
-  return frame_dimension (f->internal_border_width);
+#ifdef HAVE_WINDOW_SYSTEM
+  return FRAME_PARENT_FRAME(f)
+    ? (f->child_frame_border_width
+       ? FRAME_CHILD_FRAME_BORDER_WIDTH(f)
+       : frame_dimension (f->internal_border_width))
+    : frame_dimension (f->internal_border_width);
+#else
+  return frame_dimension (f->internal_border_width)
+#endif
 }

 /* Pixel-size of window divider lines.  */
diff --git a/src/nsfns.m b/src/nsfns.m
index ae114f83e4..1a81b07aae 100644
--- a/src/nsfns.m
+++ b/src/nsfns.m
@@ -687,6 +687,21 @@ Turn the input menu (an NSMenu) into a lisp list for tracking on lisp side.
   }
 }

+static void
+ns_set_child_frame_border_width (struct frame *f, Lisp_Object arg, Lisp_Object oldval)
+{
+  int old_width = FRAME_CHILD_FRAME_BORDER_WIDTH (f);
+  int new_width = check_int_nonnegative (arg);
+
+  if (new_width == old_width)
+    return;
+  f->child_frame_border_width = new_width;
+
+  if (FRAME_NATIVE_WINDOW (f) != 0)
+    adjust_frame_size (f, -1, -1, 3, 0, Qchild_frame_border_width);
+
+  SET_FRAME_GARBAGED (f);
+}

 static void
 ns_set_internal_border_width (struct frame *f, Lisp_Object arg, Lisp_Object oldval)
@@ -1197,6 +1212,9 @@ Turn the input menu (an NSMenu) into a lisp list for tracking on lisp side.
   gui_default_parameter (f, parms, Qinternal_border_width, make_fixnum (2),
                          "internalBorderWidth", "InternalBorderWidth",
                          RES_TYPE_NUMBER);
+  gui_default_parameter (f, parms, Qchild_frame_border_width, make_fixnum (2),
+			 "childFrameBorderWidth", "childFrameBorderWidth",
+			 RES_TYPE_NUMBER);
   gui_default_parameter (f, parms, Qright_divider_width, make_fixnum (0),
 		       NULL, NULL, RES_TYPE_NUMBER);
   gui_default_parameter (f, parms, Qbottom_divider_width, make_fixnum (0),
diff --git a/src/nsterm.m b/src/nsterm.m
index 2defb9e2ee..76f67d2531 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -3029,9 +3029,13 @@ so some key presses (TAB) are swallowed by the system.  */
       NSRectEdge edge[] = {NSMinXEdge, NSMinYEdge, NSMaxXEdge, NSMaxYEdge};

       int face_id =
-	!NILP (Vface_remapping_alist)
-	? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID)
-	: INTERNAL_BORDER_FACE_ID;
+        (FRAME_PARENT_FRAME (f)
+         ? (!NILP (Vface_remapping_alist)
+            ? lookup_basic_face (NULL, f, CHILD_FRAME_BORDER_FACE_ID)
+            : CHILD_FRAME_BORDER_FACE_ID)
+         : (!NILP (Vface_remapping_alist)
+            ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID)
+            : INTERNAL_BORDER_FACE_ID));
       struct face *face = FACE_FROM_ID_OR_NULL (f, face_id);

       if (!face)
diff --git a/src/w32fns.c b/src/w32fns.c
index c1e18ff7fa..1d3fb5d761 100644
--- a/src/w32fns.c
+++ b/src/w32fns.c
@@ -1519,9 +1519,13 @@ w32_clear_under_internal_border (struct frame *f)
       int width = FRAME_PIXEL_WIDTH (f);
       int height = FRAME_PIXEL_HEIGHT (f);
       int face_id =
-	!NILP (Vface_remapping_alist)
-	? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID)
-	: INTERNAL_BORDER_FACE_ID;
+	(FRAME_PARENT_FRAME (f)
+	 ? (!NILP (Vface_remapping_alist)
+	    ? lookup_basic_face (NULL, f, CHILD_FRAME_BORDER_FACE_ID)
+	    : CHILD_FRAME_BORDER_FACE_ID)
+	 : (!NILP (Vface_remapping_alist)
+	    ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID)
+	    : INTERNAL_BORDER_FACE_ID));
       struct face *face = FACE_FROM_ID_OR_NULL (f, face_id);

       block_input ();
@@ -1548,6 +1552,32 @@ w32_clear_under_internal_border (struct frame *f)
     }
 }

+/**
+ * w32_set_child_frame_border_width:
+ *
+ * Set width of child frame F's internal border to ARG pixels.
+ * ARG < 0 is * treated like ARG = 0.
+ */
+static void
+w32_set_internal_border_width (struct frame *f, Lisp_Object arg, Lisp_Object oldval)
+{
+  int argval = check_integer_range (arg, INT_MIN, INT_MAX);
+  int border = max (argval, 0);
+
+  if (border != FRAME_CHILD_FRAME_BORDER_WIDTH (f))
+    {
+      f->child_frame_border_width = border;
+
+      if (FRAME_NATIVE_WINDOW (f) != 0)
+	{
+	  adjust_frame_size (f, -1, -1, 3, false, Qchild_frame_border_width);
+
+	  if (FRAME_VISIBLE_P (f))
+	    w32_clear_under_internal_border (f);
+	}
+    }
+}
+

 /**
  * w32_set_internal_border_width:
@@ -5873,6 +5903,28 @@ DEFUN ("x-create-frame", Fx_create_frame, Sx_create_frame,
 			    parameters);
     }

+  /* Same for child frames.  */
+  if (NILP (Fassq (Qchild_frame_border_width, parameters)))
+    {
+      Lisp_Object value;
+
+      value = gui_display_get_arg (dpyinfo, parameters, Qchild_frame_border_width,
+                                   "childFrameBorderWidth", "childFrameBorderWidth",
+                                   RES_TYPE_NUMBER);
+      if (! EQ (value, Qunbound))
+	parameters = Fcons (Fcons (Qchild_frame_border_width, value),
+		       parameters);
+
+    }
+
+  gui_default_parameter (f, parameters, Qchild_frame_border_width,
+#ifdef USE_GTK /* We used to impose 0 in xg_create_frame_widgets.  */
+			 make_fixnum (0),
+#else
+			 make_fixnum (1),
+#endif
+			 "childFrameBorderWidth", "childFrameBorderWidth",
+			 RES_TYPE_NUMBER);
   gui_default_parameter (f, parameters, Qinternal_border_width, make_fixnum (0),
                          "internalBorderWidth", "InternalBorder", RES_TYPE_NUMBER);
   gui_default_parameter (f, parameters, Qright_divider_width, make_fixnum (0),
diff --git a/src/xfaces.c b/src/xfaces.c
index b3b19a9cb2..5080120957 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -4913,6 +4913,7 @@ lookup_basic_face (struct window *w, struct frame *f, int face_id)
     case WINDOW_DIVIDER_FIRST_PIXEL_FACE_ID:	name = Qwindow_divider_first_pixel;	break;
     case WINDOW_DIVIDER_LAST_PIXEL_FACE_ID:	name = Qwindow_divider_last_pixel;	break;
     case INTERNAL_BORDER_FACE_ID:	name = Qinternal_border; 	break;
+    case CHILD_FRAME_BORDER_FACE_ID:	name = Qchild_frame_border; 	break;

     default:
       emacs_abort (); /* the caller is supposed to pass us a basic face id */
@@ -5619,6 +5620,7 @@ realize_basic_faces (struct frame *f)
       realize_named_face (f, Qwindow_divider_last_pixel,
 			  WINDOW_DIVIDER_LAST_PIXEL_FACE_ID);
       realize_named_face (f, Qinternal_border, INTERNAL_BORDER_FACE_ID);
+      realize_named_face (f, Qchild_frame_border, CHILD_FRAME_BORDER_FACE_ID);
       realize_named_face (f, Qtab_bar, TAB_BAR_FACE_ID);
       realize_named_face (f, Qtab_line, TAB_LINE_FACE_ID);

@@ -6967,6 +6969,7 @@ syms_of_xfaces (void)
   DEFSYM (Qwindow_divider_first_pixel, "window-divider-first-pixel");
   DEFSYM (Qwindow_divider_last_pixel, "window-divider-last-pixel");
   DEFSYM (Qinternal_border, "internal-border");
+  DEFSYM (Qchild_frame_border, "child-frame-border");

   /* TTY color-related functions (defined in tty-colors.el).  */
   DEFSYM (Qtty_color_desc, "tty-color-desc");
diff --git a/src/xfns.c b/src/xfns.c
index 9ab537ca8d..cac41ee485 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -1800,6 +1800,28 @@ x_change_tool_bar_height (struct frame *f, int height)
 #endif /* USE_GTK */
 }

+static void
+x_set_child_frame_border_width (struct frame *f, Lisp_Object arg, Lisp_Object oldval)
+{
+  int border = check_int_nonnegative (arg);
+
+  if (border != FRAME_CHILD_FRAME_BORDER_WIDTH (f))
+    {
+      f->child_frame_border_width = border;
+
+#ifdef USE_X_TOOLKIT
+      if (FRAME_X_OUTPUT (f)->edit_widget)
+	widget_store_internal_border (FRAME_X_OUTPUT (f)->edit_widget);
+#endif
+
+      if (FRAME_X_WINDOW (f))
+	{
+	  adjust_frame_size (f, -1, -1, 3, false, Qchild_frame_border_width);
+	  x_clear_under_internal_border (f);
+	}
+    }
+
+}

 static void
 x_set_internal_border_width (struct frame *f, Lisp_Object arg, Lisp_Object oldval)
@@ -3897,6 +3919,29 @@ DEFUN ("x-create-frame", Fx_create_frame, Sx_create_frame,
 	parms = Fcons (Fcons (Qinternal_border_width, value),
 		       parms);
     }
+
+  /* Same for child frames.  */
+  if (NILP (Fassq (Qchild_frame_border_width, parms)))
+    {
+      Lisp_Object value;
+
+      value = gui_display_get_arg (dpyinfo, parms, Qchild_frame_border_width,
+                                   "childFrameBorderWidth", "childFrameBorderWidth",
+                                   RES_TYPE_NUMBER);
+      if (! EQ (value, Qunbound))
+	parms = Fcons (Fcons (Qchild_frame_border_width, value),
+		       parms);
+
+    }
+
+  gui_default_parameter (f, parms, Qchild_frame_border_width,
+#ifdef USE_GTK /* We used to impose 0 in xg_create_frame_widgets.  */
+			 make_fixnum (0),
+#else
+			 make_fixnum (1),
+#endif
+			 "childFrameBorderWidth", "childFrameBorderWidth",
+			 RES_TYPE_NUMBER);
   gui_default_parameter (f, parms, Qinternal_border_width,
 #ifdef USE_GTK /* We used to impose 0 in xg_create_frame_widgets.  */
                          make_fixnum (0),
@@ -7762,6 +7807,7 @@ DEFUN ("x-gtk-debug", Fx_gtk_debug, Sx_gtk_debug, 1, 1, 0,
   x_set_foreground_color,
   x_set_icon_name,
   x_set_icon_type,
+  x_set_child_frame_border_width,
   x_set_internal_border_width,
   gui_set_right_divider_width,
   gui_set_bottom_divider_width,
diff --git a/src/xterm.c b/src/xterm.c
index 0a86738cc2..92dcd95043 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -1293,9 +1293,13 @@ x_clear_under_internal_border (struct frame *f)
       int height = FRAME_PIXEL_HEIGHT (f);
       int margin = FRAME_TOP_MARGIN_HEIGHT (f);
       int face_id =
-	!NILP (Vface_remapping_alist)
-	? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID)
-	: INTERNAL_BORDER_FACE_ID;
+	(FRAME_PARENT_FRAME (f)
+	 ? (!NILP (Vface_remapping_alist)
+	    ? lookup_basic_face (NULL, f, CHILD_FRAME_BORDER_FACE_ID)
+	    : CHILD_FRAME_BORDER_FACE_ID)
+	 : (!NILP (Vface_remapping_alist)
+	    ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID)
+	    : INTERNAL_BORDER_FACE_ID));
       struct face *face = FACE_FROM_ID_OR_NULL (f, face_id);

       block_input ();
@@ -1360,9 +1364,13 @@ x_after_update_window_line (struct window *w, struct glyph_row *desired_row)
       {
 	int y = WINDOW_TO_FRAME_PIXEL_Y (w, max (0, desired_row->y));
 	int face_id =
-	  !NILP (Vface_remapping_alist)
-	  ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID)
-	  : INTERNAL_BORDER_FACE_ID;
+	  (FRAME_PARENT_FRAME (f)
+	   ? (!NILP (Vface_remapping_alist)
+	      ? lookup_basic_face (NULL, f, CHILD_FRAME_BORDER_FACE_ID)
+	      : CHILD_FRAME_BORDER_FACE_ID)
+	   : (!NILP (Vface_remapping_alist)
+	      ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID)
+	      : INTERNAL_BORDER_FACE_ID));
 	struct face *face = FACE_FROM_ID_OR_NULL (f, face_id);

 	block_input ();
--
2.29.2