Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 355683

Summary:

[10.7] API to tell whether scrollbars are overlay

Product:

[Eclipse Project] Platform

Reporter:

Markus Keller <markus.kell.r>

Component:

SWT

Assignee:

Lakshmi P Shanmugam <lshanmug>

Status:

RESOLVED FIXED

QA Contact:

Severity:

major

Priority:

CC:

arunkumar.thondapu, bolbat, daniel_megert, eclipse.felipe, lshanmug, markus.kell.r, njbartlett, remy.suen, Silenio_Quarti

Version:

3.7.1

Keywords:

api

Target Milestone:

3.8 M6

Hardware:

OS:

Mac OS X

Whiteboard:

Bug Depends on:

353503

Bug Blocks:

363482

Attachments:

Description	Flags
Screenshot	none
patch	none
patch-2	none

Description Markus Keller

2011-08-24 06:44:18 EDT

Created attachment 202074 [details]
Screenshot

+++ This bug was initially created as a clone of Bug #353503 +++

R3_7_maintenance

With the last change from bug 353503, the textWidget.computeTrim(0, 0, 0, 0) in SourceViewer.RulerLayout#layout() now returns a fake trim with width and height of 15 (which is too big, see bug 353503 comment 19).

The fake trim leads to a wrong height for the vertical ruler, since we reserve the space for the scroll bar. We still need a way to find out whether the scrollbar is always there or whether it's an overlay.

Comment 1 Silenio Quarti

2011-08-24 11:44:28 EDT

Yes, I agree we need to add API to determine whether the scrollbars are overlay or not. Client code will have to change to use the new API.

The released code makes 3.7.1 functional. We will work on the new API only for 3.8.

Comment 2 Silenio Quarti

2011-08-24 11:46:49 EDT

Overlay scrollbars are available on GTK as well (latest Ubuntu). The current implementation of SWT is to fake the trim their as well.

Comment 3 Markus Keller

2011-11-10 11:32:36 EST

Ping. Users are unhappy with the gray box in the line number ruler (bug 363482).

Comment 4 Markus Keller

2012-01-17 06:33:06 EST

This API should be added in M6.

We will maybe want to support NSScrollView's setScrollerStyle in the future, so the getter API could be "public boolean getShowOverlayScrollBars()" in Scrollable.

Comment 5 Silenio Quarti

2012-02-07 10:32:52 EST

Lakshmi, please work on this for M6. I would say the API should be:

public boolean Scrollable.getOverlayBars();

Comment 6 Lakshmi P Shanmugam

2012-02-09 03:45:06 EST

Created attachment 210779 [details]
patch

Patch adds the API for cocoa and stubs for windows & gtk.

A comment about the name: I wonder if getOverlayBars() could be misleading to return Scrollbar as it sounds similar to Scrollable.getVerticalBar() and getHorizontalBar() which returns ScrollBar. May be we could use getShowOverlayBars or getUseOverlayBars?

Comment 7 Lakshmi P Shanmugam

2012-02-09 03:56:35 EST

I checked eclipse on Ubuntu 11.04 and it doesn't show the overlay scrollbars though the required library has been installed. Looks like eclipse is in the blacklist for overlay bars. So, we have to check eclipse on the latest Ubuntu to see if the overlay scrollbars and the graybox problem appear there.

Comment 8 Silenio Quarti

2012-02-13 11:32:20 EST

