com.codename1.ui.Component.preferredSizeImpl() is unsafe as it sets the preferredSize from calcPreferredSize() which may be overridden

[GoogleCodeExporter]

com.codename1.ui.Component.preferredSizeImpl() is unsafe as it sets the 
preferredSize from calcPreferredSize() which may be overridden.

This can be troublesome because on the other side getPreferredSize() returns a 
reference to the private member preferredSize. If it springs to mind to 
override calcPreferredSize() of a component and to return the result of 
getPreferredSize of another Component one can get very funny results which 
could easily drive one crazy.

Hence I suggest that within com.codename1.ui.Component.preferredSizeImpl() the 
result of calcPreferredSize() should better be transferred into the 
preferredSize member by using its com.codename1.ui.geom.Dimension.setWidth(int) 
and com.codename1.ui.geom.Dimension.setHeight(int) methods.

Original issue reported on code.google.com by Stefan.A...@gmail.com on 19 Feb 2015 at 2:27

[GoogleCodeExporter]

com.codename1.ui.list.ContainerList.Entry.calcPreferredSize() does this crazy 
thing which enforces all entries to the same size as the renderer. That surely 
is not intended.

Original comment by Stefan.A...@gmail.com on 19 Feb 2015 at 2:41

[GoogleCodeExporter]

I don't understand this issue at all. calcPreferredSize is and should be 
overriden and that works fine. 
I looked at ContainerList and have yet to spot the crazy thing that forces all 
entries to the same size.

Original comment by shai.almog on 21 Feb 2015 at 9:50

[GoogleCodeExporter]

Well, what happens is that the mathod 
com.codename1.ui.list.ContainerList.Entry.calcPreferredSize() returns the 
preferredSize Dimension instance of the renderer which then gets injected into 
all the ContainerList entries because of the behavior of 
com.codename1.ui.Component.preferredSizeImpl().
All the preferredSize attributes of the ContainerList then share the same 
Dimension instance, as does the renderer component.
Was that clear enough? 

Original comment by Stefan.A...@gmail.com on 21 Feb 2015 at 9:56

[GoogleCodeExporter]

My suggested fix:

Original comment by Stefan.A...@gmail.com on 21 Feb 2015 at 10:08

Attachments:

• [Bildschirmfoto vom 2015-02-21 11:06:38.png](https://storage.googleapis.com/google-code-attachments/codenameone/issue-1364/comment-4/Bildschirmfoto vom 2015-02-21 11:06:38.png)

[GoogleCodeExporter]

I don't see the logic in this change. It doesn't solve the issue and FYI the 
suggested fix also causes a null pointer exception (which is easily fixed).
I did see there is an issue with ContainerList sizing incorrectly but I don't 
see why that is the reason for that.

Original comment by shai.almog on 21 Feb 2015 at 3:36

[GoogleCodeExporter]

Yes. It certainly solves the issue. 
Look at com.codename1.ui.Component.setPreferredSize(Dimension). It does the 
right thing by not just assigning the Dimension instance.

Sorry I missed the check for null in my code example. I did not test it. It was 
merely meant as a hint.

With Codename One the preferredSize is cached and only calculated when 
requested by the shouldCalcPreferredSize attribute. 
com.codename1.ui.Component.preferredSizeImpl() just assigns the Dimension 
instance which is returned by the calcPreferredSize() method. The ContainerList 
overrides calcPreferredSize() for its entries and returns the renderer 
conponents preferredSize. Therefore all the ContainerList entries end up with a 
reference to the single Dimension instance taken from the renderer components 
preferredSize attribute.
That causes all the ContainerList entries to have the same size as the entry 
for which the perferredSize has been calculated most recently. Because all 
entries share the same Dimension object as their preferredSize attribute.

On the other hand one might even question whether it is a good idea to return 
the reference to the private attribute preferredSize's Dimension instance in 
the com.codename1.ui.Component.getPreferredSize(). Would one really want to be 
able to alter width and height of the preferredSize without setting it through 
the public setter method?

Original comment by Stefan.A...@gmail.com on 21 Feb 2015 at 4:05

[GoogleCodeExporter]

I tried this before answering and it didn't solve the issue (that's how I knew 
there was an exception).

Preferred size is invoked for every Entry instance which invokes a preferred 
size on a different renderer instance hence calling calcPreferredSize() for 
every entry.

Returning an internal instance is something we do since the library is 
statically linked so the security aspect of exposing internal state is 
irrelevant.

Original comment by shai.almog on 22 Feb 2015 at 7:52

[GoogleCodeExporter]

The problem will only arise, of course, when the calcPreferredSize does not 
create a new Dimension instance for every invocation. Therefore to test it you 
must use a component derived from Component, but not a Label, for instance, 
which returns a new Dimension instance every time calcPreferredSize is called, 
since it overrides calcPreferredSize and returns  
getUIManager().getLookAndFeel().getLabelPreferredSize(this).

Original comment by Stefan.A...@gmail.com on 23 Feb 2015 at 10:49

[GoogleCodeExporter]

I posted son code to demonstrate the issue in 
https://groups.google.com/forum/#!topic/codenameone-discussions/Px4DJWito_U

Original comment by Stefan.A...@gmail.com on 23 Feb 2015 at 1:41