<Swing Dev> Reviewer needed: fix for TitledBorder class

Pavel Tisnovsky ptisnovs at redhat.com
Mon May 14 13:21:33 UTC 2012


ping - can anybody please look at this issue?

Thank you in advance,
Pavel


Pavel Tisnovsky wrote:
> 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