(In reply to comment #6)
> Created attachment 210779 [details]
> patch
> 
> Patch adds the API for cocoa and stubs for windows & gtk.
> 
> A comment about the name: I wonder if getOverlayBars() could be misleading to
> return Scrollbar as it sounds similar to Scrollable.getVerticalBar() and
> getHorizontalBar() which returns ScrollBar. May be we could use
> getShowOverlayBars or getUseOverlayBars?

Yes, I also think it is misleading, but I do not like the other options either. I believe we should add a more "generic" API in Scrollable so that we have room to extend it in the future. How about:

public int getScrollbarsMode()

And for now, we add this constant in SWT:

public static final int OVERLAY_BAR = 1 << 1;   

I could see other constants added later on to solve bug#363441 (ie. DARK_BAR, LIGHT_BAR).

Note that the patch is not calling checkWidget().

Comment 9 Silenio Quarti

2012-02-13 11:38:25 EST

(In reply to comment #7)
> I checked eclipse on Ubuntu 11.04 and it doesn't show the overlay scrollbars
> though the required library has been installed. Looks like eclipse is in the
> blacklist for overlay bars. So, we have to check eclipse on the latest Ubuntu
> to see if the overlay scrollbars and the graybox problem appear there.

The eclipse executable is blacklisted, but if you just rename it (branding), the overlay scrollbars show up. They also show up for eclipse if the launcher cannot embedded the VM and has to launch java as a separate process.

I am not sure how we could determine if the scrollbars are overlay or not. We could check the env var LIBOVERLAY_SCROLLBAR, but overlay-scrollbar package may not even be installed.

Comment 10 Markus Keller

2012-02-14 16:26:18 EST

(In reply to comment #8)
getScrollbarsMode() sounds good, but SWT.OVERLAY_BAR, DARK_BAR, LIGHT_BAR would deviate from the common naming pattern <CATEGORY>_<INSTANCE>.

BAR_OVERLAY or SCROLLBAR_OVERLAY would be better names.

Comment 11 Lakshmi P Shanmugam

2012-02-21 10:20:42 EST

Created attachment 211335 [details]
patch-2

Sorry, I had made the changes sometime back, but forgot to attach the patch here. Modified the API to public int getScrollbarsMode().

Comment 12 Lakshmi P Shanmugam

2012-02-21 10:53:30 EST

(In reply to comment #9)
> (In reply to comment #7)
> > I checked eclipse on Ubuntu 11.04 and it doesn't show the overlay scrollbars
> > though the required library has been installed. Looks like eclipse is in the
> > blacklist for overlay bars. So, we have to check eclipse on the latest Ubuntu
> > to see if the overlay scrollbars and the graybox problem appear there.
> 
> The eclipse executable is blacklisted, but if you just rename it (branding),
> the overlay scrollbars show up. They also show up for eclipse if the launcher
> cannot embedded the VM and has to launch java as a separate process.
> 
> I am not sure how we could determine if the scrollbars are overlay or not. We
> could check the env var LIBOVERLAY_SCROLLBAR, but overlay-scrollbar package may
> not even be installed.

Thanks Silenio, renaming and launching eclipse shows the overlay bars. And the graybox problem appears in the editor.
I checked but I couldn't find any API support to detect overlay bars. Also, looks like the environment variable LIBOVERLAY_SCROLLBAR is not set when the overlay bar is enabled by default.

Comment 13 Arun Thondapu

2012-02-21 11:58:26 EST

(In reply to comment #12)
> I checked but I couldn't find any API support to detect overlay bars. Also,
> looks like the environment variable LIBOVERLAY_SCROLLBAR is not set when the
> overlay bar is enabled by default.

Thats right, there is no environment variable to indicate whether overlay scrollbars are being used. I think the variable was only used earlier to explicitly enable the overlays when they weren't used by default.

Comment 14 Silenio Quarti

2012-02-23 11:03:37 EST

There are some bad imports in Scrollable.java win32, otherwise patch is good to release.

Comment 15 Lakshmi P Shanmugam

2012-02-24 07:35:52 EST

Thanks Silenio! Fixed in master --> http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=67d2c3a2030f152815bf563d9ce1b6fb14017009

I'll open a separate bug to track this on gtk.

Comment 16 Lakshmi P Shanmugam

2012-02-24 08:16:00 EST

> I'll open a separate bug to track this on gtk.
Opened Bug 372480 to track this.