Can you elaborate on what exactly you mean by "shrink"? Did you mean close or
resize? Or something else? Can you take a screenshot?

I'm presuming you mean the progress -dialog-, view's sizes are completely
determined by the stack that they're in.

AFAIK, the dialog currently doesn't 'auto-size' at all (but gets a scroll bar
when needed). UI's should avoid changing element's sizes 'on the fly' generally
as it induces visual flicker...

Created attachment 108767 [details]
screenshot

Progress item sizes and the view port of the view is absurdly large

The package manager has the same problem.  Horizontal scrollbar is visible
while everything fits in the screen 
Could this be indeed in or related to SWT?

Created attachment 111787 [details]
Screenshot of broken progress view

Currently on Linux progress view is very hard to use. In my eyes it is just
broken. 

There are always some jobs which for some strange reasons are rendered with the
too huge hight, so that it is impossible to see other jobs without resizing the
view or scrolling it. But scrolling doesn't help too because if there are many
different jobs working, their order changes and you still can't see what's
going on...

So the main use case of the progress view (show what's going on) is broken on
Linux.

Please consider to raise severity of this issue.

Are you using ubuntu?

I'm using RHEL 5.1, uname -a:

Linux socbl422 2.6.18-53.1.4.el5 #1 SMP Wed Nov 14 10:37:27 EST 2007 x86_64
x86_64 x86_64 GNU/Linux

In my case it is Fedora 9 and Fedora 10 Alpha...

Created attachment 140860 [details]
Vertical scroll bar is not always available in all
platforms(windows/linux/solaris)

Only those job entries without progress bar endup in the layout issue. (shown
in first two screenshots)
That is the job entry without progress bar takes more real estate that the
entry with progress bar. This issue happens in linux and solaris machines.

Adding to this there is no vertcal sroll bar added to the view, which makes it
difficult to view all the jobs currently running. (shown in third screenshot)
This scrolling issue is there in windows/linux/solaris machines.

As per http://wiki.eclipse.org/Platform_UI/Bug_Triage_Change_2009

This seems to be resolved in eclipse 3.6M5...

(In reply to comment #12)
> This seems to be resolved in eclipse 3.6M5...

There were some changes in the layout of ProgressView few months back. That
might have fixed this one.

Hello Prakash,

I was actually wrong. The way it exposes it self is as follows
1) A Job is started
2) Before Job declared the beginTask, DetailedProgressViewer created the item
and went through the layout()
3) Job either called beginTask or kept running without that or slept on
something

But actually the guilty part was the org.eclipse.swt.Link which during
computeSize (int wHint, int hHint, boolean changed), sets the width to 1px if
wHint == 0.

Changing layout.setWidth (1) to layout.setWidth (Integer.MAX_VALUE) fixes the
issue...

Regards,
Hasan Ceylan

Created attachment 159400 [details]
A Sample project to recreate the bug

Why no action is being taken when there's the fix for the bug?

(In reply to comment #16)
> Why no action is being taken when there's the fix for the bug?

   Because there are other bugs with high priority that needs to be attended.
Feel free to attach a patch, I'll try to push this into 3.7

Created attachment 171809 [details]
Patch to resolve the issue

This is the patch that resolves the issue.

Sorry for taking some time to submit this.
Hope this makes into 3.6 and the two year annoyance (Yes, I am obsessive :) )
is gone...

3.6 is at the door. We can't push any changes. Will try for 3.6.1

