Bug 502111 - [CTabFolder] Toolbar items overflow when explicit tab height is used
Summary: [CTabFolder] Toolbar items overflow when explicit tab height is used
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 4.6   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 4.7 M3   Edit
Assignee: Marc-André Laperle CLA
QA Contact: Niraj Modi CLA
URL:
Whiteboard:
Keywords:
: 501413 (view as bug list)
Depends on:
Blocks: 501413
  Show dependency tree
 
Reported: 2016-09-25 17:41 EDT by Marc-André Laperle CLA
Modified: 2018-01-04 09:37 EST (History)
5 users (show)

See Also:

Attachments
Before commit (4.64 KB, image/png)
2016-09-25 17:42 EDT, Marc-André Laperle CLA 		no flags Details
After commit (4.17 KB, image/png)
2016-09-25 17:42 EDT, Marc-André Laperle CLA 		no flags Details
Snippet showing bug (1.39 KB, text/plain)
2016-09-25 17:43 EDT, Marc-André Laperle CLA 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Marc-André Laperle CLA 2016-09-25 17:41:37 EDT 
From bug 501413.

Using Eclipse 4.6.0 and SWT from master.

When CTabFolder.setTabHeight is used, the height is not always respected in the case where the toolbar buttons do not fit vertically.
This behavior was changed by commit:

commit 585085075702b3c9665ad402cbe1a9f2889382a0
Author: Eugen Neufeld <eneufeld@eclipsesource.com>
Date:   Thu Jan 21 19:32:11 2016 +0530

    Bug 483688 - [CTabFolder] Icon of toolbar menu cropped
    
    Change-Id: Idbf65bdd6f5bb8ae0fad7f1a9af5dc31a103a567
    Signed-off-by: Eugen Neufeld <eneufeld@eclipsesource.com>



I think that if setTabHeight is not used, then it is OK to make the items overflow. But when setTabHeight was explicitly called, it should respect the height and not overflow.

I will attach screen shots of before and after the commit to better illustrate the differences.
Comment 1 Marc-André Laperle CLA 2016-09-25 17:42:10 EDT 
Created attachment 264398 [details]
Before commit
Comment 2 Marc-André Laperle CLA 2016-09-25 17:42:30 EDT 
Created attachment 264399 [details]
After commit
Comment 3 Marc-André Laperle CLA 2016-09-25 17:43:53 EDT 
Created attachment 264400 [details]
Snippet showing bug
Comment 4 Eclipse Genie CLA 2016-09-25 17:45:08 EDT 
New Gerrit change created: https://git.eclipse.org/r/81875
Comment 5 Niraj Modi CLA 2016-09-28 05:16:45 EDT 
(In reply to Marc-Andre Laperle from comment #0)
> I think that if setTabHeight is not used, then it is OK to make the items
> overflow. But when setTabHeight was explicitly called, it should respect the
> height and not overflow.

Yes, this makes sense.
Tested your patch on Win7, it looks good. Will verify it on Linux as well before releasing it to master.
Comment 7 Niraj Modi CLA 2016-09-28 09:15:43 EDT 
(In reply to Eclipse Genie from comment #6)
> Gerrit change https://git.eclipse.org/r/81875 was merged to [master].
> Commit:
> http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/
> ?id=8a456e1b2a83efab27513cfa8ed8f72ab32fa6e0

Thanks Marc for your work on this issue, resolving now.
Comment 8 Marc-André Laperle CLA 2016-09-28 10:33:06 EDT 
(In reply to Niraj Modi from comment #7)
> (In reply to Eclipse Genie from comment #6)
> > Gerrit change https://git.eclipse.org/r/81875 was merged to [master].
> > Commit:
> > http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/
> > ?id=8a456e1b2a83efab27513cfa8ed8f72ab32fa6e0
> 
> Thanks Marc for your work on this issue, resolving now.

Thank you for the quick review!
Comment 9 Marc-André Laperle CLA 2016-10-04 15:43:29 EDT 
*** Bug 501413 has been marked as a duplicate of this bug. ***
Comment 10 Niraj Modi CLA 2016-10-25 07:07:56 EDT 
Verified fix in Build id: I20161024-2000 on Win7
Comment 11 Andrey Loskutov CLA 2018-01-04 09:37:54 EST 
(In reply to Eclipse Genie from comment #6)
> Gerrit change https://git.eclipse.org/r/81875 was merged to [master].
> Commit:
> http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/
> ?id=8a456e1b2a83efab27513cfa8ed8f72ab32fa6e0

This patch caused a really crazy regression, see bug 529405 comment 10.