bug#37359: [PATCH] Fix ignored Motif scrollbar resources.

bug#37359: [PATCH] Fix ignored Motif scrollbar resources.

[Matt Bisson]

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

Hi Emacs maintainers,

I noticed that I was only able to set the "truck" color for scrollbars on Motif.  I tried a variety of methods, including explicitly setting resources for the scrollbar class as documented by the libXm API, and using foreground and background faces, but to no avail.  After looking at the code in src/xterm.c, I noticed that the creation code was not quite correct, so I patched the code (attached):

On Motif scrollbars, "foreground" has no meaning, while "background" means the truck and arrow colors, and "trough" means the background of the entire widget.  I have hooked up the Emacs scrollbar "foreground" color to the XmNbackground resource, and the "background" color to XmNtroughColor.  This is more in line with how Xaw scrollbars behave.

I am still a little flummoxed as to why the more sweeping resource settings like "*XmScrollBar*troughColor" seem to have no effect, but that's an issue for another day, and I may look into it further as I have time.

My Emacs environment is 26.3 (but I noticed this code has been like this a long time) running on 64-bit Linux (Gentoo).  The Motif version is 2.3.8.

Thanks,
-Matt

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-ignored-Motif-scrollbar-resources.patch --]
[-- Type: text/x-patch; name=0001-Fix-ignored-Motif-scrollbar-resources.patch, Size: 1448 bytes --]

From 60cf87f9fc6104ae44c31125b2f5050cb31b9e4a Mon Sep 17 00:00:00 2001
From: Matt Bisson <mbisson@vmware.com>
Date: Mon, 9 Sep 2019 10:46:20 -0400
Subject: [PATCH] Fix ignored Motif scrollbar resources.

On Motif scrollbars, "foreground" has no meaning, while "background"
means the truck and arrow colors, and "trough" means the background of
the entire widget.  I have hooked up the Emacs scrollbar "foreground"
color to the XmNbackground resource, and the "background" color to
XmNtroughColor.  This is more in line with how Xaw scrollbars behave.
---
 src/xterm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/xterm.c b/src/xterm.c
index 3cadf69380..70e45a964b 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -5925,17 +5925,19 @@ x_create_toolkit_scroll_bar (struct frame *f, struct scroll_bar *bar)
   XtSetArg (av[ac], XmNincrement, 1); ++ac;
   XtSetArg (av[ac], XmNpageIncrement, 1); ++ac;
 
+  /* Note: "background" is the thumb color, and "trough" is the color behind
+     everything. */
   pixel = f->output_data.x->scroll_bar_foreground_pixel;
   if (pixel != -1)
     {
-      XtSetArg (av[ac], XmNforeground, pixel);
+      XtSetArg (av[ac], XmNbackground, pixel);
       ++ac;
     }
 
   pixel = f->output_data.x->scroll_bar_background_pixel;
   if (pixel != -1)
     {
-      XtSetArg (av[ac], XmNbackground, pixel);
+      XtSetArg (av[ac], XmNtroughColor, pixel);
       ++ac;
     }
 
-- 
2.23.0

bug#37359: [PATCH] Fix ignored Motif scrollbar resources.

[martin rudalics]

 > On Motif scrollbars, "foreground" has no meaning, while "background"
 > means the truck and arrow colors, and "trough" means the background
 > of the entire widget.  I have hooked up the Emacs scrollbar
 > "foreground" color to the XmNbackground resource, and the
 > "background" color to XmNtroughColor.  This is more in line with how
 > Xaw scrollbars behave.

Sounds reasonable so I intend to install it.  I suppose that you have
not done any copyright assignment so I'll mark this as a tiny change.

Thanks, martin

bug#37359: [PATCH] Fix ignored Motif scrollbar resources.

[Matt Bisson]

> I suppose that you have not done any copyright assignment so I'll mark this as a tiny change.

From the copyright assignment page, it seems reasonable to call this a tiny change, so I didn't do the assignment:

