bug#22660: No horizontal scroll bars in NextStep build [OSX]

bug#22660: No horizontal scroll bars in NextStep build [OSX]

[Angelo Graziosi]

I notice that the NextStep build of Emacs (master, for example) does not 
offer the option for enabling the horizontal scroll bar in Options- 
Show/Hide - Scroll-bar. Only None, Left, Right is shown..

Also adding

(horizontal-scroll-bar-mode 1)

in the init file does not help..

   Angelo

bug#22660: No horizontal scroll bars in NextStep build [OSX]

[Alan Third]

On Sun, Feb 14, 2016 at 12:04:36PM +0100, Angelo Graziosi wrote:
> I notice that the NextStep build of Emacs (master, for example) does not
> offer the option for enabling the horizontal scroll bar in Options-
> Show/Hide - Scroll-bar. Only None, Left, Right is shown..

Hi Angelo, unfortunately the NextStep build doesn't support horizontal
scroll bars yet. If you do:

M-x horizontal-scroll-bar-mode

You should get a message saying:

Horizontal scroll bars are not implemented on this system

I'm going to set this bug as a wishlist item in the hope that someone
will implement them in the future.
-- 
Alan Third

bug#22660: No horizontal scroll bars in NextStep build [OSX]

[Angelo Graziosi]

Oh thanks Alan... It would be very useful to have HSB in NS..

Ciao,
   Angelo.

Il 14/02/2016 13:20, Alan Third ha scritto:
> On Sun, Feb 14, 2016 at 12:04:36PM +0100, Angelo Graziosi wrote:
>> I notice that the NextStep build of Emacs (master, for example) does not
>> offer the option for enabling the horizontal scroll bar in Options-
>> Show/Hide - Scroll-bar. Only None, Left, Right is shown..
>
> Hi Angelo, unfortunately the NextStep build doesn't support horizontal
> scroll bars yet. If you do:
>
> M-x horizontal-scroll-bar-mode
>
> You should get a message saying:
>
> Horizontal scroll bars are not implemented on this system
>
> I'm going to set this bug as a wishlist item in the hope that someone
> will implement them in the future.
>

bug#22660: No horizontal scroll bars in NextStep build [OSX]

[martin rudalics]

 > I notice that the NextStep build of Emacs (master, for example) does
 > not offer the option for enabling the horizontal scroll bar in
 > Options- Show/Hide - Scroll-bar. Only None, Left, Right is shown..

Indeed, horizontal scollbars have not been implemented for this build.

It should be pretty easy to add support for them, though.  I won't do it
because it doesn't make sense to do that without a machine running
NextStep.  But if somone would like to give it a try (some stubs should
be already present) I could certainly provide additional information.

martin

bug#22660: No horizontal scroll bars in NextStep build [OSX]

[Alan Third]

On Mon, Feb 15, 2016 at 11:57:12AM +0100, martin rudalics wrote:
> Indeed, horizontal scollbars have not been implemented for this build.
> 
> It should be pretty easy to add support for them, though.  I won't do it
> because it doesn't make sense to do that without a machine running
> NextStep.  But if somone would like to give it a try (some stubs should
> be already present) I could certainly provide additional information.

I'd be willing to give it a go if it is, as you say, pretty easy. I've
had a quick look and see that there are a few functions related to
horizontal scrollbars in the NS code already, and there's
scroll-bar.el, but I'm failing to see how it all ties together.

Mind you, I suppose the point here is that it doesn't.
-- 
Alan Third

bug#22660: [PATCH] Implement horizontal scroll-bars in NS port

[Alan Third]

* lisp/scroll-bar.el (horizontal-scroll-bars-available-p): Remove NS
check.
* lisp/term/ns-win.el: Remove custom NS scroll-bar handlers and bind
scroll-bar mouse clicks to standard handlers.
* src/nsterm.h (EmacsScroller): Add 'horizontal' property and rename
pixel_height to pixel_length.
* src/nsterm.m (ns_set_horizontal_scroll_bar): Rewrite to handle
horizontal scrollers correctly.
(initFrame): Set width or height depending on orientation.
(setFrame): Handle horizontal case.
(dealloc): Handle horizontal case.
(judge): Handle horizontal case.
(setPosition): Rename pixel_height to pixel_length.
(sendScrollEventAtLoc): Handle horizontal case.
(mouseDown): Handle horizontal case and general tidy up of code.
(mouseDragged): Handle horizontal case. Call sendScrollEventAtLoc with
absolute pixel size instead of ratio.
* src/window.h: Remove NS check.
---
 lisp/scroll-bar.el  |   3 +-
 lisp/term/ns-win.el |  56 ++--------------
 src/nsterm.h        |   4 +-
 src/nsterm.m        | 182 ++++++++++++++++++++++++++++++++--------------------
 src/window.h        |   2 +-
 5 files changed, 120 insertions(+), 127 deletions(-)