(In reply to comment #19)
> 3.6 is at the door. We can't push any changes. Will try for 3.6.1

Thanks Prakash...

This isn't a serious bug that has to be pushed to the maintenance stream. I'd
like to target to HEAD rather.

Assigning to SWT for comments. Is the width calculation of the Link widget
correct?

Can we please get a review of the patch and a fix in 3.7?

(In reply to comment #23)
> Can we please get a review of the patch and a fix in 3.7?

It is wrong, the application is calling computeSize with 0,SWT.DEFAULT right ?
The current code causes the link to wrap with width zero. With the fix the link
will no wrap (not respecting the wHint what was set).

(In reply to comment #24)
> (In reply to comment #23)
> > Can we please get a review of the patch and a fix in 3.7?
> 
> It is wrong, the application is calling computeSize with 0,SWT.DEFAULT right ?
> The current code causes the link to wrap with width zero. With the fix the link
> will no wrap (not respecting the wHint what was set).

Sorry, I'm missing the point - is the patch ok or not?

(In reply to comment #25)
> Sorry, I'm missing the point - is the patch ok or not?

the patch is wrong.

(In reply to comment #24)
> (In reply to comment #23)
> > Can we please get a review of the patch and a fix in 3.7?
> 
> It is wrong, the application is calling computeSize with 0,SWT.DEFAULT right ?
> The current code causes the link to wrap with width zero. With the fix the link
> will no wrap (not respecting the wHint what was set).

I finally got some time to play with it. Felipe, I really can not understand
your comment above. The only thing the patch does is to call now

layout.setWidth (Integer.MAX_VALUE);

instead of calling 

layout.setWidth (1);

The later one (current HEAD in Git, since ) caused the return value to contain
HUGE height values for Link objects with the "0" width.

This happens in case application provided either value smaller -1 or 0 as  a
width hint. The last one is exactly the case in the concrete implementation of
the FormLayout->layout()->FormData->computeSize() which is used in the progress
view.

So for me the bug would be fixed with the proposed patch, and it will NOT break
any wrapping code.

BTW, looking to the origin of the code in question, the layout.setWidth (1);
line was added to fix bug 85236 (do not call layout with the "0" width, commit
id 5b3ec303ac0d2af543a2733646c7864facee0355), and this bug is a side effect of
this fix.


Looking on the code in FormLayout used for the progress view, another solution
would be to fix the caller code in the FormLayout.layout:319 to

int currentWidth = Math.max (SWT.DEFAULT, x2 - x1 - trim);

instead of 

int currentWidth = Math.max (0, x2 - x1 - trim);

which has exact the same effect on the progress bar.

So either of patches above solves the progress view issue for me. As I'm not
the code owner, I can hardly decide which one (if any) is better.

So what I would ask here: please, can you review both patches again and apply
the fix for 3.7.2 and 3.8.

Thanks!

BTW my configuration is:

Ubuntu 11.04 64 bit 
dpkg -s libgtk2.0-0|grep '^Version'
Version: 2.24.4-0ubuntu2

Created attachment 206502 [details]
Combined patch for both FormLayout and Link

The Link change is from Hasan Ceylan's patch.

(In reply to comment #28)
> Created attachment 206502 [details]
> Combined patch for both FormLayout and Link

Since committing the patch I'm running the patched Eclipse 3.7.1 8 hours 5 days
in a week with no problems so far (RHEL 5.3 64 bit). The issue is solved for me
and I didn't noticed ANY regression. Sure I didn't run all UI tests, but
looking at the patch there shouldn't be any problems.

Could please somebody from SWT team re-evaluate the patch and apply it to the
3.8 stream?
Thanks!
Andrey

I reviewed the patch, I believe both solutions are wrong. I will explain:

First read the documentation for computeSize().
Note that wHint and hHint should be >= 0 or SWT.DEFAULT (-1)
and that SWT.DEFAULT (-1)  is special constant, means return the preferred
size.

Also from the doc: 
" The width hint and height hint arguments
 * allow the caller to ask a control questions such as "Given a particular
 * width, how high does the control need to be to show all of the contents?"

For the Link widget, if hHint is zero (or one) it causes the link to wrap every
characters to new line (thus the large value for height that you see). if hHint
is SWT.DEFAULT (-1) then computeSize() returns the size of the control without
any wrapping. This is correct behaviour.

The current code in FormLayout also seems correct to me:
int currentWidth = Math.max (0, x2 - x1 - trim);
It is just max with zero to avoid negative number.

changing that to SWT.DEFAULT (-1) is wrong, as I said, SWT.DEFAULT is special
case complete different than 0 or positive values.

Moving back to UI.

(In reply to comment #30)
> I reviewed the patch, I believe both solutions are wrong. I will explain:
>[...]

> The current code in FormLayout also seems correct to me:
> int currentWidth = Math.max (0, x2 - x1 - trim);
> It is just max with zero to avoid negative number.
> 
> changing that to SWT.DEFAULT (-1) is wrong, as I said, SWT.DEFAULT is special
> case complete different than 0 or positive values.

At least here I can't fully agree. I think calling computeSize with -1/-1 from
FormLayout.layout could be a possible fix (at least on GTK).

Just debugging Progress view on Linux (put conditional breakpoint currentWidth
== 0 at line 320 in FormLayout) - one can see that Link.computeSize() returns
(0,300) if called with wHint == 0, hHint == -1, and would return proper width
and hight (e.g. 150, 15) if called with wHint == -1, hHint == -1 as proposed in
the patch. And one also can see that there are LOT of calls where wHint == 0...

As you pointed, the "0" was probably chosen as the default "safe" value, but it
leads to the logically wrong assumption in the Link code that FormLayout want
wrap the Link as if there were no space anymore. What one can see from the
debugger, is that there are four children of FormLayout in the ProgressInfoItem
(Label, Label, ToolBar, Link), and that the Link is the last (right) one. If it
has no attachment on the right, its width should be calculated to the default
value, as defined in the FormLayout class doc:

 * If a side is not given an attachment, it is defined as not being attached
 * to anything, causing the child to remain at its preferred size

But by using "0" we just trying to "wrap" the Link over "0 pixel width" instead
of calculating it's default size. We see that at least on GTK this leads to
really huge progress entries (width = 0, hight = 300), and that on other
platforms everything seems to be ok. 

So for me it sounds like a GTK specific bug which need to be addressed either
in FormLayout as proposed by the patch (might be with the restriction to the
GTK platform only) or at ProgressInfoItem which creates and manages those
"Link" instances.

> Moving back to UI

Might be it would help if two (or more :) SWT + UI developers talk *together*
about this annoying bug?

Could one please consider to increase the severity of this issue? The whole
Eclipse story with Progress view is broken on Linux just because of this
"minor" problem of Link size calculations.

does the problem only happen on Linux ?
Link#computeSize() should work the same way on all platforms.

Can you post a simple testcase (SWT only) showing the problem you described
with FormLayout ? Thanks.

I had provided a test case in the attachment "A Sample project to recreate the
bug".

Sorry to be rude, but test case has been asked and it has been provided.
Please can we fix that after 3.5 years?

Regards,
Hasan Ceylan

(In reply to comment #34)
> Sorry to be rude, but test case has been asked and it has been provided.
> Please can we fix that after 3.5 years?

No one is looking at the progress view at the moment.

Is there something we can set in the DetailedProgressViewer/ProgressInfoItem to
avoid hitting this in FormLayout?  Maybe we should be using SWT.DEFAULT in one
of our calls if the Job doesn't specify progress?

PW

Come on guys,

it is almost 4 years. 

Patchs are provided, what is the big deal?

(In reply to comment #36)
> Patchs are provided, what is the big deal?

They were not appropriate, as stated in the comments?

OK, let's get a fresh set of eyes on them and see if we can make progress.

PW

Dear Paul,

1) Both patches actually fix the issue, so I do not agree that they were not
appropriate. If it is wrong, the current code is already wrong, so please
correct it, then we can provide the correct patch if the issue still persists.
The patches do not break things here, they fix what is already broken.

2) If when the width is zero and if that means every character should be laid
on a new line, how come it has a huge height with null or "" link value? 

3) I inspected the win32 code and saw that the code base is completely
different so breaking the other platforms cannot be an issue.

4) "Link#computeSize() should work the same way on all platforms" I agree,
well, guess what it does not if you count GTK as a platform.

5) Since I submitted the fix till I get tired of it, I was patching my own
eclipse with the fix and did not notice anything wrong. Also Andrey stated that
he uses patched Eclipse extensively and he is fine too. 

Comments "like provide a test case", "Is it on Linux" clearly suggests that the
responsibles for the bug report do not even take time to read the bug. It was
stated repeatedly that this is a GTK specific bug and the test case was
submitted 15 months before it was asked for.

Both Andrey and I took time off from our work and made every effort to correct
this *little* annoying bug bug we get almost no attention in return.

I highly doubt that none of Eclipse Engineers who ever replied to this bug uses
Linux as their choice of platform. 

So please someone take a little time and fix the issue.

Regards,
Hasan Ceylan

@Paul:
Please consider to look at it for 3.8 release. 

As Hasan said, it's GTK only and you have to debug it once to see that the
patches just work. Linux progress view is really broken due this bug and since
*so many* releases/years, we MUST fix this crap finally.

(In reply to comment #38)
> Dear Paul,
> 
> 1) Both patches actually fix the issue, so I do not agree that they were not
> appropriate. If it is wrong, the current code is already wrong, so please
> correct it, then we can provide the correct patch if the issue still persists.
> The patches do not break things here, they fix what is already broken.

This was a request to a new SWT committer to re-examine the bug, so we could
move this bug forward and so we could get the correct fix ... you don't apply
another wrong fix to a wrong initial state, you fix it if possible.

When I get consensus, then we can move forward.

PW

(In reply to comment #40)
> (In reply to comment #38)
> > Dear Paul,
> > 
> > 1) Both patches actually fix the issue, so I do not agree that they were not
> > appropriate. If it is wrong, the current code is already wrong, so please
> > correct it, then we can provide the correct patch if the issue still persists.
> > The patches do not break things here, they fix what is already broken.
> 
> This was a request to a new SWT committer to re-examine the bug, so we could
> move this bug forward and so we could get the correct fix ... you don't apply
> another wrong fix to a wrong initial state, you fix it if possible.
> 

Yes, I would agree if it was fixed in a few months. But, as Andrey said, after
"*so many* releases/years", you simply start to seek what just works...

> When I get consensus, then we can move forward.
> 
> PW

(In reply to comment #38)
> I highly doubt that none of Eclipse Engineers who ever replied to this bug uses
> Linux as their choice of platform. 
> 

And this is where I draw the line.  Disagree with the fix or lack of it,
disagree with the prioritization of the bug.  That's acceptable.

Attacking people is never acceptable.  See the slides for
http://www.eclipsecon.org/2012/sessions/assholes-are-killing-your-project

PW

(In reply to comment #42)
> (In reply to comment #38)
> > I highly doubt that none of Eclipse Engineers who ever replied to this bug uses
> > Linux as their choice of platform. 
> > 
> 
> And this is where I draw the line.  Disagree with the fix or lack of it,
> disagree with the prioritization of the bug.  That's acceptable.
> 
> Attacking people is never acceptable.  See the slides for
> http://www.eclipsecon.org/2012/sessions/assholes-are-killing-your-project
> 
> PW

Paul,

My comment was an observation, and I think I stand corrected, if any of the
engineers involved in this bug report were using eclipse on Linux, they would
be as disturbed as we are. And I get to be called "Asshole".

And last but not the least, I am going to take this to whatever eclipse
formation that can deal with that.

At first not treating community, I can see that happening in a lot of open
source projects. With 15 years of open source / eclipse interaction, this is
the time I am called an asshole. That's a first.