> We can accept small changes (roughly, fewer than 15 lines) without an assignment. This is a cumulative limit (e.g., three separate 5 line patches) over all your contributions.

If you prefer, I can go through that process as well.  Whatever is easier.

> Sounds reasonable so I intend to install it.

When you say "install" do you mean you are going to test it, and I should push to the repo (which I am completely willing to do), or do you mean you are going to commit the patch when ready?  (The patch is against the emacs-26 branch, btw.)

Thanks,
-Matt

----- Original Message -----
From: "martin rudalics" <rudalics@gmx.at>
To: "mbisson" <mbisson@ccs.neu.edu>, 37359@debbugs.gnu.org
Sent: Tuesday, September 10, 2019 3:56:03 AM
Subject: Re: bug#37359: [PATCH] Fix ignored Motif scrollbar resources.

> On Motif scrollbars, "foreground" has no meaning, while "background"
 > means the truck and arrow colors, and "trough" means the background
 > of the entire widget.  I have hooked up the Emacs scrollbar
 > "foreground" color to the XmNbackground resource, and the
 > "background" color to XmNtroughColor.  This is more in line with how
 > Xaw scrollbars behave.

Sounds reasonable so I intend to install it.  I suppose that you have
not done any copyright assignment so I'll mark this as a tiny change.

Thanks, martin

bug#37359: [PATCH] Fix ignored Motif scrollbar resources.

[martin rudalics]

 > If you prefer, I can go through that process as well.  Whatever is easier.

I obviously prefer that you go through that process so we can accept
your future changes (resolving the "*XmScrollBar*troughColor" issue,
for instance).  But it's not necessary for the change at hand.

 > When you say "install" do you mean you are going to test it, and I
 > should push to the repo (which I am completely willing to do), or do
 > you mean you are going to commit the patch when ready?  (The patch
 > is against the emacs-26 branch, btw.)

I have tested it on current master (Motif 2.3.4) and will commit it
there.  It would be nice though if you provided a ChangeLog entry.

TIA, martin

bug#37359: [PATCH] Fix ignored Motif scrollbar resources.

[Matt Bisson]

Good point (about the ChangeLog).  Let me update that, and get the necessary legal items taken care of, and get back to you.  It will be a few days probably, so hopefully nobody is desperate for the fix. ;-D

Thanks,
-Matt

----- Original Message -----
From: "martin rudalics" <rudalics@gmx.at>
To: "mbisson" <mbisson@ccs.neu.edu>
Cc: 37359@debbugs.gnu.org
Sent: Tuesday, September 10, 2019 4:25:18 PM
Subject: Re: bug#37359: [PATCH] Fix ignored Motif scrollbar resources.

> If you prefer, I can go through that process as well.  Whatever is easier.

I obviously prefer that you go through that process so we can accept
your future changes (resolving the "*XmScrollBar*troughColor" issue,
for instance).  But it's not necessary for the change at hand.

 > When you say "install" do you mean you are going to test it, and I
 > should push to the repo (which I am completely willing to do), or do
 > you mean you are going to commit the patch when ready?  (The patch
 > is against the emacs-26 branch, btw.)

I have tested it on current master (Motif 2.3.4) and will commit it
there.  It would be nice though if you provided a ChangeLog entry.

TIA, martin

bug#37359: [PATCH] Fix ignored Motif scrollbar resources.

[Glenn Morris]

Not related to your report in any way, but the Motif version of Emacs is
very rarely used. Could you say why you use it?

bug#37359: [PATCH] Fix ignored Motif scrollbar resources.

[Matt Bisson]

A few reasons:
1) I like it. :)
2) I'm warded away from GTK because of the very long-standing bug in the toolkit that results in core files when I use --daemon.
3) I've been doing some work on CDE, which of course tightly integrates with Motif.

Cheers,
-Matt

PS: I'm still waiting to hear back from the copyright-assign mailing list about my filing, so it will be a little bit longer yet.

