<Swing Dev> Reviewer needed: fix for TitledBorder class

Pavel Tisnovsky ptisnovs at redhat.com
Fri May 4 09:30:06 UTC 2012


Pavel Porvatov wrote:
> Hi Pavel,
>> Alexander Scherbatiy wrote:
>>> On 5/2/2012 5:09 PM, Pavel Tisnovsky wrote:
>>>> Hi Pavel and Alexander,
>>>> I've prepared second version of fix for TitledBorder class&   JTreg
>>>> test is
>>>> included too. Both changes can be seen here:
>>>>
>>>> http://cr.openjdk.java.net/~ptisnovs/TitledBorder_v2/
>>>>
>>>> Could you please look at it?
>>>      Just two brief questions.
>>>     - Why do we need the extra Font f variable? Is it possible to use
>>> only the titleFont?
>> Yes, you are right - it is not really needed, it only makes debugging
>> easier.
>> It's possible to shorten both getters to one liner.
> I'm also voting to remove unnecessary variable.

Hi Pavel and Alexander,

here's third version of TitledBorder fix - getters now don't contain
any temporary variable:

http://cr.openjdk.java.net/~ptisnovs/TitledBorder_v3/

>>
>>>     - In case if the titleFont variable is null would it be better to
>>> set
>>> it to the default L&F font?
>>>       So in the next getTitleFont() call the
>>> UIManager.getFont("TitledBorder.font")  method will not be executed.
>> Hmm it might be better (it is even possible to do it directly in
>> setters which
>> could be called from constructor), but
>> it changes object state (it's attributes) and it IMHO could(?) cause
>> problems
>> with serialization and/or XML encoding. If you think it's not a problem
>> I'll be happy to create new version of this fix.
> I think we can't set variable, because after LAF changing the current
> functionality will be broken.
> 
> BTW: why don't you use Oracle copyright in test header?

Well, I'm not Oracle employee ;-) I've asked Mark Reinhold about this
issue and he told me that it's ok to use RH copyright header.

Thank,
Pavel
> 
> Regards, Pavel
>>
>>>    Thanks,
>>>    Alexandr.
>>>
>>>> Thank you in advance,
>>>> Pavel
>>>>
>>>>> Regards, Pavel
>>>>>>>      Thanks,
>>>>>>>      Alexandr.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 4/27/2012 1:14 PM, Pavel Tisnovsky wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> I think there's a bug in a TitledBorder class. According to JavaDoc
>>>>>>>> the methods getTitleColor() and getTitleFont() should use look&feel
>>>>>>>> settings when nothing is explicitly changed by constructor/setters:
>>>>>>>>
>>>>>>>> <javadoc>
>>>>>>>> If the border, font, or color property values are not specified in
>>>>>>>> the
>>>>>>>> constuctor or by invoking the appropriate set
>>>>>>>> methods, the property values will be defined by the current look
>>>>>>>> and
>>>>>>>> feel, using the following property names in the
>>>>>>>> Defaults Table:
>>>>>>>> "TitledBorder.border"
>>>>>>>> "TitledBorder.font"
>>>>>>>> "TitledBorder.titleColor"
>>>>>>>> </javadoc>
>>>>>>>>
>>>>>>>> This behaviour were removed by following changeset:
>>>>>>>> changeset:   2529:d062afbe2107
>>>>>>>> user:        malenkov
>>>>>>>> date:        Thu Jul 01 18:09:45 2010 +0400
>>>>>>>> summary:     4129681: Cannot get a title border to display its
>>>>>>>> label
>>>>>>>> as disabled
>>>>>>>>
>>>>>>>>
>>>>>>>> Here is a webrew which contains fix for this issue:
>>>>>>>> http://cr.openjdk.java.net/~ptisnovs/TitledBorder/
>>>>>>>>
>>>>>>>> Can anybody please review this fix?
>>>>>>>> (I'd like to push the same change to OpenJDK7 too it it will be
>>>>>>>> reviewed&     accepted)
>>>>>>>>
>>>>>>>> Thank you in advance,
>>>>>>>> Pavel Tisnovsky
> 




More information about the swing-dev mailing list