bug#34569: 26.1.90; Zero wide scroll bars

bug#34569: 26.1.90; Zero wide scroll bars

[martin rudalics]

Setting 'scroll-bar-width' to zero can have unforeseen consequences
depending on the toolkit used.  To reproduce with Emacs 26 run a Lucid
or Motif build as

emacs - Q --eval "(setq default-frame-alist '((minibuffer . nil) (vertical-scroll-bars . nil) (scroll-bar-width . 0)))"

Then evaluate

(set-frame-parameter nil 'vertical-scroll-bars 'left)

Emacs aborts with a

X protocol error: BadValue (integer parameter out of range for operation) on protocol request 12

GTK builds do not abort but have for example a scroll bar on the left
overwrite buffer text.  A similar bug can be produced with horizontal
scroll bars.

It's not entirely trivial to explain why Emacs aborts here.  The cause
is a combination of contrived logic and the absence of a more cautious
initial setting for X builds.  For starters, frame.h says that

/* Width that a scroll bar in frame F should have, if there is one.
    Measured in pixels.
    If scroll bars are turned off, this is still nonzero.  */
#define FRAME_CONFIG_SCROLL_BAR_WIDTH(f) ((f)->config_scroll_bar_width)

This comment is, unfortunately, wrong because in our example
'frame-notice-user-settings'

	    (setq parms (frame-parameters frame-initial-frame))

'frame-parameters' creates a zero entry for the 'scroll-bar-width'
frame parameter since our initial frame has no scroll bars and
'frame-notice-user-settings' passes that parameter on to 'make-frame'.
In frame.c this subsequently bypasses both checks in