----- Original Message -----
From: "Glenn Morris" <rgm@gnu.org>
To: "mbisson" <mbisson@ccs.neu.edu>
Cc: "martin rudalics" <rudalics@gmx.at>, 37359@debbugs.gnu.org
Sent: Monday, September 16, 2019 4:00:28 PM
Subject: Re: bug#37359: [PATCH] Fix ignored Motif scrollbar resources.

Not related to your report in any way, but the Motif version of Emacs is
very rarely used. Could you say why you use it?

bug#37359: [PATCH] Fix ignored Motif scrollbar resources.

[Glenn Morris]

Matt Bisson wrote:

> 2) I'm warded away from GTK because of the very long-standing bug in
> the toolkit that results in core files when I use --daemon.

(FWIW, the more common solution for this is the Lucid/Athena toolkit version.)

> 3) I've been doing some work on CDE, which of course tightly
> integrates with Motif.

Thanks, I might try out CDE to see if it looks like I remember from back
in the day! :)

bug#37359: [PATCH] Fix ignored Motif scrollbar resources.

[Matt Bisson]

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

I have filed the legal copyright transfer paperwork, but this takes time, and I was told to go ahead and update the patches without it, so I've updated the patch (mostly the commit description), and I'm attaching a ChangeLog file here as requested.

Would it be possible to back-port this into an earlier version of Emacs than ToT (my patch was done against the emacs-26 branch)?  Just curious.

Cheers,
-Matt

----- Original Message -----
From: "martin rudalics" <rudalics@gmx.at>
To: "mbisson" <mbisson@ccs.neu.edu>
Cc: 37359@debbugs.gnu.org
Sent: Tuesday, September 10, 2019 4:25:18 PM
Subject: Re: bug#37359: [PATCH] Fix ignored Motif scrollbar resources.

> If you prefer, I can go through that process as well.  Whatever is easier.

I obviously prefer that you go through that process so we can accept
your future changes (resolving the "*XmScrollBar*troughColor" issue,
for instance).  But it's not necessary for the change at hand.

 > When you say "install" do you mean you are going to test it, and I
 > should push to the repo (which I am completely willing to do), or do
 > you mean you are going to commit the patch when ready?  (The patch
 > is against the emacs-26 branch, btw.)

I have tested it on current master (Motif 2.3.4) and will commit it
there.  It would be nice though if you provided a ChangeLog entry.

TIA, martin

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ChangeLog --]
[-- Type: text/x-changelog; name=ChangeLog, Size: 498 bytes --]

2019-09-17  Matt Bisson  <mbisson@mbisson-devd1>

	Fix ignored Motif scrollbar resources

	* src/xterm.c (x_create_toolkit_scroll_bar): On Motif scrollbars,
	"foreground" has no meaning, while "background" means the truck and
	arrow colors, and "trough" means the background of the entire widget
	(bug#37359) I have hooked up the Emacs scrollbar "foreground" color to
	the XmNbackground resource, and the "background" color to
	XmNtroughColor.  This is more in line with how Xaw scrollbars behave.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-Fix-ignored-Motif-scrollbar-resources.patch --]
[-- Type: text/x-patch; name=0001-Fix-ignored-Motif-scrollbar-resources.patch, Size: 1502 bytes --]

From 6168bbca7e471447f84e3de1a9fa6fbabcc9a962 Mon Sep 17 00:00:00 2001
From: Matt Bisson <mbisson@vmware.com>
Date: Mon, 9 Sep 2019 10:46:20 -0400
Subject: [PATCH] Fix ignored Motif scrollbar resources