diff --git a/lisp/scroll-bar.el b/lisp/scroll-bar.el
index 5cfa2c4..838f9bf 100644
--- a/lisp/scroll-bar.el
+++ b/lisp/scroll-bar.el
@@ -148,8 +148,7 @@ horizontal-scroll-bars-available-p
   "Return non-nil when horizontal scroll bars are available on this system."
   (and (display-graphic-p)
        (boundp 'x-toolkit-scroll-bars)
-       x-toolkit-scroll-bars
-       (not (eq (window-system) 'ns))))
+       x-toolkit-scroll-bars))
 
 (define-minor-mode horizontal-scroll-bar-mode
   "Toggle horizontal scroll bars on all frames (Horizontal Scroll Bar mode).
diff --git a/lisp/term/ns-win.el b/lisp/term/ns-win.el
index e737131..8b676ce 100644
--- a/lisp/term/ns-win.el
+++ b/lisp/term/ns-win.el
@@ -717,60 +717,12 @@ ns-paste-secondary
 
 ;;;; Scrollbar handling.
 
-(global-set-key [vertical-scroll-bar down-mouse-1] 'ns-handle-scroll-bar-event)
+(global-set-key [vertical-scroll-bar down-mouse-1] 'scroll-bar-toolkit-scroll)
+(global-set-key [horizontal-scroll-bar down-mouse-1] 'scroll-bar-toolkit-horizontal-scroll)
 (global-unset-key [vertical-scroll-bar mouse-1])
 (global-unset-key [vertical-scroll-bar drag-mouse-1])
-
-(declare-function scroll-bar-scale "scroll-bar" (num-denom whole))
-
-(defun ns-scroll-bar-move (event)
-  "Scroll the frame according to a Nextstep scroller event."
-  (interactive "e")
-  (let* ((pos (event-end event))
-         (window (nth 0 pos))
-         (scale (nth 2 pos)))
-    (with-current-buffer (window-buffer window)
-      (cond
-       ((eq (car scale) (cdr scale))
-	(goto-char (point-max)))
-       ((= (car scale) 0)
-	(goto-char (point-min)))
-       (t
-	(goto-char (+ (point-min) 1
-		      (scroll-bar-scale scale (- (point-max) (point-min)))))))
-      (beginning-of-line)
-      (set-window-start window (point))
-      (vertical-motion (/ (window-height window) 2) window))))
-
-(defun ns-handle-scroll-bar-event (event)
-  "Handle scroll bar EVENT to emulate Nextstep style scrolling."
-  (interactive "e")
-  (let* ((position (event-start event))
-	 (bar-part (nth 4 position))
-	 (window (nth 0 position))
-	 (old-window (selected-window)))
-    (cond
-     ((eq bar-part 'ratio)
-      (ns-scroll-bar-move event))
-     ((eq bar-part 'handle)
-      (if (eq window (selected-window))
-	  (track-mouse (ns-scroll-bar-move event))
-        ;; track-mouse faster for selected window, slower for unselected.
-	(ns-scroll-bar-move event)))
-     (t
-      (select-window window)
-      (cond
-       ((eq bar-part 'up)
-	(goto-char (window-start window))
-	(scroll-down 1))
-       ((eq bar-part 'above-handle)
-	(scroll-down))
-       ((eq bar-part 'below-handle)
-	(scroll-up))
-       ((eq bar-part 'down)
-	(goto-char (window-start window))
-	(scroll-up 1)))
-      (select-window old-window)))))
+(global-unset-key [horizontal-scroll-bar mouse-1])
+(global-unset-key [horizontal-scroll-bar drag-mouse-1])
 
 
 ;;;; Color support.
diff --git a/src/nsterm.h b/src/nsterm.h
index 0aea9cc..6cad337 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -676,11 +676,13 @@ char const * nstrace_fullscreen_type_name (int);
    /* offset to the bottom of knob of last mouse down */
    CGFloat last_mouse_offset;
    float min_portion;
-   int pixel_height;
+   int pixel_length;
    enum scroll_bar_part last_hit_part;
 
    BOOL condemned;
 
+   BOOL horizontal;
+
    /* optimize against excessive positioning calls generated by emacs */
    int em_position;
    int em_portion;
diff --git a/src/nsterm.m b/src/nsterm.m
index 34c5395..53ef136 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -4441,35 +4441,20 @@ ns_set_horizontal_scroll_bar (struct window *window,
 
   NSTRACE ("ns_set_horizontal_scroll_bar");
 
+  v = [view frame];
+
   /* Get dimensions.  */
-  window_box (window, ANY_AREA, 0, &window_x, &window_width, 0);
+  window_box (window, ANY_AREA, &window_x, 0, &window_width, 0);
   left = window_x;
   width = window_width;
-  height = WINDOW_CONFIG_SCROLL_BAR_LINES (window) * FRAME_LINE_HEIGHT (f);
-  top = WINDOW_SCROLL_BAR_AREA_Y (window);
+  height = [EmacsScroller scrollerWidth];
+  top = v.size.height - WINDOW_SCROLL_BAR_AREA_Y (window) - height;
 
   r = NSMakeRect (left, top, width, height);
-  /* the parent view is flipped, so we need to flip y value */
-  v = [view frame];
-  /* ??????? PXW/scrollbars !!!!!!!!!!!!!!!!!!!! */
-  r.origin.y = (v.size.height - r.size.height - r.origin.y);
 
   XSETWINDOW (win, window);
   block_input ();
 
-  if (WINDOW_TOTAL_COLS (window) < 5)
-    {
-      if (!NILP (window->horizontal_scroll_bar))
-        {
-          bar = XNS_SCROLL_BAR (window->horizontal_scroll_bar);
-          [bar removeFromSuperview];
-          wset_horizontal_scroll_bar (window, Qnil);
-        }
-      ns_clear_frame_area (f, left, top, width, height);
-      unblock_input ();
-      return;
-    }
-
   if (NILP (window->horizontal_scroll_bar))
     {
       if (width > 0 && height > 0)
@@ -4484,11 +4469,10 @@ ns_set_horizontal_scroll_bar (struct window *window,
       NSRect oldRect;
       bar = XNS_SCROLL_BAR (window->horizontal_scroll_bar);
       oldRect = [bar frame];
-      r.size.width = oldRect.size.width;
       if (FRAME_LIVE_P (f) && !NSEqualRects (oldRect, r))
         {
-          if (oldRect.origin.x != r.origin.x)
-              ns_clear_frame_area (f, left, top, width, height);
+          if (oldRect.origin.y != r.origin.y)
+            ns_clear_frame_area (f, left, top, width, height);
           [bar setFrame: r];
           update_p = YES;
         }
@@ -8110,12 +8094,21 @@ not_in_argv (NSString *arg)
   return r;
 }
 
-
 - initFrame: (NSRect )r window: (Lisp_Object)nwin
 {
   NSTRACE ("[EmacsScroller initFrame: window:]");
 
-  r.size.width = [EmacsScroller scrollerWidth];
+  if (r.size.width > r.size.height)
+    {
+      horizontal = YES;
+      r.size.height = [EmacsScroller scrollerWidth];
+    }
+  else
+    {
+      horizontal = NO;
+      r.size.width = [EmacsScroller scrollerWidth];
+    }
+
   [super initWithFrame: r/*NSMakeRect (0, 0, 0, 0)*/];
   [self setContinuous: YES];
   [self setEnabled: YES];
@@ -8131,9 +8124,12 @@ not_in_argv (NSString *arg)
 
   window = XWINDOW (nwin);
   condemned = NO;
-  pixel_height = NSHeight (r);
-  if (pixel_height == 0) pixel_height = 1;
-  min_portion = 20 / pixel_height;
+  if (horizontal)
+    pixel_length = NSWidth (r);
+  else
+    pixel_length = NSHeight (r);
+  if (pixel_length == 0) pixel_length = 1;
+  min_portion = 20 / pixel_length;
 
   frame = XFRAME (window->frame);
   if (FRAME_LIVE_P (frame))
@@ -8162,9 +8158,12 @@ not_in_argv (NSString *arg)
   NSTRACE ("[EmacsScroller setFrame:]");
 
 /*  block_input (); */
-  pixel_height = NSHeight (newRect);
-  if (pixel_height == 0) pixel_height = 1;
-  min_portion = 20 / pixel_height;
+  if (horizontal)
+    pixel_length = NSWidth (newRect);
+  else
+    pixel_length = NSHeight (newRect);
+  if (pixel_length == 0) pixel_length = 1;
+  min_portion = 20 / pixel_length;
   [super setFrame: newRect];
 /*  unblock_input (); */
 }
@@ -8174,7 +8173,12 @@ not_in_argv (NSString *arg)
 {
   NSTRACE ("[EmacsScroller dealloc]");
   if (window)
-    wset_vertical_scroll_bar (window, Qnil);
+    {
+      if (horizontal)
+        wset_horizontal_scroll_bar (window, Qnil);
+      else
+        wset_vertical_scroll_bar (window, Qnil);
+    }
   window = 0;
   [super dealloc];
 }
@@ -8209,7 +8213,12 @@ not_in_argv (NSString *arg)
       if (view != nil)
         view->scrollbarsNeedingUpdate++;
       if (window)
-        wset_vertical_scroll_bar (window, Qnil);
+        {
+          if (horizontal)
+            wset_horizontal_scroll_bar (window, Qnil);
+          else
+            wset_vertical_scroll_bar (window, Qnil);
+        }
       window = 0;
       [self removeFromSuperview];
       [self release];
@@ -8259,7 +8268,7 @@ not_in_argv (NSString *arg)
     {
       float pos;
       CGFloat por;
-      portion = max ((float)whole*min_portion/pixel_height, portion);
+      portion = max ((float)whole*min_portion/pixel_length, portion);
       pos = (float)position / (whole - portion);
       por = (CGFloat)portion/whole;
 #ifdef NS_IMPL_COCOA
@@ -8289,10 +8298,20 @@ not_in_argv (NSString *arg)
   XSETWINDOW (win, window);
   emacs_event->frame_or_window = win;
   emacs_event->timestamp = EV_TIMESTAMP (e);
-  emacs_event->kind = SCROLL_BAR_CLICK_EVENT;
   emacs_event->arg = Qnil;
-  XSETINT (emacs_event->x, loc * pixel_height);
-  XSETINT (emacs_event->y, pixel_height-20);
+
+  if (horizontal)
+    {
+      emacs_event->kind = HORIZONTAL_SCROLL_BAR_CLICK_EVENT;
+      XSETINT (emacs_event->x, em_whole * loc / pixel_length);
+      XSETINT (emacs_event->y, em_whole);
+    }
+  else
+    {
+      emacs_event->kind = SCROLL_BAR_CLICK_EVENT;
+      XSETINT (emacs_event->x, loc);
+      XSETINT (emacs_event->y, pixel_length-20);
+    }
 
   if (q_event_ptr)
     {
@@ -8355,15 +8374,15 @@ not_in_argv (NSString *arg)
   switch (part)
     {
     case NSScrollerDecrementPage:
-        last_hit_part = scroll_bar_above_handle; inc = -1.0; break;
+      last_hit_part = horizontal ? scroll_bar_before_handle : scroll_bar_above_handle; break;
     case NSScrollerIncrementPage:
-        last_hit_part = scroll_bar_below_handle; inc = 1.0; break;
+      last_hit_part = horizontal ? scroll_bar_after_handle : scroll_bar_below_handle; break;
     case NSScrollerDecrementLine:
-      last_hit_part = scroll_bar_up_arrow; inc = -0.1; break;
+      last_hit_part = horizontal ? scroll_bar_left_arrow : scroll_bar_up_arrow; break;
     case NSScrollerIncrementLine:
-      last_hit_part = scroll_bar_down_arrow; inc = 0.1; break;
+      last_hit_part = horizontal ? scroll_bar_right_arrow : scroll_bar_down_arrow; break;
     case NSScrollerKnob:
-      last_hit_part = scroll_bar_handle; break;
+      last_hit_part = horizontal ? scroll_bar_horizontal_handle : scroll_bar_handle; break;
     case NSScrollerKnobSlot:  /* GNUstep-only */
       last_hit_part = scroll_bar_move_ratio; break;
     default:  /* NSScrollerNoPart? */
@@ -8372,36 +8391,34 @@ not_in_argv (NSString *arg)
       return;
     }
 
-  if (inc != 0.0)
-    {
-      pos = 0;      /* ignored */
-
-      /* set a timer to repeat, as we can't let superclass do this modally */
-      scroll_repeat_entry
-	= [[NSTimer scheduledTimerWithTimeInterval: SCROLL_BAR_FIRST_DELAY
-                                            target: self
-                                          selector: @selector (repeatScroll:)
-                                          userInfo: 0
-                                           repeats: YES]
-	    retain];
-    }
-  else
+  if (part == NSScrollerKnob || part == NSScrollerKnobSlot)
     {
       /* handle, or on GNUstep possibly slot */
       NSEvent *fake_event;
+      int length;
 
       /* compute float loc in slot and mouse offset on knob */
       sr = [self convertRect: [self rectForPart: NSScrollerKnobSlot]
                       toView: nil];
-      loc = NSHeight (sr) - ([e locationInWindow].y - NSMinY (sr));
+      if (horizontal)
+        {
+          length = NSWidth (sr);
+          loc = ([e locationInWindow].x - NSMinX (sr));
+        }
+      else
+        {
+          length = NSHeight (sr);
+          loc = length - ([e locationInWindow].y - NSMinY (sr));
+        }
+
       if (loc <= 0.0)
         {
           loc = 0.0;
           edge = -1;
         }
-      else if (loc >= NSHeight (sr))
+      else if (loc >= length)
         {
-          loc = NSHeight (sr);
+          loc = length;
           edge = 1;
         }
 
@@ -8411,17 +8428,16 @@ not_in_argv (NSString *arg)
         {
           kr = [self convertRect: [self rectForPart: NSScrollerKnob]
                           toView: nil];
-          kloc = NSHeight (kr) - ([e locationInWindow].y - NSMinY (kr));
+          if (horizontal)
+            kloc = ([e locationInWindow].x - NSMinX (kr));
+          else
+            kloc = NSHeight (kr) - ([e locationInWindow].y - NSMinY (kr));
         }
       last_mouse_offset = kloc;
 
-      /* if knob, tell emacs a location offset by knob pos
-         (to indicate top of handle) */
-      if (part == NSScrollerKnob)
-          pos = (loc - last_mouse_offset) / NSHeight (sr);
-      else
-        /* else this is a slot click on GNUstep: go straight there */
-        pos = loc / NSHeight (sr);
+      if (part != NSScrollerKnob)
+        /* this is a slot click on GNUstep: go straight there */
+        pos = loc;
 
       /* send a fake mouse-up to super to preempt modal -trackKnob: mode */
       fake_event = [NSEvent mouseEventWithType: NSLeftMouseUp
@@ -8435,6 +8451,19 @@ not_in_argv (NSString *arg)
                                       pressure: [e pressure]];
       [super mouseUp: fake_event];
     }
+  else
+    {
+      pos = 0;      /* ignored */
+
+      /* set a timer to repeat, as we can't let superclass do this modally */
+      scroll_repeat_entry
+	= [[NSTimer scheduledTimerWithTimeInterval: SCROLL_BAR_FIRST_DELAY
+                                            target: self
+                                          selector: @selector (repeatScroll:)
+                                          userInfo: 0
+                                           repeats: YES]
+	    retain];
+    }
 
   if (part != NSScrollerKnob)
     [self sendScrollEventAtLoc: pos fromEvent: e];
@@ -8446,23 +8475,34 @@ not_in_argv (NSString *arg)
 {
     NSRect sr;
     double loc, pos;
+    int length;
 
     NSTRACE ("[EmacsScroller mouseDragged:]");
 
       sr = [self convertRect: [self rectForPart: NSScrollerKnobSlot]
                       toView: nil];
-      loc = NSHeight (sr) - ([e locationInWindow].y - NSMinY (sr));
+
+      if (horizontal)
+        {
+          length = NSWidth (sr);
+          loc = ([e locationInWindow].x - NSMinX (sr));
+        }
+      else
+        {
+          length = NSHeight (sr);
+          loc = length - ([e locationInWindow].y - NSMinY (sr));
+        }
 
       if (loc <= 0.0)
         {
           loc = 0.0;
         }
-      else if (loc >= NSHeight (sr) + last_mouse_offset)
+      else if (loc >= length + last_mouse_offset)
         {
-          loc = NSHeight (sr) + last_mouse_offset;
+          loc = length + last_mouse_offset;
         }
 
-      pos = (loc - last_mouse_offset) / NSHeight (sr);
+      pos = (loc - last_mouse_offset);
       [self sendScrollEventAtLoc: pos fromEvent: e];
 }
 
diff --git a/src/window.h b/src/window.h
index 0cfff88..a1c4aaa 100644
--- a/src/window.h
+++ b/src/window.h
@@ -786,7 +786,7 @@ wset_next_buffers (struct window *w, Lisp_Object val)
    || WINDOW_HAS_VERTICAL_SCROLL_BAR_ON_RIGHT (W))
 
 #if (defined (HAVE_WINDOW_SYSTEM)					\
-     && ((defined (USE_TOOLKIT_SCROLL_BARS) && !defined (HAVE_NS))	\
+     && ((defined (USE_TOOLKIT_SCROLL_BARS))	\
 	 || defined (HAVE_NTGUI)))
 # define USE_HORIZONTAL_SCROLL_BARS true
 #else
-- 

My attempt at enabling horizontal scroll-bars in the NS port. This
patch also removes the custom NS scroll-bar handlers in favour of the
standard ones. I think this *may* cause behaviour of the vertical
scroll-bars to change slightly.

I've tried to clean up some of the code as there were a few places
where it didn't make a whole lot of sense to me. I guess that's due to
the code I started with being a partial implementation.
-- 
Alan Third

bug#22660: [PATCH] Implement horizontal scroll-bars in NS port

[martin rudalics]

 > My attempt at enabling horizontal scroll-bars in the NS port. This
 > patch also removes the custom NS scroll-bar handlers in favour of the
 > standard ones. I think this *may* cause behaviour of the vertical
 > scroll-bars to change slightly.
 >
 > I've tried to clean up some of the code as there were a few places
 > where it didn't make a whole lot of sense to me. I guess that's due to
 > the code I started with being a partial implementation.

Thank you.  With the limited testing capabilities of GNUStep it seems to
work.  Or better: If everything in the GNUStep worked as well as your
patch, the GNUStep port might even become usable one day ;-)

There is one aspect which might be part of the design but appears
nevertheless distracting for somone working mainly on other platforms:
When the entire buffer is visible, no sliders/rulers are shown.  Now,
some people want that in this case the scrollbar area(s) get removed
completely.  Personally, I'd prefer the sliders/ruler cover the entire
scroll bar area with the arrows possibly removed.  But maybe the current
behavior (which is so here ever since for the vertical case) is standard
on NS so don't let my remarks distract you.

Personally, I'd vote for including this in Emacs-25.  It's a completely
self-confined, local patch and we could avoid answering questions like
"why don't these work on Macs" after the forthcoming release.

In any case, many thanks for the accomplishment, martin

bug#22660: [PATCH] Implement horizontal scroll-bars in NS port

[Alan Third]

On 28 April 2016 at 07:33, martin rudalics <rudalics@gmx.at> wrote:
> Thank you.  With the limited testing capabilities of GNUStep it seems to
> work.  Or better: If everything in the GNUStep worked as well as your
> patch, the GNUStep port might even become usable one day ;-)

I'm just glad I didn't break anything! I think I'll look into whether
I can set up a GNUStep development environment so I can test against it.

> There is one aspect which might be part of the design but appears
> nevertheless distracting for somone working mainly on other platforms:
> When the entire buffer is visible, no sliders/rulers are shown.  Now,
> some people want that in this case the scrollbar area(s) get removed
> completely.  Personally, I'd prefer the sliders/ruler cover the entire
> scroll bar area with the arrows possibly removed.  But maybe the current
> behavior (which is so here ever since for the vertical case) is standard
> on NS so don't let my remarks distract you.

I believe the only behaviour of that sort is that the scroll-bars get
disabled when they cover the whole area. On OS X that just means
they're greyed out, but I guess GNUStep removes them completely. It
could be worth changing that section of the code to be Cocoa only, so
in GNUStep they stay visible.

-- 
Alan Third

bug#22660: [PATCH] Implement horizontal scroll-bars in NS port

[Alan Third]

On Thu, Apr 28, 2016 at 10:16:40AM +0100, Alan Third wrote:
> On 28 April 2016 at 07:33, martin rudalics <rudalics@gmx.at> wrote:
>
> I believe the only behaviour of that sort is that the scroll-bars get
> disabled when they cover the whole area. On OS X that just means
> they're greyed out, but I guess GNUStep removes them completely. It
> could be worth changing that section of the code to be Cocoa only, so
> in GNUStep they stay visible.

Having checked the code, this is wrong. Vertical scroll-bars are only
automatically disabled if there are less than 5 lines in the window.
It must be part of GNUStep's standard behaviour to hide the
scroll-bars. It wouldn't be something you can configure, would it?
GNUStep's look seems quite configurable.
-- 
Alan Third

bug#22660: [PATCH] Implement horizontal scroll-bars in NS port

[martin rudalics]

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

 >> I believe the only behaviour of that sort is that the scroll-bars get
 >> disabled when they cover the whole area. On OS X that just means
 >> they're greyed out, but I guess GNUStep removes them completely. It
 >> could be worth changing that section of the code to be Cocoa only, so
 >> in GNUStep they stay visible.
 >
 > Having checked the code, this is wrong. Vertical scroll-bars are only
 > automatically disabled if there are less than 5 lines in the window.
 > It must be part of GNUStep's standard behaviour to hide the
 > scroll-bars. It wouldn't be something you can configure, would it?
 > GNUStep's look seems quite configurable.

IIUC there are two issues involved.  In GNUStep-scrolllbars-1.png, the
lower window has less than 5 lines and exhibits the behavior you
describe here with the scrollbar area completely empty.  The upper
window has more than 5 lines and the scroll bar background is drawn but
no slider is shown.

GNUStep-scrolllbars-2.png depicts another issue which happens
immediately after enabling horizontal scroll bars via the menu.  (BTW,
vertical scrollbar menu entries are never checked here for whatever
reason, the horizontal menu entry is.)  As you can see, the vertical
scroll bar covers the area used by the horizontal one.  After a few
operations like resizing or splitting, that effect is gone and scroll
bars appear correctly as in GNUStep-scrolllbars-3.png.

martin

[-- Attachment #2: GNUStep-scrolllbars-1.png --]
[-- Type: image/png, Size: 21948 bytes --]

[-- Attachment #3: GNUStep-scrolllbars-2.png --]
[-- Type: image/png, Size: 18999 bytes --]

[-- Attachment #4: GNUStep-scrolllbars-3.png --]
[-- Type: image/png, Size: 36325 bytes --]

bug#22660: [PATCH] Implement horizontal scroll-bars in NS port

[Alan Third]

* lisp/scroll-bar.el (horizontal-scroll-bars-available-p): Remove NS
check.
* lisp/term/ns-win.el: Remove custom NS scroll-bar handlers and bind
scroll-bar mouse clicks to standard handlers.
* src/nsterm.h (EmacsScroller): Add 'horizontal' property and rename
pixel_height to pixel_length.
* src/nsterm.m (ns_set_horizontal_scroll_bar): Rewrite to handle
horizontal scrollers correctly.
(ns_set_vertical_scroll_bar): Set width to actual scroller width.
(setFrame): Handle horizontal case.
(dealloc): Handle horizontal case.
(judge): Handle horizontal case.
(setPosition): Rename pixel_height to pixel_length.
(sendScrollEventAtLoc): Handle horizontal case.
(mouseDown): Handle horizontal case and general tidy up of code.
(mouseDragged): Handle horizontal case. Call sendScrollEventAtLoc with
absolute pixel size instead of ratio.
* src/window.h: Remove NS check.
---
 lisp/scroll-bar.el  |   3 +-
 lisp/term/ns-win.el |  56 ++--------------
 src/nsterm.h        |   4 +-
 src/nsterm.m        | 184 ++++++++++++++++++++++++++++++++--------------------
 src/window.h        |   2 +-
 5 files changed, 123 insertions(+), 126 deletions(-)

diff --git a/lisp/scroll-bar.el b/lisp/scroll-bar.el
index 5cfa2c4..838f9bf 100644
--- a/lisp/scroll-bar.el
+++ b/lisp/scroll-bar.el
@@ -148,8 +148,7 @@ horizontal-scroll-bars-available-p
   "Return non-nil when horizontal scroll bars are available on this system."
   (and (display-graphic-p)
        (boundp 'x-toolkit-scroll-bars)
-       x-toolkit-scroll-bars
-       (not (eq (window-system) 'ns))))
+       x-toolkit-scroll-bars))
 
 (define-minor-mode horizontal-scroll-bar-mode
   "Toggle horizontal scroll bars on all frames (Horizontal Scroll Bar mode).
diff --git a/lisp/term/ns-win.el b/lisp/term/ns-win.el
index e737131..8b676ce 100644
--- a/lisp/term/ns-win.el
+++ b/lisp/term/ns-win.el
@@ -717,60 +717,12 @@ ns-paste-secondary
 
 ;;;; Scrollbar handling.
 
-(global-set-key [vertical-scroll-bar down-mouse-1] 'ns-handle-scroll-bar-event)
+(global-set-key [vertical-scroll-bar down-mouse-1] 'scroll-bar-toolkit-scroll)
+(global-set-key [horizontal-scroll-bar down-mouse-1] 'scroll-bar-toolkit-horizontal-scroll)
 (global-unset-key [vertical-scroll-bar mouse-1])
 (global-unset-key [vertical-scroll-bar drag-mouse-1])
-
-(declare-function scroll-bar-scale "scroll-bar" (num-denom whole))
-
-(defun ns-scroll-bar-move (event)
-  "Scroll the frame according to a Nextstep scroller event."
-  (interactive "e")
-  (let* ((pos (event-end event))
-         (window (nth 0 pos))
-         (scale (nth 2 pos)))
-    (with-current-buffer (window-buffer window)
-      (cond
-       ((eq (car scale) (cdr scale))
-	(goto-char (point-max)))
-       ((= (car scale) 0)
-	(goto-char (point-min)))
-       (t
-	(goto-char (+ (point-min) 1
-		      (scroll-bar-scale scale (- (point-max) (point-min)))))))
-      (beginning-of-line)
-      (set-window-start window (point))
-      (vertical-motion (/ (window-height window) 2) window))))
-
-(defun ns-handle-scroll-bar-event (event)
-  "Handle scroll bar EVENT to emulate Nextstep style scrolling."
-  (interactive "e")
-  (let* ((position (event-start event))
-	 (bar-part (nth 4 position))
-	 (window (nth 0 position))
-	 (old-window (selected-window)))
-    (cond
-     ((eq bar-part 'ratio)
-      (ns-scroll-bar-move event))
-     ((eq bar-part 'handle)
-      (if (eq window (selected-window))
-	  (track-mouse (ns-scroll-bar-move event))
-        ;; track-mouse faster for selected window, slower for unselected.
-	(ns-scroll-bar-move event)))
-     (t
-      (select-window window)
-      (cond
-       ((eq bar-part 'up)
-	(goto-char (window-start window))
-	(scroll-down 1))
-       ((eq bar-part 'above-handle)
-	(scroll-down))
-       ((eq bar-part 'below-handle)
-	(scroll-up))
-       ((eq bar-part 'down)
-	(goto-char (window-start window))
-	(scroll-up 1)))
-      (select-window old-window)))))
+(global-unset-key [horizontal-scroll-bar mouse-1])
+(global-unset-key [horizontal-scroll-bar drag-mouse-1])
 
 
 ;;;; Color support.
diff --git a/src/nsterm.h b/src/nsterm.h
index 0aea9cc..6cad337 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -676,11 +676,13 @@ char const * nstrace_fullscreen_type_name (int);
    /* offset to the bottom of knob of last mouse down */
    CGFloat last_mouse_offset;
    float min_portion;
-   int pixel_height;
+   int pixel_length;
    enum scroll_bar_part last_hit_part;
 
    BOOL condemned;
 
+   BOOL horizontal;
+
    /* optimize against excessive positioning calls generated by emacs */
    int em_position;
    int em_portion;
diff --git a/src/nsterm.m b/src/nsterm.m
index 34c5395..f7b3db6 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -4351,7 +4351,7 @@ ns_set_vertical_scroll_bar (struct window *window,
   window_box (window, ANY_AREA, 0, &window_y, 0, &window_height);
   top = window_y;
   height = window_height;
-  width = WINDOW_CONFIG_SCROLL_BAR_COLS (window) * FRAME_COLUMN_WIDTH (f);
+  width = [EmacsScroller scrollerWidth];
   left = WINDOW_SCROLL_BAR_AREA_X (window);
 
   r = NSMakeRect (left, top, width, height);
@@ -4442,34 +4442,20 @@ ns_set_horizontal_scroll_bar (struct window *window,
   NSTRACE ("ns_set_horizontal_scroll_bar");
 
   /* Get dimensions.  */
-  window_box (window, ANY_AREA, 0, &window_x, &window_width, 0);
+  window_box (window, ANY_AREA, &window_x, 0, &window_width, 0);
   left = window_x;
   width = window_width;
-  height = WINDOW_CONFIG_SCROLL_BAR_LINES (window) * FRAME_LINE_HEIGHT (f);
+  height = [EmacsScroller scrollerWidth];
   top = WINDOW_SCROLL_BAR_AREA_Y (window);
 
   r = NSMakeRect (left, top, width, height);
   /* the parent view is flipped, so we need to flip y value */
   v = [view frame];
-  /* ??????? PXW/scrollbars !!!!!!!!!!!!!!!!!!!! */
   r.origin.y = (v.size.height - r.size.height - r.origin.y);
 
   XSETWINDOW (win, window);
   block_input ();
 
-  if (WINDOW_TOTAL_COLS (window) < 5)
-    {
-      if (!NILP (window->horizontal_scroll_bar))
-        {
-          bar = XNS_SCROLL_BAR (window->horizontal_scroll_bar);
-          [bar removeFromSuperview];
-          wset_horizontal_scroll_bar (window, Qnil);
-        }
-      ns_clear_frame_area (f, left, top, width, height);
-      unblock_input ();
-      return;
-    }
-
   if (NILP (window->horizontal_scroll_bar))
     {
       if (width > 0 && height > 0)
@@ -4484,16 +4470,22 @@ ns_set_horizontal_scroll_bar (struct window *window,
       NSRect oldRect;
       bar = XNS_SCROLL_BAR (window->horizontal_scroll_bar);
       oldRect = [bar frame];
-      r.size.width = oldRect.size.width;
       if (FRAME_LIVE_P (f) && !NSEqualRects (oldRect, r))
         {
-          if (oldRect.origin.x != r.origin.x)
-              ns_clear_frame_area (f, left, top, width, height);
+          if (oldRect.origin.y != r.origin.y)
+            ns_clear_frame_area (f, left, top, width, height);
           [bar setFrame: r];
           update_p = YES;
         }
     }
 
+  /* If there are both horizontal and vertical scroll-bars they leave
+     a square that belongs to neither. We need to clear it otherwise
+     it fills with junk. */
+  if (!NILP (window->vertical_scroll_bar))
+    ns_clear_frame_area (f, WINDOW_SCROLL_BAR_AREA_X (window), top,
+                         [EmacsScroller scrollerWidth], height);
+
   if (update_p)
     [bar setPosition: position portion: portion whole: whole];
   unblock_input ();
@@ -4531,13 +4523,15 @@ ns_redeem_scroll_bar (struct window *window)
 {
   id bar;
   NSTRACE ("ns_redeem_scroll_bar");
-  if (!NILP (window->vertical_scroll_bar))
+  if (!NILP (window->vertical_scroll_bar)
+      && WINDOW_HAS_VERTICAL_SCROLL_BAR (window))
     {
       bar = XNS_SCROLL_BAR (window->vertical_scroll_bar);
       [bar reprieve];
     }
 
-  if (!NILP (window->horizontal_scroll_bar))
+  if (!NILP (window->horizontal_scroll_bar)
+      && WINDOW_HAS_HORIZONTAL_SCROLL_BAR (window))
     {
       bar = XNS_SCROLL_BAR (window->horizontal_scroll_bar);
       [bar reprieve];
@@ -8110,12 +8104,15 @@ not_in_argv (NSString *arg)
   return r;
 }
 
-
 - initFrame: (NSRect )r window: (Lisp_Object)nwin
 {
   NSTRACE ("[EmacsScroller initFrame: window:]");
 
-  r.size.width = [EmacsScroller scrollerWidth];
+  if (r.size.width > r.size.height)
+      horizontal = YES;
+  else
+      horizontal = NO;
+
   [super initWithFrame: r/*NSMakeRect (0, 0, 0, 0)*/];
   [self setContinuous: YES];
   [self setEnabled: YES];
@@ -8131,9 +8128,12 @@ not_in_argv (NSString *arg)
 
   window = XWINDOW (nwin);
   condemned = NO;
-  pixel_height = NSHeight (r);
-  if (pixel_height == 0) pixel_height = 1;
-  min_portion = 20 / pixel_height;
+  if (horizontal)
+    pixel_length = NSWidth (r);
+  else
+    pixel_length = NSHeight (r);
+  if (pixel_length == 0) pixel_length = 1;
+  min_portion = 20 / pixel_length;
 
   frame = XFRAME (window->frame);
   if (FRAME_LIVE_P (frame))
@@ -8162,9 +8162,12 @@ not_in_argv (NSString *arg)
   NSTRACE ("[EmacsScroller setFrame:]");
 
 /*  block_input (); */
-  pixel_height = NSHeight (newRect);
-  if (pixel_height == 0) pixel_height = 1;
-  min_portion = 20 / pixel_height;
+  if (horizontal)
+    pixel_length = NSWidth (newRect);
+  else
+    pixel_length = NSHeight (newRect);
+  if (pixel_length == 0) pixel_length = 1;
+  min_portion = 20 / pixel_length;
   [super setFrame: newRect];
 /*  unblock_input (); */
 }
@@ -8174,7 +8177,12 @@ not_in_argv (NSString *arg)
 {
   NSTRACE ("[EmacsScroller dealloc]");
   if (window)
-    wset_vertical_scroll_bar (window, Qnil);
+    {
+      if (horizontal)
+        wset_horizontal_scroll_bar (window, Qnil);
+      else
+        wset_vertical_scroll_bar (window, Qnil);
+    }
   window = 0;
   [super dealloc];
 }
@@ -8209,7 +8217,12 @@ not_in_argv (NSString *arg)
       if (view != nil)
         view->scrollbarsNeedingUpdate++;
       if (window)
-        wset_vertical_scroll_bar (window, Qnil);
+        {
+          if (horizontal)
+            wset_horizontal_scroll_bar (window, Qnil);
+          else
+            wset_vertical_scroll_bar (window, Qnil);
+        }
       window = 0;
       [self removeFromSuperview];
       [self release];
@@ -8259,7 +8272,7 @@ not_in_argv (NSString *arg)
     {
       float pos;
       CGFloat por;
-      portion = max ((float)whole*min_portion/pixel_height, portion);
+      portion = max ((float)whole*min_portion/pixel_length, portion);
       pos = (float)position / (whole - portion);
       por = (CGFloat)portion/whole;
 #ifdef NS_IMPL_COCOA
@@ -8289,10 +8302,20 @@ not_in_argv (NSString *arg)
   XSETWINDOW (win, window);
   emacs_event->frame_or_window = win;
   emacs_event->timestamp = EV_TIMESTAMP (e);
-  emacs_event->kind = SCROLL_BAR_CLICK_EVENT;
   emacs_event->arg = Qnil;
-  XSETINT (emacs_event->x, loc * pixel_height);
-  XSETINT (emacs_event->y, pixel_height-20);
+
+  if (horizontal)
+    {
+      emacs_event->kind = HORIZONTAL_SCROLL_BAR_CLICK_EVENT;
+      XSETINT (emacs_event->x, em_whole * loc / pixel_length);
+      XSETINT (emacs_event->y, em_whole);
+    }
+  else
+    {
+      emacs_event->kind = SCROLL_BAR_CLICK_EVENT;
+      XSETINT (emacs_event->x, loc);
+      XSETINT (emacs_event->y, pixel_length-20);
+    }
 
   if (q_event_ptr)
     {
@@ -8355,15 +8378,15 @@ not_in_argv (NSString *arg)
   switch (part)
     {
     case NSScrollerDecrementPage:
-        last_hit_part = scroll_bar_above_handle; inc = -1.0; break;
+      last_hit_part = horizontal ? scroll_bar_before_handle : scroll_bar_above_handle; break;
     case NSScrollerIncrementPage:
-        last_hit_part = scroll_bar_below_handle; inc = 1.0; break;
+      last_hit_part = horizontal ? scroll_bar_after_handle : scroll_bar_below_handle; break;
     case NSScrollerDecrementLine:
-      last_hit_part = scroll_bar_up_arrow; inc = -0.1; break;
+      last_hit_part = horizontal ? scroll_bar_left_arrow : scroll_bar_up_arrow; break;
     case NSScrollerIncrementLine:
-      last_hit_part = scroll_bar_down_arrow; inc = 0.1; break;
+      last_hit_part = horizontal ? scroll_bar_right_arrow : scroll_bar_down_arrow; break;
     case NSScrollerKnob:
-      last_hit_part = scroll_bar_handle; break;
+      last_hit_part = horizontal ? scroll_bar_horizontal_handle : scroll_bar_handle; break;
     case NSScrollerKnobSlot:  /* GNUstep-only */
       last_hit_part = scroll_bar_move_ratio; break;
     default:  /* NSScrollerNoPart? */
@@ -8372,36 +8395,34 @@ not_in_argv (NSString *arg)
       return;
     }
 
-  if (inc != 0.0)
-    {
-      pos = 0;      /* ignored */
-
-      /* set a timer to repeat, as we can't let superclass do this modally */
-      scroll_repeat_entry
-	= [[NSTimer scheduledTimerWithTimeInterval: SCROLL_BAR_FIRST_DELAY
-                                            target: self
-                                          selector: @selector (repeatScroll:)
-                                          userInfo: 0
-                                           repeats: YES]
-	    retain];
-    }
-  else
+  if (part == NSScrollerKnob || part == NSScrollerKnobSlot)
     {
       /* handle, or on GNUstep possibly slot */
       NSEvent *fake_event;
+      int length;
 
       /* compute float loc in slot and mouse offset on knob */
       sr = [self convertRect: [self rectForPart: NSScrollerKnobSlot]
                       toView: nil];
-      loc = NSHeight (sr) - ([e locationInWindow].y - NSMinY (sr));
+      if (horizontal)
+        {
+          length = NSWidth (sr);
+          loc = ([e locationInWindow].x - NSMinX (sr));
+        }
+      else
+        {
+          length = NSHeight (sr);
+          loc = length - ([e locationInWindow].y - NSMinY (sr));
+        }
+
       if (loc <= 0.0)
         {
           loc = 0.0;
           edge = -1;
         }
-      else if (loc >= NSHeight (sr))
+      else if (loc >= length)
         {
-          loc = NSHeight (sr);
+          loc = length;
           edge = 1;
         }
 
@@ -8411,17 +8432,16 @@ not_in_argv (NSString *arg)
         {
           kr = [self convertRect: [self rectForPart: NSScrollerKnob]
                           toView: nil];
-          kloc = NSHeight (kr) - ([e locationInWindow].y - NSMinY (kr));
+          if (horizontal)
+            kloc = ([e locationInWindow].x - NSMinX (kr));
+          else
+            kloc = NSHeight (kr) - ([e locationInWindow].y - NSMinY (kr));
         }
       last_mouse_offset = kloc;
 
-      /* if knob, tell emacs a location offset by knob pos
-         (to indicate top of handle) */
-      if (part == NSScrollerKnob)
-          pos = (loc - last_mouse_offset) / NSHeight (sr);
-      else
-        /* else this is a slot click on GNUstep: go straight there */
-        pos = loc / NSHeight (sr);
+      if (part != NSScrollerKnob)
+        /* this is a slot click on GNUstep: go straight there */
+        pos = loc;
 
       /* send a fake mouse-up to super to preempt modal -trackKnob: mode */
       fake_event = [NSEvent mouseEventWithType: NSLeftMouseUp
@@ -8435,6 +8455,19 @@ not_in_argv (NSString *arg)
                                       pressure: [e pressure]];
       [super mouseUp: fake_event];
     }
+  else
+    {
+      pos = 0;      /* ignored */
+
+      /* set a timer to repeat, as we can't let superclass do this modally */
+      scroll_repeat_entry
+	= [[NSTimer scheduledTimerWithTimeInterval: SCROLL_BAR_FIRST_DELAY
+                                            target: self
+                                          selector: @selector (repeatScroll:)
+                                          userInfo: 0
+                                           repeats: YES]
+	    retain];
+    }
 
   if (part != NSScrollerKnob)
     [self sendScrollEventAtLoc: pos fromEvent: e];
@@ -8446,23 +8479,34 @@ not_in_argv (NSString *arg)
 {
     NSRect sr;
     double loc, pos;
+    int length;
 
     NSTRACE ("[EmacsScroller mouseDragged:]");
 
       sr = [self convertRect: [self rectForPart: NSScrollerKnobSlot]
                       toView: nil];
-      loc = NSHeight (sr) - ([e locationInWindow].y - NSMinY (sr));
+
+      if (horizontal)
+        {
+          length = NSWidth (sr);
+          loc = ([e locationInWindow].x - NSMinX (sr));
+        }
+      else
+        {
+          length = NSHeight (sr);
+          loc = length - ([e locationInWindow].y - NSMinY (sr));
+        }
 
       if (loc <= 0.0)
         {
           loc = 0.0;
         }
-      else if (loc >= NSHeight (sr) + last_mouse_offset)
+      else if (loc >= length + last_mouse_offset)
         {
-          loc = NSHeight (sr) + last_mouse_offset;
+          loc = length + last_mouse_offset;
         }
 
-      pos = (loc - last_mouse_offset) / NSHeight (sr);
+      pos = (loc - last_mouse_offset);
       [self sendScrollEventAtLoc: pos fromEvent: e];
 }
 
diff --git a/src/window.h b/src/window.h
index 0cfff88..a1c4aaa 100644
--- a/src/window.h
+++ b/src/window.h
@@ -786,7 +786,7 @@ wset_next_buffers (struct window *w, Lisp_Object val)
    || WINDOW_HAS_VERTICAL_SCROLL_BAR_ON_RIGHT (W))
 
 #if (defined (HAVE_WINDOW_SYSTEM)					\
-     && ((defined (USE_TOOLKIT_SCROLL_BARS) && !defined (HAVE_NS))	\
+     && ((defined (USE_TOOLKIT_SCROLL_BARS))	\
 	 || defined (HAVE_NTGUI)))
 # define USE_HORIZONTAL_SCROLL_BARS true
 #else
-- 

Hopefully fixed the problem with the last patch and a few other bugs I
found while working on it.

-- 
Alan Third

bug#22660: [PATCH v2] Implement horizontal scroll-bars in NS port

[Alan Third]

* lisp/scroll-bar.el (horizontal-scroll-bars-available-p): Remove NS
check.
* lisp/term/ns-win.el: Remove custom NS scroll-bar handlers and bind
scroll-bar mouse clicks to standard handlers.
* src/nsterm.h (EmacsScroller): Add 'horizontal' property and rename
pixel_height to pixel_length.
* src/nsterm.m (x_set_window_size): Remove left-hand scroll-bar code. It
caused scroll-bars to be over-drawn and the best working solution
appears to be complete removal.
(ns_set_horizontal_scroll_bar): Rewrite to handle horizontal scrollers
correctly.
(ns_set_vertical_scroll_bar): Set width to actual scroller width.
(setFrame): Handle horizontal case.
(dealloc): Handle horizontal case.
(judge): Handle horizontal case.
(setPosition): Rename pixel_height to pixel_length.
(sendScrollEventAtLoc): Handle horizontal case.
(mouseDown): Handle horizontal case and general tidy up of code.
(mouseDragged): Handle horizontal case. Call sendScrollEventAtLoc with
absolute pixel size instead of ratio.
* src/window.h: Remove NS check.
---
 lisp/scroll-bar.el  |   3 +-
 lisp/term/ns-win.el |  56 ++-------------
 src/nsterm.h        |   4 +-
 src/nsterm.m        | 201 +++++++++++++++++++++++++++++-----------------------
 src/window.h        |   2 +-
 5 files changed, 123 insertions(+), 143 deletions(-)

diff --git a/lisp/scroll-bar.el b/lisp/scroll-bar.el
index 5cfa2c4..838f9bf 100644
--- a/lisp/scroll-bar.el
+++ b/lisp/scroll-bar.el
@@ -148,8 +148,7 @@ horizontal-scroll-bars-available-p
   "Return non-nil when horizontal scroll bars are available on this system."
   (and (display-graphic-p)
        (boundp 'x-toolkit-scroll-bars)
-       x-toolkit-scroll-bars
-       (not (eq (window-system) 'ns))))
+       x-toolkit-scroll-bars))
 
 (define-minor-mode horizontal-scroll-bar-mode
   "Toggle horizontal scroll bars on all frames (Horizontal Scroll Bar mode).
diff --git a/lisp/term/ns-win.el b/lisp/term/ns-win.el
index e737131..8b676ce 100644
--- a/lisp/term/ns-win.el
+++ b/lisp/term/ns-win.el
@@ -717,60 +717,12 @@ ns-paste-secondary
 
 ;;;; Scrollbar handling.
 
-(global-set-key [vertical-scroll-bar down-mouse-1] 'ns-handle-scroll-bar-event)
+(global-set-key [vertical-scroll-bar down-mouse-1] 'scroll-bar-toolkit-scroll)
+(global-set-key [horizontal-scroll-bar down-mouse-1] 'scroll-bar-toolkit-horizontal-scroll)
 (global-unset-key [vertical-scroll-bar mouse-1])
 (global-unset-key [vertical-scroll-bar drag-mouse-1])
-
-(declare-function scroll-bar-scale "scroll-bar" (num-denom whole))
-
-(defun ns-scroll-bar-move (event)
-  "Scroll the frame according to a Nextstep scroller event."
-  (interactive "e")
-  (let* ((pos (event-end event))
-         (window (nth 0 pos))
-         (scale (nth 2 pos)))
-    (with-current-buffer (window-buffer window)
-      (cond
-       ((eq (car scale) (cdr scale))
-	(goto-char (point-max)))
-       ((= (car scale) 0)
-	(goto-char (point-min)))
-       (t
-	(goto-char (+ (point-min) 1
-		      (scroll-bar-scale scale (- (point-max) (point-min)))))))
-      (beginning-of-line)
-      (set-window-start window (point))
-      (vertical-motion (/ (window-height window) 2) window))))
-
-(defun ns-handle-scroll-bar-event (event)
-  "Handle scroll bar EVENT to emulate Nextstep style scrolling."
-  (interactive "e")
-  (let* ((position (event-start event))
-	 (bar-part (nth 4 position))
-	 (window (nth 0 position))
-	 (old-window (selected-window)))
-    (cond
-     ((eq bar-part 'ratio)
-      (ns-scroll-bar-move event))
-     ((eq bar-part 'handle)
-      (if (eq window (selected-window))
-	  (track-mouse (ns-scroll-bar-move event))
-        ;; track-mouse faster for selected window, slower for unselected.
-	(ns-scroll-bar-move event)))
-     (t
-      (select-window window)
-      (cond
-       ((eq bar-part 'up)
-	(goto-char (window-start window))
-	(scroll-down 1))
-       ((eq bar-part 'above-handle)
-	(scroll-down))
-       ((eq bar-part 'below-handle)
-	(scroll-up))
-       ((eq bar-part 'down)
-	(goto-char (window-start window))
-	(scroll-up 1)))
-      (select-window old-window)))))
+(global-unset-key [horizontal-scroll-bar mouse-1])
+(global-unset-key [horizontal-scroll-bar drag-mouse-1])
 
 
 ;;;; Color support.
diff --git a/src/nsterm.h b/src/nsterm.h
index 0aea9cc..6cad337 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -676,11 +676,13 @@ char const * nstrace_fullscreen_type_name (int);
    /* offset to the bottom of knob of last mouse down */
    CGFloat last_mouse_offset;
    float min_portion;
-   int pixel_height;
+   int pixel_length;
    enum scroll_bar_part last_hit_part;
 
    BOOL condemned;
 
+   BOOL horizontal;
+
    /* optimize against excessive positioning calls generated by emacs */
    int em_position;
    int em_portion;
diff --git a/src/nsterm.m b/src/nsterm.m
index 34c5395..1d48c04 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1801,23 +1801,6 @@ x_set_window_size (struct frame *f,
 
   [window setFrame: wr display: YES];
 
-  /* This is a trick to compensate for Emacs' managing the scrollbar area
-     as a fixed number of standard character columns.  Instead of leaving
-     blank space for the extra, we chopped it off above.  Now for
-     left-hand scrollbars, we shift all rendering to the left by the
-     difference between the real width and Emacs' imagined one.  For
-     right-hand bars, don't worry about it since the extra is never used.
-     (Obviously doesn't work for vertically split windows tho..) */
-  {
-    NSPoint origin = FRAME_HAS_VERTICAL_SCROLL_BARS_ON_LEFT (f)
-      ? NSMakePoint (FRAME_SCROLL_BAR_COLS (f) * FRAME_COLUMN_WIDTH (f)
-                     - NS_SCROLL_BAR_WIDTH (f), 0)
-      : NSMakePoint (0, 0);
-
-    [view setFrame: NSMakeRect (0, 0, pixelwidth, pixelheight)];
-    [view setBoundsOrigin: origin];
-  }
-
   [view updateFrameSize: NO];
   unblock_input ();
 }
@@ -4351,7 +4334,7 @@ ns_set_vertical_scroll_bar (struct window *window,
   window_box (window, ANY_AREA, 0, &window_y, 0, &window_height);
   top = window_y;
   height = window_height;
-  width = WINDOW_CONFIG_SCROLL_BAR_COLS (window) * FRAME_COLUMN_WIDTH (f);
+  width = NS_SCROLL_BAR_WIDTH (f);
   left = WINDOW_SCROLL_BAR_AREA_X (window);
 
   r = NSMakeRect (left, top, width, height);
@@ -4442,34 +4425,20 @@ ns_set_horizontal_scroll_bar (struct window *window,
   NSTRACE ("ns_set_horizontal_scroll_bar");
 
   /* Get dimensions.  */
-  window_box (window, ANY_AREA, 0, &window_x, &window_width, 0);
+  window_box (window, ANY_AREA, &window_x, 0, &window_width, 0);
   left = window_x;
   width = window_width;
-  height = WINDOW_CONFIG_SCROLL_BAR_LINES (window) * FRAME_LINE_HEIGHT (f);
+  height = NS_SCROLL_BAR_HEIGHT (f);
   top = WINDOW_SCROLL_BAR_AREA_Y (window);
 
   r = NSMakeRect (left, top, width, height);
   /* the parent view is flipped, so we need to flip y value */
   v = [view frame];
-  /* ??????? PXW/scrollbars !!!!!!!!!!!!!!!!!!!! */
   r.origin.y = (v.size.height - r.size.height - r.origin.y);
 
   XSETWINDOW (win, window);
   block_input ();
 
-  if (WINDOW_TOTAL_COLS (window) < 5)
-    {
-      if (!NILP (window->horizontal_scroll_bar))
-        {
-          bar = XNS_SCROLL_BAR (window->horizontal_scroll_bar);
-          [bar removeFromSuperview];
-          wset_horizontal_scroll_bar (window, Qnil);
-        }
-      ns_clear_frame_area (f, left, top, width, height);
-      unblock_input ();
-      return;
-    }
-
   if (NILP (window->horizontal_scroll_bar))
     {
       if (width > 0 && height > 0)
@@ -4484,16 +4453,22 @@ ns_set_horizontal_scroll_bar (struct window *window,
       NSRect oldRect;
       bar = XNS_SCROLL_BAR (window->horizontal_scroll_bar);
       oldRect = [bar frame];
-      r.size.width = oldRect.size.width;
       if (FRAME_LIVE_P (f) && !NSEqualRects (oldRect, r))
         {
-          if (oldRect.origin.x != r.origin.x)
-              ns_clear_frame_area (f, left, top, width, height);
+          if (oldRect.origin.y != r.origin.y)
+            ns_clear_frame_area (f, left, top, width, height);
           [bar setFrame: r];
           update_p = YES;
         }
     }
 
+  /* If there are both horizontal and vertical scroll-bars they leave
+     a square that belongs to neither. We need to clear it otherwise
+     it fills with junk. */
+  if (!NILP (window->vertical_scroll_bar))
+    ns_clear_frame_area (f, WINDOW_SCROLL_BAR_AREA_X (window), top,
+                         NS_SCROLL_BAR_HEIGHT (f), height);
+
   if (update_p)
     [bar setPosition: position portion: portion whole: whole];
   unblock_input ();
@@ -4531,13 +4506,15 @@ ns_redeem_scroll_bar (struct window *window)
 {
   id bar;
   NSTRACE ("ns_redeem_scroll_bar");
-  if (!NILP (window->vertical_scroll_bar))
+  if (!NILP (window->vertical_scroll_bar)
+      && WINDOW_HAS_VERTICAL_SCROLL_BAR (window))
     {
       bar = XNS_SCROLL_BAR (window->vertical_scroll_bar);
       [bar reprieve];
     }
 
-  if (!NILP (window->horizontal_scroll_bar))
+  if (!NILP (window->horizontal_scroll_bar)
+      && WINDOW_HAS_HORIZONTAL_SCROLL_BAR (window))
     {
       bar = XNS_SCROLL_BAR (window->horizontal_scroll_bar);
       [bar reprieve];
@@ -8110,12 +8087,15 @@ not_in_argv (NSString *arg)
   return r;
 }
 
-
 - initFrame: (NSRect )r window: (Lisp_Object)nwin
 {
   NSTRACE ("[EmacsScroller initFrame: window:]");
 
-  r.size.width = [EmacsScroller scrollerWidth];
+  if (r.size.width > r.size.height)
+      horizontal = YES;
+  else
+      horizontal = NO;
+
   [super initWithFrame: r/*NSMakeRect (0, 0, 0, 0)*/];
   [self setContinuous: YES];
   [self setEnabled: YES];
@@ -8131,9 +8111,12 @@ not_in_argv (NSString *arg)
 
   window = XWINDOW (nwin);
   condemned = NO;
-  pixel_height = NSHeight (r);
-  if (pixel_height == 0) pixel_height = 1;
-  min_portion = 20 / pixel_height;
+  if (horizontal)
+    pixel_length = NSWidth (r);
+  else
+    pixel_length = NSHeight (r);
+  if (pixel_length == 0) pixel_length = 1;
+  min_portion = 20 / pixel_length;
 
   frame = XFRAME (window->frame);
   if (FRAME_LIVE_P (frame))
@@ -8162,9 +8145,12 @@ not_in_argv (NSString *arg)
   NSTRACE ("[EmacsScroller setFrame:]");
 
 /*  block_input (); */
-  pixel_height = NSHeight (newRect);
-  if (pixel_height == 0) pixel_height = 1;
-  min_portion = 20 / pixel_height;
+  if (horizontal)
+    pixel_length = NSWidth (newRect);
+  else
+    pixel_length = NSHeight (newRect);
+  if (pixel_length == 0) pixel_length = 1;
+  min_portion = 20 / pixel_length;
   [super setFrame: newRect];
 /*  unblock_input (); */
 }
@@ -8174,7 +8160,12 @@ not_in_argv (NSString *arg)
 {
   NSTRACE ("[EmacsScroller dealloc]");
   if (window)
-    wset_vertical_scroll_bar (window, Qnil);
+    {
+      if (horizontal)
+        wset_horizontal_scroll_bar (window, Qnil);
+      else
+        wset_vertical_scroll_bar (window, Qnil);
+    }
   window = 0;
   [super dealloc];
 }
@@ -8209,7 +8200,12 @@ not_in_argv (NSString *arg)
       if (view != nil)
         view->scrollbarsNeedingUpdate++;
       if (window)
-        wset_vertical_scroll_bar (window, Qnil);
+        {
+          if (horizontal)
+            wset_horizontal_scroll_bar (window, Qnil);
+          else
+            wset_vertical_scroll_bar (window, Qnil);
+        }
       window = 0;
       [self removeFromSuperview];
       [self release];
@@ -8259,7 +8255,7 @@ not_in_argv (NSString *arg)
     {
       float pos;
       CGFloat por;
-      portion = max ((float)whole*min_portion/pixel_height, portion);
+      portion = max ((float)whole*min_portion/pixel_length, portion);
       pos = (float)position / (whole - portion);
       por = (CGFloat)portion/whole;
 #ifdef NS_IMPL_COCOA
@@ -8289,10 +8285,20 @@ not_in_argv (NSString *arg)
   XSETWINDOW (win, window);
   emacs_event->frame_or_window = win;
   emacs_event->timestamp = EV_TIMESTAMP (e);
-  emacs_event->kind = SCROLL_BAR_CLICK_EVENT;
   emacs_event->arg = Qnil;
-  XSETINT (emacs_event->x, loc * pixel_height);
-  XSETINT (emacs_event->y, pixel_height-20);
+
+  if (horizontal)
+    {
+      emacs_event->kind = HORIZONTAL_SCROLL_BAR_CLICK_EVENT;
+      XSETINT (emacs_event->x, em_whole * loc / pixel_length);
+      XSETINT (emacs_event->y, em_whole);
+    }
+  else
+    {
+      emacs_event->kind = SCROLL_BAR_CLICK_EVENT;
+      XSETINT (emacs_event->x, loc);
+      XSETINT (emacs_event->y, pixel_length-20);
+    }
 
   if (q_event_ptr)
     {
@@ -8355,15 +8361,15 @@ not_in_argv (NSString *arg)
   switch (part)
     {
     case NSScrollerDecrementPage:
-        last_hit_part = scroll_bar_above_handle; inc = -1.0; break;
+      last_hit_part = horizontal ? scroll_bar_before_handle : scroll_bar_above_handle; break;
     case NSScrollerIncrementPage:
-        last_hit_part = scroll_bar_below_handle; inc = 1.0; break;
+      last_hit_part = horizontal ? scroll_bar_after_handle : scroll_bar_below_handle; break;
     case NSScrollerDecrementLine:
-      last_hit_part = scroll_bar_up_arrow; inc = -0.1; break;
+      last_hit_part = horizontal ? scroll_bar_left_arrow : scroll_bar_up_arrow; break;
     case NSScrollerIncrementLine:
-      last_hit_part = scroll_bar_down_arrow; inc = 0.1; break;
+      last_hit_part = horizontal ? scroll_bar_right_arrow : scroll_bar_down_arrow; break;
     case NSScrollerKnob:
-      last_hit_part = scroll_bar_handle; break;
+      last_hit_part = horizontal ? scroll_bar_horizontal_handle : scroll_bar_handle; break;
     case NSScrollerKnobSlot:  /* GNUstep-only */
       last_hit_part = scroll_bar_move_ratio; break;
     default:  /* NSScrollerNoPart? */
@@ -8372,36 +8378,34 @@ not_in_argv (NSString *arg)
       return;
     }
 
-  if (inc != 0.0)
-    {
-      pos = 0;      /* ignored */
-
-      /* set a timer to repeat, as we can't let superclass do this modally */
-      scroll_repeat_entry
-	= [[NSTimer scheduledTimerWithTimeInterval: SCROLL_BAR_FIRST_DELAY
-                                            target: self
-                                          selector: @selector (repeatScroll:)
-                                          userInfo: 0
-                                           repeats: YES]
-	    retain];
-    }
-  else
+  if (part == NSScrollerKnob || part == NSScrollerKnobSlot)
     {
       /* handle, or on GNUstep possibly slot */
       NSEvent *fake_event;
+      int length;
 
       /* compute float loc in slot and mouse offset on knob */
       sr = [self convertRect: [self rectForPart: NSScrollerKnobSlot]
                       toView: nil];
-      loc = NSHeight (sr) - ([e locationInWindow].y - NSMinY (sr));
+      if (horizontal)
+        {
+          length = NSWidth (sr);
+          loc = ([e locationInWindow].x - NSMinX (sr));
+        }
+      else
+        {
+          length = NSHeight (sr);
+          loc = length - ([e locationInWindow].y - NSMinY (sr));
+        }
+
       if (loc <= 0.0)
         {
           loc = 0.0;
           edge = -1;
         }
-      else if (loc >= NSHeight (sr))
+      else if (loc >= length)
         {
-          loc = NSHeight (sr);
+          loc = length;
           edge = 1;
         }
 
@@ -8411,17 +8415,16 @@ not_in_argv (NSString *arg)
         {
           kr = [self convertRect: [self rectForPart: NSScrollerKnob]
                           toView: nil];
-          kloc = NSHeight (kr) - ([e locationInWindow].y - NSMinY (kr));
+          if (horizontal)
+            kloc = ([e locationInWindow].x - NSMinX (kr));
+          else
+            kloc = NSHeight (kr) - ([e locationInWindow].y - NSMinY (kr));
         }
       last_mouse_offset = kloc;
 
-      /* if knob, tell emacs a location offset by knob pos
-         (to indicate top of handle) */
-      if (part == NSScrollerKnob)
-          pos = (loc - last_mouse_offset) / NSHeight (sr);
-      else
-        /* else this is a slot click on GNUstep: go straight there */
-        pos = loc / NSHeight (sr);
+      if (part != NSScrollerKnob)
+        /* this is a slot click on GNUstep: go straight there */
+        pos = loc;
 
       /* send a fake mouse-up to super to preempt modal -trackKnob: mode */
       fake_event = [NSEvent mouseEventWithType: NSLeftMouseUp
@@ -8435,6 +8438,19 @@ not_in_argv (NSString *arg)
                                       pressure: [e pressure]];
       [super mouseUp: fake_event];
     }
+  else
+    {
+      pos = 0;      /* ignored */
+
+      /* set a timer to repeat, as we can't let superclass do this modally */
+      scroll_repeat_entry
+	= [[NSTimer scheduledTimerWithTimeInterval: SCROLL_BAR_FIRST_DELAY
+                                            target: self
+                                          selector: @selector (repeatScroll:)
+                                          userInfo: 0
+                                           repeats: YES]
+	    retain];
+    }
 
   if (part != NSScrollerKnob)
     [self sendScrollEventAtLoc: pos fromEvent: e];
@@ -8446,23 +8462,34 @@ not_in_argv (NSString *arg)
 {
     NSRect sr;
     double loc, pos;
+    int length;
 
     NSTRACE ("[EmacsScroller mouseDragged:]");
 
       sr = [self convertRect: [self rectForPart: NSScrollerKnobSlot]
                       toView: nil];
-      loc = NSHeight (sr) - ([e locationInWindow].y - NSMinY (sr));
+
+      if (horizontal)
+        {
+          length = NSWidth (sr);
+          loc = ([e locationInWindow].x - NSMinX (sr));
+        }
+      else
+        {
+          length = NSHeight (sr);
+          loc = length - ([e locationInWindow].y - NSMinY (sr));
+        }
 
       if (loc <= 0.0)
         {
           loc = 0.0;
         }
-      else if (loc >= NSHeight (sr) + last_mouse_offset)
+      else if (loc >= length + last_mouse_offset)
         {
-          loc = NSHeight (sr) + last_mouse_offset;
+          loc = length + last_mouse_offset;
         }
 
-      pos = (loc - last_mouse_offset) / NSHeight (sr);
+      pos = (loc - last_mouse_offset);
       [self sendScrollEventAtLoc: pos fromEvent: e];
 }
 
diff --git a/src/window.h b/src/window.h
index 0cfff88..a1c4aaa 100644
--- a/src/window.h
+++ b/src/window.h
@@ -786,7 +786,7 @@ wset_next_buffers (struct window *w, Lisp_Object val)
    || WINDOW_HAS_VERTICAL_SCROLL_BAR_ON_RIGHT (W))
 
 #if (defined (HAVE_WINDOW_SYSTEM)					\
-     && ((defined (USE_TOOLKIT_SCROLL_BARS) && !defined (HAVE_NS))	\
+     && ((defined (USE_TOOLKIT_SCROLL_BARS))	\
 	 || defined (HAVE_NTGUI)))
 # define USE_HORIZONTAL_SCROLL_BARS true
 #else
-- 

OK, I really hope I've cracked it this time! Just after I sent the
last patch I noticed an issue with left-hand scroll-bars where they
were overlapping the text area, but only if you enabled them
*without* any other scroll-bars being enabled first.

-- 
Alan Third

bug#22660: [PATCH v2] Implement horizontal scroll-bars in NS port

[martin rudalics]

 > OK, I really hope I've cracked it this time! Just after I sent the
 > last patch I noticed an issue with left-hand scroll-bars where they
 > were overlapping the text area, but only if you enabled them
 > *without* any other scroll-bars being enabled first.

Thanks.  It definitely fixes the issue from my second GNUStep cap.  You
should install your patch on master now so more people can test it.

martin

bug#22660: [PATCH v2] Implement horizontal scroll-bars in NS port

[Alan Third]

On Sun, May 01, 2016 at 10:24:14AM +0200, martin rudalics wrote:
> Thanks.  It definitely fixes the issue from my second GNUStep cap.  You
> should install your patch on master now so more people can test it.

I don't have write access to the repository, so you, or someone else,
will have to do that for me.
-- 
Alan Third

bug#22660: [PATCH v2] Implement horizontal scroll-bars in NS port

[martin rudalics]

 > I don't have write access to the repository, so you, or someone else,
 > will have to do that for me.

Pushed as commit e683a2570be6f2e4f8defa4e7ee35a7d87f18918 to master.
Test reports welcome.

Thanks again, martin