x_set_scroll_bar_width (struct frame *f, Lisp_Object arg, Lisp_Object oldval)
{
   int unit = FRAME_COLUMN_WIDTH (f);

   if (NILP (arg))
     {
       x_set_scroll_bar_default_width (f);
...
     }
   else if (RANGED_INTEGERP (1, arg, INT_MAX)
	   && XFASTINT (arg) != FRAME_CONFIG_SCROLL_BAR_WIDTH (f))
     {
       FRAME_CONFIG_SCROLL_BAR_WIDTH (f) = XFASTINT (arg);
...

since ARG is zero which is neither nil nor a ranged integer > 0.  So
we continue with the initially zero FRAME_CONFIG_SCROLL_BAR_WIDTH (f)
and try to make a window with a zero wide scroll bar produced from the
initial w->scroll_bar_width (which equals -1) and these definitions in
window.h:

/* Width that a scroll bar in window W should have, if there is one.
    Measured in pixels.  If scroll bars are turned off, this is still
    nonzero.  */
#define WINDOW_CONFIG_SCROLL_BAR_WIDTH(W)		\
   (W->scroll_bar_width >= 0				\
    ? W->scroll_bar_width				\
    : FRAME_CONFIG_SCROLL_BAR_WIDTH (WINDOW_XFRAME (W)))

/* Width of scroll bar area in window W, measured in pixels.  */
#define WINDOW_SCROLL_BAR_AREA_WIDTH(W)					\
   (WINDOW_HAS_VERTICAL_SCROLL_BAR (W)					\
    ? WINDOW_CONFIG_SCROLL_BAR_WIDTH (W)					\
    : 0)

Here the first comment is wrong again, WINDOW_CONFIG_SCROLL_BAR_WIDTH
is zero and X is rightfully annoyed.

The easisest fix I could come up with is switching the two branches of
the if clause in x_set_scroll_bar_width thusly:

void
x_set_scroll_bar_width (struct frame *f, Lisp_Object arg, Lisp_Object oldval)
{
   int unit = FRAME_COLUMN_WIDTH (f);

  if (RANGED_INTEGERP (1, arg, INT_MAX)
	   && XFASTINT (arg) != FRAME_CONFIG_SCROLL_BAR_WIDTH (f))
     {
       FRAME_CONFIG_SCROLL_BAR_WIDTH (f) = XFASTINT (arg);
       FRAME_CONFIG_SCROLL_BAR_COLS (f) = (XFASTINT (arg) + unit - 1) / unit;
       if (FRAME_X_WINDOW (f))
	adjust_frame_size (f, -1, -1, 3, 0, Qscroll_bar_width);

       SET_FRAME_GARBAGED (f);
     }
   else
     {
       x_set_scroll_bar_default_width (f);

       if (FRAME_X_WINDOW (f))
	adjust_frame_size (f, -1, -1, 3, 0, Qscroll_bar_width);

       SET_FRAME_GARBAGED (f);
     }

   XWINDOW (FRAME_SELECTED_WINDOW (f))->cursor.hpos = 0;
   XWINDOW (FRAME_SELECTED_WINDOW (f))->cursor.x = 0;
}

But maybe someone has a better idea.

Note that Windows builds sidestep the problem by unconditionally doing

   /* By default, make scrollbars the system standard width and height. */
   FRAME_CONFIG_SCROLL_BAR_WIDTH (f) = GetSystemMetrics (SM_CXVSCROLL);

in 'x-create-frame'.

Note also that with emacs 27.1 the bug can be produced more directly by
including

(setq default-frame-alist '((vertical-scroll-bars . nil) (scroll-bar-width . 0)))

in the early-init.el and then enabling vertical scroll bars.  The
indirection via 'frame-notice-user-settings' is not needed there.

Thanks, martin

bug#34569: 26.1.90; Zero wide scroll bars

[Eli Zaretskii]

> Date: Tue, 19 Feb 2019 10:08:15 +0100
> From: martin rudalics <rudalics@gmx.at>
> 
> Setting 'scroll-bar-width' to zero can have unforeseen consequences
> depending on the toolkit used.

Why would users set 'scroll-bar-width' to zero, instead of turning off
scroll-bar-mode?  Or are you saying that turning off scroll-bar-mode
also produces the same bugs?

If turning off scroll-bar-mode does work, then how about disabling the
direct setting of 'scroll-bar-width', either silently or noisily?

> /* Width that a scroll bar in frame F should have, if there is one.
>     Measured in pixels.
>     If scroll bars are turned off, this is still nonzero.  */
> #define FRAME_CONFIG_SCROLL_BAR_WIDTH(f) ((f)->config_scroll_bar_width)
> 
> This comment is, unfortunately, wrong

But the comment says "disabled", not "width set to zero".  is it
correct when scroll-bar-mode is turned off?  If so, perhaps just
qualifying the comment by the method by which the scroll bars are
disabled will be good enough?

> Note also that with emacs 27.1 the bug can be produced more directly by
> including
> 
> (setq default-frame-alist '((vertical-scroll-bars . nil) (scroll-bar-width . 0)))
> 
> in the early-init.el and then enabling vertical scroll bars.  The
> indirection via 'frame-notice-user-settings' is not needed there.

If we disallow setting this parameter directly, or at least setting it
to zero, this problem will also go away, right?

Thanks.

bug#34569: 26.1.90; Zero wide scroll bars

[martin rudalics]

 > Why would users set 'scroll-bar-width' to zero, instead of turning off
 > scroll-bar-mode?

When they want to turn scroll bars off for a specific frame.  Turning
scroll bars on later would leave the frame with zero width scroll bars
alone.  It won't work but it could be the idea.

 > Or are you saying that turning off scroll-bar-mode
 > also produces the same bugs?

No (otherwise this would have been an issue known ever since).

 > If turning off scroll-bar-mode does work, then how about disabling the
 > direct setting of 'scroll-bar-width', either silently or noisily?

Setting 'scroll-bar-width' is fragile.  With GTK builds you can set
it, Emacs will respect it and GTK will either overdraw or leave a gap
because you can't change the GTK scroll bar from within Emacs.  Note
the dual use of this parameter: 'scroll-bar-width' is (1) passed on to
the toolkit to draw scroll bars of the desired width and (2) used by
Emacs to clear various areas of the window and calculate the width of
the text area.

But yes: One way to fix the aborts should be to disallow setting the
'scroll-bar-width' frame parameter to zero.

 >> /* Width that a scroll bar in frame F should have, if there is one.
 >>      Measured in pixels.
 >>      If scroll bars are turned off, this is still nonzero.  */
 >> #define FRAME_CONFIG_SCROLL_BAR_WIDTH(f) ((f)->config_scroll_bar_width)
 >>
 >> This comment is, unfortunately, wrong
 >
 > But the comment says "disabled", not "width set to zero".  is it
 > correct when scroll-bar-mode is turned off?  If so, perhaps just
 > qualifying the comment by the method by which the scroll bars are
 > disabled will be good enough?

This hints at another way of fixing the aborts: Handle zero wide
scroll bars just as if scroll bars were disabled/turned off.

 >> Note also that with emacs 27.1 the bug can be produced more directly by
 >> including
 >>
 >> (setq default-frame-alist '((vertical-scroll-bars . nil) (scroll-bar-width . 0)))
 >>
 >> in the early-init.el and then enabling vertical scroll bars.  The
 >> indirection via 'frame-notice-user-settings' is not needed there.
 >
 > If we disallow setting this parameter directly, or at least setting it
 > to zero, this problem will also go away, right?

Why would we disallow setting this parameter directly?  On Windows it
works perfectly.  On Lucid and Motif all widths but zero work well.

martin

bug#34569: 26.1.90; Zero wide scroll bars

[Eli Zaretskii]

> Date: Sat, 23 Feb 2019 15:01:54 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: 34569@debbugs.gnu.org
> 
>  > If turning off scroll-bar-mode does work, then how about disabling the
>  > direct setting of 'scroll-bar-width', either silently or noisily?
> 
> Setting 'scroll-bar-width' is fragile.  With GTK builds you can set
> it, Emacs will respect it and GTK will either overdraw or leave a gap
> because you can't change the GTK scroll bar from within Emacs.  Note
> the dual use of this parameter: 'scroll-bar-width' is (1) passed on to
> the toolkit to draw scroll bars of the desired width and (2) used by
> Emacs to clear various areas of the window and calculate the width of
> the text area.
> 
> But yes: One way to fix the aborts should be to disallow setting the
> 'scroll-bar-width' frame parameter to zero.

Then I think we should do that.

> This hints at another way of fixing the aborts: Handle zero wide
> scroll bars just as if scroll bars were disabled/turned off.

Yes.

>  > If we disallow setting this parameter directly, or at least setting it
>  > to zero, this problem will also go away, right?
> 
> Why would we disallow setting this parameter directly?  On Windows it
> works perfectly.  On Lucid and Motif all widths but zero work well.

I said "or at least setting it to zero".

bug#34569: 26.1.90; Zero wide scroll bars

[martin rudalics]

 >> But yes: One way to fix the aborts should be to disallow setting the
 >> 'scroll-bar-width' frame parameter to zero.
 >
 > Then I think we should do that.

It means though that we have to do some non-standard surgery in the
frame parameters area.  Note that x_report_frame_params inserts the
zero value automatically via

   store_in_alist (alistptr, Qscroll_bar_width,
		  (! FRAME_HAS_VERTICAL_SCROLL_BARS (f)
		   ? make_fixnum (0)
		   : FRAME_CONFIG_SCROLL_BAR_WIDTH (f) > 0
		   ? make_fixnum (FRAME_CONFIG_SCROLL_BAR_WIDTH (f))
		   /* nil means "use default width"
		      for non-toolkit scroll bar.
		      ruler-mode.el depends on this.  */
		   : Qnil));

What should we use instead?

martin

bug#34569: 26.1.90; Zero wide scroll bars

[Eli Zaretskii]

> Date: Sun, 24 Feb 2019 09:43:37 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: 34569@debbugs.gnu.org
> 
>  >> But yes: One way to fix the aborts should be to disallow setting the
>  >> 'scroll-bar-width' frame parameter to zero.
>  >
>  > Then I think we should do that.
> 
> It means though that we have to do some non-standard surgery in the
> frame parameters area.

That's okay, we already do similar things with some parameters.

bug#34569: 26.1.90; Zero wide scroll bars

[martin rudalics]

 >> It means though that we have to do some non-standard surgery in the
 >> frame parameters area.
 >
 > That's okay, we already do similar things with some parameters.

You didn't answer that question:

 > Note that x_report_frame_params inserts the
 > zero value automatically via
 >
 >   store_in_alist (alistptr, Qscroll_bar_width,
 >           (! FRAME_HAS_VERTICAL_SCROLL_BARS (f)
 >            ? make_fixnum (0)
 >            : FRAME_CONFIG_SCROLL_BAR_WIDTH (f) > 0
 >            ? make_fixnum (FRAME_CONFIG_SCROLL_BAR_WIDTH (f))
 >            /* nil means "use default width"
 >               for non-toolkit scroll bar.
 >               ruler-mode.el depends on this.  */
 >            : Qnil));
 >
 > What should we use instead?

martin

bug#34569: 26.1.90; Zero wide scroll bars

[Eli Zaretskii]

> Date: Sun, 24 Feb 2019 19:31:47 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: 34569@debbugs.gnu.org
> 
>  >> It means though that we have to do some non-standard surgery in the
>  >> frame parameters area.
>  >
>  > That's okay, we already do similar things with some parameters.
> 
> You didn't answer that question:
> 
>  > Note that x_report_frame_params inserts the
>  > zero value automatically via
>  >
>  >   store_in_alist (alistptr, Qscroll_bar_width,
>  >           (! FRAME_HAS_VERTICAL_SCROLL_BARS (f)
>  >            ? make_fixnum (0)
>  >            : FRAME_CONFIG_SCROLL_BAR_WIDTH (f) > 0
>  >            ? make_fixnum (FRAME_CONFIG_SCROLL_BAR_WIDTH (f))
>  >            /* nil means "use default width"
>  >               for non-toolkit scroll bar.
>  >               ruler-mode.el depends on this.  */
>  >            : Qnil));
>  >
>  > What should we use instead?

I thought I did.  Maybe I didn't understand what exactly you are
asking.

bug#34569: 26.1.90; Zero wide scroll bars

[martin rudalics]

 >> You didn't answer that question:
 >>
 >>   > Note that x_report_frame_params inserts the
 >>   > zero value automatically via
 >>   >
 >>   >   store_in_alist (alistptr, Qscroll_bar_width,
 >>   >           (! FRAME_HAS_VERTICAL_SCROLL_BARS (f)
 >>   >            ? make_fixnum (0)
 >>   >            : FRAME_CONFIG_SCROLL_BAR_WIDTH (f) > 0
 >>   >            ? make_fixnum (FRAME_CONFIG_SCROLL_BAR_WIDTH (f))
 >>   >            /* nil means "use default width"
 >>   >               for non-toolkit scroll bar.
 >>   >               ruler-mode.el depends on this.  */
 >>   >            : Qnil));
 >>   >
 >>   > What should we use instead?
 >
 > I thought I did.  Maybe I didn't understand what exactly you are
 > asking.

I wanted to know which value to assign when a frame does not have
vertical scroll bars.  We could try with

   store_in_alist (alistptr, Qscroll_bar_width,
		  (FRAME_CONFIG_SCROLL_BAR_WIDTH (f) > 0
		   ? make_fixnum (FRAME_CONFIG_SCROLL_BAR_WIDTH (f))
		   /* nil means "use default width"
		      for non-toolkit scroll bar.
		      ruler-mode.el depends on this.  */
		   : Qnil));

but I'm not sure whether that's what you meant.

martin

bug#34569: 26.1.90; Zero wide scroll bars

[Eli Zaretskii]

> Date: Mon, 25 Feb 2019 11:13:13 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: 34569@debbugs.gnu.org
> 
> I wanted to know which value to assign when a frame does not have
> vertical scroll bars.  We could try with
> 
>    store_in_alist (alistptr, Qscroll_bar_width,
> 		  (FRAME_CONFIG_SCROLL_BAR_WIDTH (f) > 0
> 		   ? make_fixnum (FRAME_CONFIG_SCROLL_BAR_WIDTH (f))
> 		   /* nil means "use default width"
> 		      for non-toolkit scroll bar.
> 		      ruler-mode.el depends on this.  */
> 		   : Qnil));
> 
> but I'm not sure whether that's what you meant.

It would work for me, thanks.

bug#34569: 26.1.90; Zero wide scroll bars

[martin rudalics]

fixed 34569 27.1
quit

 >> It means though that we have to do some non-standard surgery in the
 >> frame parameters area.
 >
 > That's okay, we already do similar things with some parameters.

I now installed the most comprehensive fix I came up with.  Marking
bug as done.

Thanks, martin