Bug 197838 - [Forms] ExpandableComposite increases Browser client's size increase on a re-layout
Summary: [Forms] ExpandableComposite increases Browser client's size increase on a re-...
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: User Assistance (show other bugs)
Version: 3.2.1   Edit
Hardware: PC Windows XP
: P3 major (vote)
Target Milestone: 3.4 M2   Edit
Assignee: Adam Archer CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2007-07-25 13:05 EDT by Srimanth CLA
Modified: 2007-09-25 12:22 EDT (History)
2 users (show)

See Also:


Attachments
Sample View showing the re-layout problem. (1.53 KB, application/octet-stream)
2007-07-25 13:06 EDT, Srimanth CLA
no flags Details
patch (2.56 KB, patch)
2007-08-17 10:51 EDT, Adam Archer CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Srimanth CLA 2007-07-25 13:05:31 EDT
I have a simple layout of 
Composite (rowlayout)
 - Button (push)
 - ExpandableComposite
   - Browser (client)

the push button asks the top Composite to re-layout and redraw itself. Just doing this causes the width of the ExpandableComposite to increase when it should not. Something about the browser and ExpandableComposite's layout.

I have attached a sample view containing the scenario reproducing this problem.
Comment 1 Srimanth CLA 2007-07-25 13:06:32 EDT
Created attachment 74595 [details]
Sample View showing the re-layout problem.
Comment 2 Srimanth CLA 2007-07-26 16:51:35 EDT
After some quick debugging the problem seems to be in ExpandableLayout class. For a browser, it computes the size of the ExpandableComposite perfectly, but when laying out the children it forgets to remove a VSPACE from the height of the browser. Due to this browser height keeps growing by VSPACE everytime a layout is done.

Also an inconsistency is that when computing the size the following is done
			if (hasTitleBar())
				height += VSPACE;
and while laying it out 
			if (hasTitleBar())
				y += tvmargin;
is done.
Comment 3 Dejan Glozic CLA 2007-08-16 11:28:37 EDT
Adam, see if this is still the case. There have been some changes in 3.3 in layout computation - this may have been fixed already.
Comment 4 Adam Archer CLA 2007-08-16 16:00:44 EDT
(In reply to comment #2)
> After some quick debugging the problem seems to be in ExpandableLayout class.
> For a browser, it computes the size of the ExpandableComposite perfectly, but
> when laying out the children it forgets to remove a VSPACE from the height of
> the browser. Due to this browser height keeps growing by VSPACE everytime a
> layout is done.

I am not seeing the browser grow vertically running the sample code on I20070815-1600. I am, however, seeing it grow horizontally by 19 pixels on each layout. What seems to be happening is that the compute size is finding the maximum width of all its parts including the label/text client, the description, and the client. Once if finds the largest, it is checking if there is a toggle and, if there is, adding its width to the overall result. The toggle width should only affect the overall width if it is based on the title/text client. Since the width, in this case, is based on the browser, adding the toggle size to it is increasing the result of the x coordinate on compute size every time.

I have come up with a fix for this, but I'm very surprised that this isn't affecting more forms clients if the problem is actually as serious as it appears. I will test some more before uploading it.

> Also an inconsistency is that when computing the size the following is done
>                         if (hasTitleBar())
>                                 height += VSPACE;
> and while laying it out 
>                         if (hasTitleBar())
>                                 y += tvmargin;

The lines you are referencing in the computeSize method are commented out and the result that it returns adds tvmargin to the y coordinate. Earlier in the computeSize(), tvmargin is set according to the result of hasTitleBar(). Thus, the result is the same as the behaviour in the layout.
Comment 5 Adam Archer CLA 2007-08-17 10:51:45 EDT
Created attachment 76308 [details]
patch

There were a couple problems in computeSize and layout for ExpandableComposite. It seems the the toggle width in the computeSize() had an extra IGAP in all cases.

Also, as I mentioned in comment 4, the toggle width was being added to the total width even if it was set based on the client. This should only be the case if the CLIENT_INDENT style is used.

This patch fixes these problems and has tested well so far.
Comment 6 Dejan Glozic CLA 2007-08-27 14:02:21 EDT
Patch released into HEAD and versioned for the next I build.