On 19.07.2013 19:05, Anthony Petrov wrote:
The updated fix looks fine. Thanks.
I'd only suggest to test whether the initial content.*SizeChanged() calls in the setContent() are
enough, or they should be moved/duplicated somewhere else in case creating and showing of a
SwingNode instance is stretched in time, so as to make sure the FX code always operates with valid
values when the SwingNode gets added to a live scene or gets modified while still being inserted
into the scene.
What you're talking about relates to the javafx part of the fix (which I'll submit for review
separately via the javafx review tool, and I'll add you as a reviewer =).
Just to answer your question. The layout data is stored in SwingNode. When it's added to scene after
the content has been set on it, it will return valid values via its pref/max/minWidth/Heigh()
methods which its containing layout pane will ask for while laying it out as a newly added node. If
a content is set/replaced on already visible SwingNode, it will ask for relayout via calling
impl_notifyLayoutBoundsChanged(). If you're suggesting to test it, let me leave that to SQE as they
have "layout" in their SwingNode test plan. Because layout means quite a lot testcases...
Thanks,
Anton.
--
best regards,
Anthony
On 08/25/2013 06:53 PM, Anton V. Tarasov wrote:
Hi Anthony,
Thanks for the review.
On 19.07.2013 17:43, Anthony Petrov wrote:
Hi Anton,
The fix looks good overall.
I'm just not sure about the exact symmetry between the setContent()
method and the componentRemoved() listener wrt. the layoutSizeListener
adding/removing. In theory, the setContent() may be called several
times with different (or same) content objects. Similarly, the
contentPane may in theory be added or removed manually several times
(or even transferred between different JLFs).
setContent() is the API method which we don't control (and which indeed,
in theory, may be called several times with any content).
But the content pane is what we manage internally. It's tight to the JLF
instance and follows its life cycle. (Yes, a developer may hack it, but
this is strongly discouraged =)
Perhaps both adding and removing the layoutSizeListener should happen
in componentAdded() and componentRemoved() correspondingly?
I like the idea. Here's the new version:
http://cr.openjdk.java.net/~ant/JDK-8020927/webrev.1
Thanks,
Anton.
--
best regards,
Anthony
On 07/19/2013 04:27 PM, Anton V. Tarasov wrote:
Please, review a fix.
jira: https://jbs.oracle.com/bugs/browse/JDK-8020927
webrev: http://cr.openjdk.java.net/~ant/JDK-8020927/webrev.0
Layout bounds notifications are added to internal JLightweightFrame API.
(Just FYI, related fx changes are here:
http://cr.openjdk.java.net/~ant/RT-30650/webrev.0)
Thanks,
Anton.