* src/xterm.c (x_create_toolkit_scroll_bar): On Motif scrollbars,
"foreground" has no meaning, while "background" means the truck and
arrow colors, and "trough" means the background of the entire widget
(bug#37359) I have hooked up the Emacs scrollbar "foreground" color to
the XmNbackground resource, and the "background" color to
XmNtroughColor.  This is more in line with how Xaw scrollbars behave.
---
 src/xterm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/xterm.c b/src/xterm.c
index 3cadf69380..70e45a964b 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -5925,17 +5925,19 @@ x_create_toolkit_scroll_bar (struct frame *f, struct scroll_bar *bar)
   XtSetArg (av[ac], XmNincrement, 1); ++ac;
   XtSetArg (av[ac], XmNpageIncrement, 1); ++ac;
 
+  /* Note: "background" is the thumb color, and "trough" is the color behind
+     everything. */
   pixel = f->output_data.x->scroll_bar_foreground_pixel;
   if (pixel != -1)
     {
-      XtSetArg (av[ac], XmNforeground, pixel);
+      XtSetArg (av[ac], XmNbackground, pixel);
       ++ac;
     }
 
   pixel = f->output_data.x->scroll_bar_background_pixel;
   if (pixel != -1)
     {
-      XtSetArg (av[ac], XmNbackground, pixel);
+      XtSetArg (av[ac], XmNtroughColor, pixel);
       ++ac;
     }
 
-- 
2.23.0

bug#37359: [PATCH] Fix ignored Motif scrollbar resources.

[Eli Zaretskii]

> Date: Tue, 17 Sep 2019 11:41:56 -0400 (EDT)
> From: Matt Bisson <mbisson@ccs.neu.edu>
> Cc: 37359@debbugs.gnu.org
> 
> Would it be possible to back-port this into an earlier version of Emacs than ToT (my patch was done against the emacs-26 branch)?  Just curious.

We could back-port it, but it makes little sense, since we don't plan
to have any further releases from the emacs-26 branch.

Thanks.

bug#37359: [PATCH] Fix ignored Motif scrollbar resources.

[martin rudalics]

 > I have filed the legal copyright transfer paperwork, but this takes
 > time, and I was told to go ahead and update the patches without it,
 > so I've updated the patch (mostly the commit description), and I'm
 > attaching a ChangeLog file here as requested.

Thanks.  I'll install this as soon as I'm able to build Emacs again.

martin

bug#37359: [PATCH] Fix ignored Motif scrollbar resources.

[martin rudalics]

 > I have filed the legal copyright transfer paperwork, but this takes
 > time, and I was told to go ahead and update the patches without it,
 > so I've updated the patch (mostly the commit description), and I'm
 > attaching a ChangeLog file here as requested.

Installed.  Just that I completely missed the fact that a similar
change would be now required for horizontal scroll bars as well.  Can
you please provide one.

Thanks, martin

bug#37359: [PATCH] Fix ignored Motif scrollbar resources.

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> Installed.  Just that I completely missed the fact that a similar
> change would be now required for horizontal scroll bars as well.  Can
> you please provide one.

That was a couple of weeks ago, so I just cargo-culted the vertical
scroll bar changes to the horizontal ones, and it seems to work fine.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#37359: [PATCH] Fix ignored Motif scrollbar resources.

[martin rudalics]

> That was a couple of weeks ago, so I just cargo-culted the vertical
> scroll bar changes to the horizontal ones, and it seems to work fine.

Thanks, martin

bug#37359: [PATCH] Fix ignored Motif scrollbar resources.

[Matt Bisson]

Sorry, I didn't get back to this in time -- work has been hectic of late!  Thanks for fixing this.

----- Original Message -----
From: "martin rudalics" <rudalics@gmx.at>
To: "Lars Ingebrigtsen" <larsi@gnus.org>
Cc: "mbisson" <mbisson@ccs.neu.edu>, 37359@debbugs.gnu.org
Sent: Monday, October 7, 2019 5:26:29 AM
Subject: Re: bug#37359: [PATCH] Fix ignored Motif scrollbar resources.

> That was a couple of weeks ago, so I just cargo-culted the vertical
> scroll bar changes to the horizontal ones, and it seems to work fine.

Thanks, martin